Skip to content

Conversation

@Horiodino
Copy link
Contributor

Description:
Added support for extraLabels in spec.observability.metrics to allow users to attach additional labels to the generated ServiceMonitor.

Fixes: #4138 (comment)

Link to tracking Issue(s):

Testing:

  • Added unit tests to cover cases:
    • With empty extraLabels
    • With custom extraLabels map containing multiple labels
  • Verified generated ServiceMonitor contains the expected labels.

Documentation:

  • Updated API docs to document the new extraLabels field under spec.observability.metrics.
  • Regenerated CRDs and bundle manifests to include the new field.

@Horiodino Horiodino requested a review from a team as a code owner July 9, 2025 13:08
@Horiodino Horiodino changed the title Add extra labels to spec observability metrics [chore] Add extra labels to spec observability metrics Jul 9, 2025
Copy link
Contributor

@iblancasa iblancasa left a comment

Choose a reason for hiding this comment

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

Please add:

  • a changelog
  • improve the e2e test to check this new feature

@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch from a9a621b to cd9ae41 Compare July 11, 2025 07:05
@github-actions
Copy link
Contributor

github-actions bot commented Jul 11, 2025

E2E Test Results

 33 files  ±0  221 suites  ±0   3h 36m 13s ⏱️ - 9m 30s
 84 tests ±0   84 ✅ +1  0 💤 ±0  0 ❌  - 1 
221 runs  ±0  221 ✅ +1  0 💤 ±0  0 ❌  - 1 

Results for commit ce341ea. ± Comparison against base commit ad9d30a.

♻️ This comment has been updated with latest results.

@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch 2 times, most recently from 3022ccc to 5ac32a6 Compare July 11, 2025 07:49
@Horiodino
Copy link
Contributor Author

@iblancasa could you please cross-check #4184?
I thought it might be related to the changes from this PR, but I already tested it locally and didn’t get any errors:

✓  internal/instrumentation (21.838s)
✓  internal/controllers (40.73s)

=== Skipped
=== SKIP: internal/controllers TestRegisterWithManager_OpAMPBridge (0.00s)
    opampbridge_controller_test.go:159: this test requires a real cluster, otherwise the GetConfigOrDie will die

=== SKIP: internal/controllers TestRegisterWithManager (0.00s)
    reconcile_test.go:1188: this test requires a real cluster, otherwise the GetConfigOrDie will die

DONE 1795 tests, 2 skipped in 55.687s

@swiatekm
Copy link
Contributor

@Horiodino rebasing on main should fix that flaky test.

@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch 3 times, most recently from 39181b7 to 901a40d Compare July 11, 2025 17:46
@swiatekm swiatekm requested a review from iblancasa July 12, 2025 13:42
@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch 2 times, most recently from d3d9c67 to a4aa808 Compare July 15, 2025 04:21
@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch from a4aa808 to ce341ea Compare July 15, 2025 05:17
@swiatekm swiatekm self-requested a review July 15, 2025 10:30
@swiatekm swiatekm dismissed iblancasa’s stale review July 15, 2025 10:32

The objections have been addressed, and the reviewer is currently unavailable. Dismissing to avoid blocking the PR unnecessarily.

@Horiodino Horiodino force-pushed the Add-extraLabels-to__spec_observability_metrics branch from ce341ea to c44c817 Compare July 15, 2025 11:10
@swiatekm
Copy link
Contributor

Thank you for the contribution and patience with the review process, @Horiodino!

@swiatekm swiatekm merged commit 4ab7a05 into open-telemetry:main Jul 16, 2025
48 checks passed
@danielfm
Copy link

Will this change be bundled into the next stable release? Any expected date for the launch?

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.

Add extraLabels to spec.observability.metrics

5 participants