Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make mapper cache respect metric type #229

Conversation

andy-paine
Copy link
Contributor

@andy-paine andy-paine commented Jun 6, 2019

  • Statsd allows users to provide a metric with the same name but
    differing types (counter, gauge, timer)
  • The exporter allows this by letting users specify a
    "match_metric_type" in the mapping config
  • However the mapper cache does not look at the metric type so it would
    return a MetricMapperCacheResult for the type of the first metric with
    that name that the exporter saw
  • Add MetricType to the signature for the cache and format the metric
    name with the type to provide unique keys for a metric with the same
    name but differing type

Fixes #228

@andy-paine andy-paine force-pushed the make-mapper-cache-respect-metric-type branch from 26ac776 to 97c4051 Compare June 6, 2019 10:29
- Statsd allows users to provide a metric with the same name but
  differing types (counter, gauge, timer)
- The exporter allows this by letting users specify a
  "match_metric_type" in the mapping config
- However the mapper cache does not look at the metric type so it would
  return a MetricMapperCacheResult for the type of the first metric with
that name that the exporter saw
- Add MetricType to the signature for the cache and format the metric
  name with the type to provide unique keys for a metric with the same
name but differing type

Signed-off-by: Andy Paine <[email protected]>
@andy-paine andy-paine force-pushed the make-mapper-cache-respect-metric-type branch from 97c4051 to 0135b40 Compare June 6, 2019 10:34
@SpencerMalone
Copy link
Contributor

To me this looks good, do you have thoughts on adding a test? If not, I'm good with this going in, and I can open a followup PR for a test to try and ensure this edge case isn't missed in any future work.

@andy-paine
Copy link
Contributor Author

My Golang isn't all that great - I tried writing some tests but given the lack of tests in general in this repo I didn't really have a pattern to follow and don't feel in a position to be defining patterns for a whole repo. I figured I would raise this now and potentially look to add some tests later.

@matthiasr
Copy link
Contributor

ok, I'll merge it then and wait for the test from @SpencerMalone

@matthiasr matthiasr merged commit 8551a65 into prometheus:master Jun 6, 2019
matthiasr pushed a commit that referenced this pull request Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Match metric type not working as documented
3 participants