Skip to content

Conversation

@vlad-coman-hs
Copy link
Contributor

This PR addresses the need of adding dynamic attributes on the auto-instrumented metrics from otelgrpc. It potentially addresses #6026 without giving access to the payload since it is not available during every phase which adds complexity when trying to add the labels to all available metrics. It might also generate more confusion to the end user by exposing internal implementation details.

Initial draft that implemented the Labeler pattern from otelhttp (it also explains why it was closed): #8135

@vlad-coman-hs vlad-coman-hs requested review from a team and dashpole as code owners November 19, 2025 09:37
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 19, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: vlad-coman-hs / name: Vlad Coman (baaae0f)
  • ✅ login: vlad-coman-hs / name: vlad-coman-hs (6e56dbb)

@dashpole
Copy link
Contributor

Can you add documentation or an example_test.go to demonstrate how someone could use this to add attributes to metrics?

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (9b21944) to head (6e56dbb).
⚠️ Report is 32 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8191   +/-   ##
=====================================
  Coverage   82.3%   82.3%           
=====================================
  Files        193     193           
  Lines      13769   13779   +10     
=====================================
+ Hits       11334   11344   +10     
  Misses      2030    2030           
  Partials     405     405           
Files with missing lines Coverage Δ
...entation/google.golang.org/grpc/otelgrpc/config.go 87.0% <100.0%> (+0.8%) ⬆️
...n/google.golang.org/grpc/otelgrpc/stats_handler.go 98.5% <100.0%> (+<0.1%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vlad-coman-hs
Copy link
Contributor Author

Hello @dashpole,
Just wanted to check in on this PR. Please let me know if there's anything I should adjust or clarify.
Thank you for your time!

@dashpole
Copy link
Contributor

There was a release, so you need to move the changelog entry up to the unreleased section. You may need to add the appropriate header if the section doesn't already exist.

Co-authored-by: David Ashpole <[email protected]>
@vlad-coman-hs
Copy link
Contributor Author

Hi! Thanks for reviewing this. Please let me know if there's anything else needed from my side before merging.

@dmathieu
Copy link
Member

Nothing needed on your end for now. We require a second approval though.

Copy link
Member

@flc1125 flc1125 left a comment

Choose a reason for hiding this comment

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

Based on the current situation, I think there is no problem; however, I am not entirely sure if this is the best implementation plan.

@dashpole dashpole merged commit 92f77de into open-telemetry:main Jan 6, 2026
29 checks passed
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.

4 participants