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

fix: prepend the special __name__ label to the front of the label slice (prometheusremotewrite) #11674

Conversation

nvinzens
Copy link
Contributor

Required for all PRs

resolves #9365

The Thanos project recently merged thanos-io/thanos#5508 which will break every setup that uses telegraf in its current state to remote write to Thanos. All write requests will be rejected with a 409.
There's a "hidden" prerequisite that all labels in Prometheus TSDB (which Thanos also uses) need to be sorted, I couldn't find official documentation that mentions this but it has come up in other PRs, for example prometheus/prometheus#5372.

My change will always put the __name__ as the first label. This could potentially also be wrong when labels that start with uppercase letters are used but since it was already completely broken I would like to fix this without introducing too big of a change (and why would you ever use uppercase label names?).

I'm not sure if this should be mentioned in any README and I'm also not sure how we can test this since I don't see anything regarding the __name__ label in the telegraf metrics that are used in the tests. One of the tests has a slight change in the metric ordering but that should be OK since the goal is to sort by the metric names and not by the labels themselves.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Aug 15, 2022
@nvinzens nvinzens force-pushed the bugfix/prometheusremotewrite-label-ordering branch from 0094986 to a454fc6 Compare August 15, 2022 15:13
@nvinzens
Copy link
Contributor Author

I would have to run make fmt to make the test-go-linux check pass but that would introduce quite a lot of changes in many files to this PR, should I commit that?

@nvinzens
Copy link
Contributor Author

Closed in favor of #11683

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
Development

Successfully merging this pull request may close these issues.

Prometheus remote write with Thanos out of order metrics stops metrics processing
1 participant