interop/xds: Avoid setting unit suffixes#8479
Conversation
b5dccae to
f63369e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8479 +/- ##
==========================================
- Coverage 82.47% 82.41% -0.06%
==========================================
Files 414 413 -1
Lines 40531 40518 -13
==========================================
- Hits 33429 33394 -35
- Misses 5743 5762 +19
- Partials 1359 1362 +3 🚀 New features to boost your workflow:
|
arjan-bal
left a comment
There was a problem hiding this comment.
I went through the otel PRs that caused the behaviour change again and I think that there is a bug in gRPCs otel plugin:
We're specifying the unit as call instead of {call}. Based on the metric naming conventions anything enclosed in curly braces is an annotation and in the absence of a prefix, the unit is 1. The gRPC docs also mention that non-standard units need to be enclosed in curly braces: https://grpc.io/docs/guides/opentelemetry-metrics/#server-instruments.
I believe the correct fix would be to follow the gRPC docs and enclose the required units in {}, however I'm not sure how this would impact existing users of the gRPC otel plugin.
|
Oh good catch. I would classify that as a bug fix. We should include it in the release notes, but I don't think we have much other option here.... |
|
Closing this PR, will open another PR with the correct fix. |
The prometheus exporter changed it's behaviour to add not standard unit suffixes to metrics: open-telemetry/opentelemetry-go#6839. As a result
grpc_server_call_started_totalbecomesgrpc_server_call_started_call_total.From the release notes:
Similar fix in another project: https://github.com/pomerium/pomerium/pull/5749/files
Tested
Verified by running the otel examples that the units are no longer part of the metric names.
RELEASE NOTES: N/A