Metrics

[lukesteensen]

Closes #64

I would review the commits one at a time. The high level steps are:

1. Add a metrics variant of Event that's basically just the statsd metric model for now
2. Do a minimal transition of statsd source and prom sink to use the new model
3. Simplify the prom sink so you don't need to configure every metric you want it to collect
4. Add a log_to_metric transform that can turn a log event into a metric event in much the way that the prom sink used to

This is a little more limited than I'd like, but there were some hairy things I decided to roll back and wait on for the timing being. Some of the limitations:

• Due to how we've designed the Transform trait, you can only derive a single metric from a given log event at a time, so it currently looks through its config in order and take the first one that matches (you could work around this by configuring multiple transforms)
• The transform and prometheus sink only support counters and gauges right now, all other metric types are ignored
• Because the prometheus sink will aggregate any metrics you send it, there's the potential for unbounded memory use. It'd probably be good to provide some default limit there.
• Both the transform and the prom sink just take the field name as the name of the metric as is. It'd be nice to have an option for configuring that, but you can work around it by putting another transform in front

I'm planning to write up issues for most of those limitations.

Refs aws/containers-roadmap#699

[LucioFranco]

Looks good to me! Nothing really stands out as needing a change. So just some minor comments.

[LucioFranco]

just gonna drop this thought here, do we want to include tags?

[lukesteensen]

That'd be a good subject for an RFC! Personally I'm not super familiar with metrics systems that support tags, so I'd have to do some more research and probably implement a sink or two that uses them before I'd be comfortable baking them into the data model.

[LucioFranco]

it should be something we can add later that is backwards compat with our proto.

[LucioFranco]

possible to have a is_metric?

[lukesteensen]

I still think that's an anti pattern and consumers should know what type they're getting and just do the coercion.

[LucioFranco]

why does this not have its own config?

[lukesteensen]

I couldn't think of anything to configure for gauges. You need to be able to parse the value, which is the only option we have for counters.

[LucioFranco]

do we need this dbg!?

[lukesteensen]

Nope! Good catch

[LucioFranco]

warn!?

[lukesteensen]

This is another place where I'm torn. I feel like a mistake in configuration could cause us to absolutely blow up logging these errors on every event, so I'd feel a lot more comfortable if we had a way to throttle them before turning them on by default.

[LucioFranco]

yeah that is a good point, I can see if this is something we could include in a tokio-trace subscriber.

[LucioFranco]

same here?

[lukesteensen]

Same as #374 (comment)

[binarylogic]

This is fantastic for a first step, nice work.

Because the prometheus sink will aggregate any metrics you send it, there's the potential for unbounded memory use. It'd probably be good to provide some default limit there.

I would love to understand this more, but this is fine for now.

[lukesteensen]

@binarylogic I was thinking about cases where a transform would unintentionally create a field name with super high cardinality (i.e. "I thought I was parsing out the hostname but ended up getting the timestamp"), which would mean a new counter/gauge for pretty much every event that comes in.