chore: Add leader election for cluster metrics#42
Merged
Conversation
jsingleton-dev
requested review from
adityachoudhari26,
jsbroks and
zacharyblasczyk
as code owners
jsingleton-dev
temporarily deployed
to
Helm Charts
GitHub Actions
Inactive
📝 WalkthroughWalkthroughThis PR adds Kubernetes leader election support to OpenTelemetry's Kubernetes cluster receiver. When the ChangesOTEL Leader Election Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
charts/ctrlplane/charts/otel/templates/_config.tpl (1)
62-64: 💤 Low valueConsider a more flexible regex pattern.
The regex on line 64 assumes a specific order and formatting for the extensions list. While this works for the current implementation, it could break if whitespace or ordering changes in the future.
More flexible alternative
You could use separate assertions to be more resilient:
- matchRegex: path: data.config pattern: 'service:[\s\S]*extensions:[\s\S]*- k8s_leader_elector'This validates that
k8s_leader_electorappears in the service extensions list without assuming specific ordering or adjacent items. However, the current approach is perfectly acceptable for Helm chart testing where template output is predictable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/ctrlplane/charts/otel/templates/_config.tpl` around lines 62 - 64, The current matchRegex pattern in the matchRegex block for path: data.config is too strict about ordering/whitespace; update the pattern used by matchRegex (the pattern property) to a more flexible regex that uses broader dotall-style matching (e.g., using [\s\S]* between sections) so it asserts "service:" appears before "extensions:" and that "k8s_leader_elector" occurs somewhere in the extensions list without requiring exact adjacency or whitespace; locate the matchRegex/pattern entry in the template and replace the rigid pattern with the relaxed one while keeping path: data.config and the matchRegex key intact.charts/ctrlplane/tests/otel_leader_election_test.yaml (1)
62-64: ⚡ Quick winMake
service.extensionsassertion order-agnostic to reduce flaky tests.This regex hard-codes extension order and formatting, so harmless reordering can break the test. Prefer asserting presence of both entries independently (or parse/assert list membership) instead of sequence.
Proposed test adjustment
- - matchRegex: - path: data.config - pattern: 'extensions:\s*\n\s+- health_check\s*\n\s+- k8s_leader_elector' + - matchRegex: + path: data.config + pattern: 'extensions:[\s\S]*- health_check' + - matchRegex: + path: data.config + pattern: 'extensions:[\s\S]*- k8s_leader_elector'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/ctrlplane/tests/otel_leader_election_test.yaml` around lines 62 - 64, The test currently uses matchRegex on path "data.config" with a hard-coded ordered pattern ('extensions: ... - health_check ... - k8s_leader_elector') which is brittle; update the assertion for "service.extensions" to be order-agnostic by replacing the single ordered regex with either two independent presence checks (e.g., separate matchRegex/assertions for "- health_check" and "- k8s_leader_elector" against data.config) or a single regex that uses lookahead assertions to require both "- health_check" and "- k8s_leader_elector" under the "extensions:" block so the test no longer depends on extension order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@charts/ctrlplane/charts/otel/templates/_config.tpl`:
- Around line 62-64: The current matchRegex pattern in the matchRegex block for
path: data.config is too strict about ordering/whitespace; update the pattern
used by matchRegex (the pattern property) to a more flexible regex that uses
broader dotall-style matching (e.g., using [\s\S]* between sections) so it
asserts "service:" appears before "extensions:" and that "k8s_leader_elector"
occurs somewhere in the extensions list without requiring exact adjacency or
whitespace; locate the matchRegex/pattern entry in the template and replace the
rigid pattern with the relaxed one while keeping path: data.config and the
matchRegex key intact.
In `@charts/ctrlplane/tests/otel_leader_election_test.yaml`:
- Around line 62-64: The test currently uses matchRegex on path "data.config"
with a hard-coded ordered pattern ('extensions: ... - health_check ... -
k8s_leader_elector') which is brittle; update the assertion for
"service.extensions" to be order-agnostic by replacing the single ordered regex
with either two independent presence checks (e.g., separate
matchRegex/assertions for "- health_check" and "- k8s_leader_elector" against
data.config) or a single regex that uses lookahead assertions to require both "-
health_check" and "- k8s_leader_elector" under the "extensions:" block so the
test no longer depends on extension order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f643a42a-0727-44d4-80c8-22359a84db95
⛔ Files ignored due to path filters (1)
charts/ctrlplane/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
charts/ctrlplane/Chart.yamlcharts/ctrlplane/charts/otel/Chart.yamlcharts/ctrlplane/charts/otel/templates/_config.tplcharts/ctrlplane/charts/otel/templates/_receivers.tplcharts/ctrlplane/charts/otel/templates/clusterrole.yamlcharts/ctrlplane/tests/otel_leader_election_test.yaml
zacharyblasczyk
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The OTeL collector runs with one pod per node. If we enable cluster metrics as is, each node would report the same metrics and we would have duplicate numbers. In order to avoid duplicates, we can use the leader election extension for OTeL. This will elect a single leader from the DaemonSet to report cluster metrics and will prevent duplicate reporting.
An alternative would be to make a separate single-replica deployment for just cluster metrics, but that would require a larger change and this leader election fits nicely into our existing charts.
Summary by CodeRabbit
Release Notes
New Features
Chores