Vector: Provide metric naming and strategy overrides in the prometheus sink

Created on 5 Aug 2019  路  13Comments  路  Source: timberio/vector

Currently the prometheus sink does not offer the ability to rename or change metric types. This is described in detail in point 3 via https://github.com/timberio/vector/pull/675#issuecomment-514770387. The config interface should look something like:

[sinks.prometheus_id]
  type = "prometheus"

  # Renames all foo.* metrics to bazz.*
  [[sinks.prometheus_id.metrics]]
    match_name = "foo.*" # regex pattern
    name = "bazz.$1"

  # Changes all histogram type metrics to a summary
  [[sinks.prometheus_id.metrics]]
    match_type = "histogram"
    type = "summary"

  # Changes all bar.* metrics to a histogram with the specified buckets
  [[sinks.prometheus_id.metrics]]
    match_name = "bar.*" # regex pattern
    type = "histogram"
    buckets = [0.1, 0.25, 0.5, 0.75, 1] # optional

  # ... and so on ...

Requirements

  1. The match_name is just a name I came up with, feel free to rename this.
  2. The match_name should support regex, but feel free to provide a limited matching pattern if we have a reason for it (such as a performance benefit).
  3. We should not support every conversion if it adds a ton of complexity (ex: set to counter). In general, we do not need to support unrealistic edge cases if they provide a lot of overhead.
  4. The first match wins and the order they are executed is bottom-up. Meaning the last rule defined will be executed first. Note the proposed defaults here: https://github.com/timberio/vector/issues/711#issuecomment-522329503.
  5. Metric renaming and definitions are likely to live in every metrics sink (ex: aws_cloudwatch_metrics). It might be worth thinking about ways to make this simple, although each metrics sink will have specific settings (ex: buckets) so I'm unsure if this can be fully extracted.
metrics remap sinks approval more demand prometheus enhancement

All 13 comments

@lukesteensen do you mind reviewing this issue to make sure there aren't any glaring issues? If you think this is good can you remove the meta: needs review label?

Makes sense, just a few comments:

  • I'd probably put them under an overrides key rather than a metrics key in the config, which didn't feel terribly clear.

  • I might prefer something like name_pattern over match_name?

  • We'll need to think through how to handle incoming events where the specified type doesn't make sense. Right now I think we implicitly branch based on the type first. If this becomes a pre-processing step, we'll have to decide which conversions make sense to support and what to do with events where there is no conversion. My guess is that there are only a very limited number of conversions that make logical sense, so we definitely shouldn't spend extra time trying to support weird cases.

  • There seem to be two types of these: (1) things like renames (which are simple event -> event transforms), and (2) sink-specific config overrides (e.g. buckets). While it might make sense to expose them to the user in the same way, I'm not sure if they should be implemented in the same way. We could also consider splitting out (1) into a normal transform instead of having a shared implementation within sinks.

377 looks relevant.

I'm not sure about #377. It'd be simple, but almost too simple to be worth its own implementation and docs. Unless there's a strong demand, I wouldn't worry about it as we work on this issue.

Yeah, I just thought that it was capturing the idea of

We could also consider splitting out (1) into a normal transform instead of having a shared implementation within sinks.

Also it seems that string interpolation could be useful in this context. But I don't see a clear path of doing that, because we do not pass implicit fields to metrics.

This would be an excellent transform to have. My current use case is re-exporting statsd metrics to the prometheus sink, which in some cases will actually work very well already if you control the code carefully. But some cases are impossible: many applications use a name like foo.bar in order to register statsd metrics, which are invalid promethus stat names. In such a setup I could do something like this:

[sinks.prometheus_id]
  type = "prometheus"
  [[sinks.prometheus_id.metrics]]
    match_name = "(.?)\.(.?)"
    name = "$1_$2"

which would be excellent!

^ @lukesteensen I believe we could add some names normalisation into the Prometheus sink. I don't really think we need a transform or special configuration for that.

Replacing unsupported characters with underscores seems natural, and we were actually doing a similar thing for internal metrics.

I'd be willing to write a patch for some basic normalization rules like underscore-to-period if that's fine and easy to do for the Prometheus sink.

Normalizing metric names that are invalid for Prometheus seems like a perfectly reasonable thing for the Prometheus sink to do. That should get quite a bit easier with #1217, since there is only a single name field to worry about. It should be quite straightforward to do early in the start_send implementation. Does that sound right to you @loony-bean?

@thoughtpolice Sure!

According to this doc metric names must match [a-zA-Z_:][a-zA-Z0-9_:]* regex, and label names must match this one [a-zA-Z_][a-zA-Z0-9_]*. The rule could be to replace unsupported symbols with '_'.

Please let me know if you'll have any questions.

Any updates ?

@freeseacher we haven't made any updates to the Prometheus sink recently, but we'll be spending more time on our metrics support soon. This is one we should be able to tackle as part of that work.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

raghu999 picture raghu999  路  3Comments

LucioFranco picture LucioFranco  路  3Comments

a-rodin picture a-rodin  路  3Comments

trK54Ylmz picture trK54Ylmz  路  3Comments

jhgg picture jhgg  路  4Comments