-
Notifications
You must be signed in to change notification settings - Fork 4.5k
examples/opentelemetry: enabling experimental metrics #8441
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
base: master
Are you sure you want to change the base?
examples/opentelemetry: enabling experimental metrics #8441
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8441 +/- ##
==========================================
+ Coverage 82.31% 82.36% +0.04%
==========================================
Files 414 413 -1
Lines 40464 40513 +49
==========================================
+ Hits 33308 33368 +60
+ Misses 5791 5774 -17
- Partials 1365 1371 +6 🚀 New features to boost your workflow:
|
Please link the comment in response to which this PR has been raised. @vinothkumarr227 |
@arjan-bal This fix was made following the review comment from in this discussion |
@@ -70,8 +70,8 @@ func main() { | |||
// up-to-date list of metrics, see: | |||
// https://grpc.io/docs/guides/opentelemetry-metrics/#instruments | |||
Metrics: opentelemetry.DefaultMetrics().Add( | |||
"grpc.client.attempt.started", | |||
"grpc.client.attempt.duration", | |||
"grpc.xds_client.resource_updates_valid", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I am wrong but I dont think we are using xds in this example , so using xds metrics might not make sense. Can you change to use some other metrics? Example : grpc.lb.pick_first. connection_attempts_succeeded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eshitachandwani , Thanks for the review! I don’t think we need to change this example — we’ve already mentioned that it’s meant to demonstrate experimental metrics. Also, this is aligned with the discussion here: grpc-go#8234 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the example in my opinion should be a working example , like if someone is trying to run the example , they should be able to see some metrics. And since the example is not using xds , there will be no metrics for the xds client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I’ve updated the example to use pick_first with a valid address, and it now emits metrics
Fixes #8311
RELEASE NOTES: