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
feat: add deployment label in grpc stub metrics #5344
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5344 +/- ##
==========================================
+ Coverage 86.69% 86.80% +0.11%
==========================================
Files 99 99
Lines 6470 6488 +18
==========================================
+ Hits 5609 5632 +23
+ Misses 861 856 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
30f051c
to
cd5e45a
Compare
0c4337c
to
7ccd89d
Compare
5e9ca09
to
cbaa4bb
Compare
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.
Good work. We don't mention the available labels in the docs so I'm not sure if we need to update the docs.
if request_type == DataRequest and len(requests) > 1: | ||
# TODO: This section of the code has no observability |
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.
Maybe @samsja can answer why it was left out in the first implementation.
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.
@Jackmin801 what do u think is missing ?
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.
an issue has been opened
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.
@samsja, may u assist on whether docs need to be changed? |
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.
Really good work. I added some comments
Co-authored-by: samsja <55492238+samsja@users.noreply.github.com>
No need to add them |
Goals: