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

loki/wal: fix panic for duplicate registered metrics #5705

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

tpaschalis
Copy link
Member

PR Description

The original issue in #5414 is easily reproducible by changing the configuration of a loki.write component and hitting the reload endpoint, as the NewWatcherMetrics will attempt to re-register the same set of metrics.

Which issue(s) this PR fixes

Fixes #5414

Notes to the Reviewer

Another possible fix would be to create these metrics at the top-level, when creating a New component and propagate a pointer to the metrics struct down to here. Both solutions could work, but for now, this is what we've mainly been doing in loki.* components when forklifting Promtail code, so an argument can be made to leave this as-is for now to not trip off people when trying to bring new Promtail code in.

PR Checklist

  • CHANGELOG.md updated
  • Documentation added (N/A)
  • Tests updated (N/A)

@tpaschalis tpaschalis self-assigned this Nov 3, 2023
@tpaschalis tpaschalis requested a review from thepalbi November 3, 2023 11:43
@tpaschalis tpaschalis marked this pull request as ready for review November 3, 2023 12:01
@tpaschalis tpaschalis requested a review from a team as a code owner November 3, 2023 12:01
@tpaschalis
Copy link
Member Author

cc @thepalbi I'm merging this, you might have to rebase your other PR, sorry for the trouble!

@tpaschalis tpaschalis merged commit 7817c22 into grafana:main Nov 3, 2023
8 checks passed
tpaschalis added a commit to tpaschalis/agent that referenced this pull request Nov 6, 2023
Signed-off-by: Paschalis Tsilias <[email protected]>
(cherry picked from commit 7817c22)
tpaschalis added a commit that referenced this pull request Nov 6, 2023
* loki/wal: fix panic for duplicate registered metrics (#5705)

Signed-off-by: Paschalis Tsilias <[email protected]>
(cherry picked from commit 7817c22)

* otelcol.exporter.prometheus add_metrics_suffixes option (#5568)

* otelcol.exporter.prometheus add add_metrics_suffixes option

* otelcol.exporter.prometheus add add_metrics_suffixes option. Add tests

* otelcol.exporter.prometheus add add_metrics_suffixes option. Update docs

* otelcol.exporter.prometheus add add_metrics_suffixes option. Added changelog, fixed readme

* otelcol.exporter.prometheus add add_metric_suffixes option. Fix option name

* otelcol.exporter.prometheus add add_metric_suffixes option. Add more tests

* otelcol.exporter.prometheus add add_metric_suffixes option. Added convertArgumentsToConvertOptions

* otelcol.exporter.prometheus add add_metric_suffixes option. Fix changelog message

(cherry picked from commit 5a7c16a)

---------

Co-authored-by: Irina <[email protected]>
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate metrics collector registration attempted
2 participants