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

Preserve metric type when using filters in output plugins #4481

Merged
merged 1 commit into from
Aug 1, 2018

Conversation

roxit
Copy link
Contributor

@roxit roxit commented Jul 27, 2018

Resolves #4501

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Steps to reproduce:

Using telegraf 1.7.2 with the following config to export kube-scheduler metrics:

[[outputs.prometheus_client]]
  listen = "0.0.0.0:9273"
  expiration_interval = "60s"
  [outputs.prometheus_client.tagpass]
    _OUTPUT = ["prometheus"]

[[inputs.prometheus]]
  urls = ["http://127.0.0.1:10251/metrics"]
  [inputs.prometheus.tags]
    _OUTPUT = "prometheus"

Histograms in http://127.0.0.1:9273/metrics look like:

...
# HELP scheduler_scheduling_algorithm_latency_microseconds_16000 Telegraf collected metric
# TYPE scheduler_scheduling_algorithm_latency_microseconds_16000 untyped
scheduler_scheduling_algorithm_latency_microseconds_16000{_OUTPUT="prometheus",host="xxx",url="http://127.0.0.1:10251/metrics"} 0
# HELP scheduler_scheduling_algorithm_latency_microseconds_1_024e_06 Telegraf collected metric
# TYPE scheduler_scheduling_algorithm_latency_microseconds_1_024e_06 untyped
scheduler_scheduling_algorithm_latency_microseconds_1_024e_06{_OUTPUT="prometheus",host="xxx",url="http://127.0.0.1:10251/metrics"} 0
...

With the tagpass filter removed from outputs.prometheus_client, the output preserved the metric type:

...
# HELP scheduler_scheduling_algorithm_latency_microseconds Telegraf collected metric
# TYPE scheduler_scheduling_algorithm_latency_microseconds histogram
...
scheduler_scheduling_algorithm_latency_microseconds_bucket{_OUTPUT="prometheus",host="xxx",url="http://127.0.0.1:10251/metrics",le="16000"} 0
scheduler_scheduling_algorithm_latency_microseconds_bucket{_OUTPUT="prometheus",host="xxx",url="http://127.0.0.1:10251/metrics",le="1.024e+06"} 0
...

Fix

The fix is simple, just pass the metric type when recreating the filtered metric in RunningOutput.
I tried to add a test for it but it seems testutil.TestMetric needs to updated to support metric type first, which seems too big to be included in this commit.

If an output plugin uses filters like tagpass/namedrop, it recreates
the metric with filtered name/tags/fields. For complex metrics like
prometheus histograms, the metric type should also be passed to recreate
the new metric, otherwise, they become untyped in the final output.
@danielnelson danielnelson added this to the 1.7.3 milestone Jul 27, 2018
@danielnelson danielnelson added the fix pr to fix corresponding bug label Jul 27, 2018
@danielnelson danielnelson requested a review from glinton July 27, 2018 19:36
@glinton glinton merged commit e538433 into influxdata:master Aug 1, 2018
glinton pushed a commit that referenced this pull request Aug 1, 2018
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
3 participants