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 panic when setting only_counter for default relabelings #279

Merged
merged 3 commits into from
Oct 11, 2022

Conversation

e1mo
Copy link
Contributor

@e1mo e1mo commented Sep 17, 2022

As noted in #216, adding relabel_configs for status or method (own testing) with only_counter would cause a panic.
The issue was, that the Collection struct (and thus histograms) were set-up without deduplicating the labels first (or at least applying the counterOnly property to the DefaultRelabelings), thus the histograms were initialised with the counterOnly labels.
I have opted against doing some sort of merging the defaults with user configuration. In part this is due to my absolute lack of knowledge of golang, but primarily since I didn't know how to differentiate user not setting values, e.g. whitelist, (=> apply defaults) or the user actively setting the defaults to revert back to them (=> do not apply defaults).

Resolve #216

Deduplicate labels used in `Collection.init()` in the same way as in
`processSource`.
Side-Effect: When overriding `method` one needs to manually add the
split/whitelist config from `DefaultRelabelings` in their own config
file.
Resolve martin-helmich#216
Copy link
Owner

@martin-helmich martin-helmich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- Thanks for your contribution! 👍

@codeclimate
Copy link

codeclimate bot commented Oct 11, 2022

Code Climate has analyzed commit 7850511 and detected 0 issues on this pull request.

View more on Code Climate.

@martin-helmich martin-helmich merged commit 334a25a into martin-helmich:master Oct 11, 2022
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.

cannot set only_counter for status, daemon panics
2 participants