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

Configure metrics exporter for loadbalancing when possible #2242

Merged
merged 11 commits into from
Dec 11, 2024

Conversation

madaraszg-tulip
Copy link
Contributor

@madaraszg-tulip madaraszg-tulip commented Dec 7, 2024

PR Description

Metrics support was turned off in otelcol.exporter.loadbalancing for two reasons:

  • metrics support is considered development upstream
  • the metrics exporter crashed when configured with the default routing_key = traceID

Solve both issues by supporting metrics only if the specified routing_key supports it and stability level is set to experimental

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@madaraszg-tulip madaraszg-tulip requested a review from a team as a code owner December 7, 2024 10:52
@madaraszg-tulip madaraszg-tulip force-pushed the loadbalancing-metrics-exporter branch 2 times, most recently from 5b6f51c to f21e22d Compare December 7, 2024 23:21
@madaraszg-tulip madaraszg-tulip force-pushed the loadbalancing-metrics-exporter branch from f21e22d to fad70bc Compare December 7, 2024 23:25
@clayton-cornell
Copy link
Contributor

@madaraszg-tulip madaraszg-tulip force-pushed the loadbalancing-metrics-exporter branch from 9913ab6 to 5e5af96 Compare December 9, 2024 19:05
@madaraszg-tulip
Copy link
Contributor Author

Does this need a doc update in https://grafana.com/docs/alloy/latest/reference/components/otelcol/otelcol.exporter.loadbalancing/ ?

I have now added a doc update to the PR

@clayton-cornell clayton-cornell requested a review from a team December 9, 2024 19:20
madaraszg-tulip and others added 6 commits December 9, 2024 21:37
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…dbalancing.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Copy link
Contributor

@ptodev ptodev left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! If I remember correctly, there were two main problems in the past:

  • We had to always send metrics, logs, and traces in OTLP components. #5684 fixed this so that we can only send a subset of those. Unfortunately, it also came with a limitation - a component is bound to the same types of telemetry throughout its lifetime. E.g. it might only be able to work for metrics.
  • A component could only have one stability label. This is not a problem anymore - we now have the stability cmd args which can check if certain functionality is experimental.

Fortunately, there are ready solutions to those problems now. Unfortunately, one remaining challenge is that this exporter can change the types of outputs at runtime. It can start by accepting only metrics, then after a config update it may want to only accept traces in a way that's not compatible with any metrics config. I'm not sure what a good way to solve this problem would look like. If you want to try to implement a solution, I'd be happy to review the PR again 😊

@ptodev ptodev self-assigned this Dec 10, 2024
@madaraszg-tulip madaraszg-tulip force-pushed the loadbalancing-metrics-exporter branch from 225d467 to 5ad5662 Compare December 10, 2024 12:55
@ptodev ptodev merged commit 057deff into grafana:main Dec 11, 2024
15 checks passed
@madaraszg-tulip madaraszg-tulip deleted the loadbalancing-metrics-exporter branch December 11, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants