core: Migrate to new OpenCensus method & status tags#5996
Merged
zhangkun83 merged 4 commits intogrpc:masterfrom Aug 1, 2019
Merged
core: Migrate to new OpenCensus method & status tags#5996zhangkun83 merged 4 commits intogrpc:masterfrom
zhangkun83 merged 4 commits intogrpc:masterfrom
Conversation
Fixes grpc#5593 and supersedes grpc#5601 Now that census-instrumentation/opencensus-java#1854 has been merged & released as 0.21.0. We can start using the method & status tags. Background: Opencensus introduced new tags for status and method (census-instrumentation/opencensus-java#1115). The old views that used those tags were deprecated and new views were created that used the new tags. However grpc-java wasn't updated to use the new tags due to concern of breaking existing metrics. This resulted in the old views being deprecated while the new views were broken (goomics grpc#50). census-instrumentation/opencensus-java#1854 added a compatibility layer to opencensus that would remap new tags to old tags for old views. This should unblock grpc to switching to the new tags while allowing old views to still be populated. That commit was released as part of opencensus 0.21, which grpc currently uses
Contributor
Author
zhangkun83
reviewed
Jul 22, 2019
Contributor
zhangkun83
left a comment
zhangkun83
left a comment
There was a problem hiding this comment.
LGTM except for a minor comment.
@songy23, can you confirm it's safe to switch to the new tags for both OSS and internal usages?
Contributor
Yes it should be safe to do the switch. The new tags will be mapped properly to keep backwards-compatibility. |
Contributor
Author
|
Thanks for the review, all feedback should be addressed. I'll be OOO for the rest of the week, so if anything else needs to change feel free to update the PR as needed |
zhangkun83
approved these changes
Jul 23, 2019
songy23
approved these changes
Jul 23, 2019
Contributor
songy23
left a comment
songy23
left a comment
There was a problem hiding this comment.
Some minor nits, otherwise LGTM. Thanks for making this change!
Contributor
Author
|
I think this is ready to be merged |
Contributor
|
@zhangkun83 Can you add the label to trigger Kokoro? |
dapengzhang0
added
the
kokoro:run
grpc-kokoro
removed
the
kokoro:run
Contributor
Author
|
Thank you |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5593 and supersedes #5601
Now that census-instrumentation/opencensus-java#1854 has been merged & released as 0.21.0. We can start using the method & status tags.
Background:
Opencensus introduced new tags for status and method (census-instrumentation/opencensus-java#1115). The old views that used those tags were deprecated and new views were created that used the new tags. However grpc-java wasn't updated to use the new tags due to concern of breaking existing metrics. This resulted in the old views being deprecated while the new views were broken (goomics #50).
census-instrumentation/opencensus-java#1854 added a compatibility layer to opencensus that would remap new tags to old tags for old views. This should unblock grpc to switching to the new tags while allowing old views to still be populated. That commit was released as part of opencensus 0.21, which grpc currently uses.