feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics #214
Conversation
This file contains 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
googlebot
added
the
cla: yes
igorbernstein2
force-pushed
the
metrics-rewrite
branch
from
1e414f4
to
0afccbb
Compare
Codecov Report
@@ Coverage Diff @@
## master #214 +/- ##
============================================
+ Coverage 79.89% 80.69% +0.79%
- Complexity 992 1049 +57
============================================
Files 99 99
Lines 6447 6541 +94
Branches 340 344 +4
============================================
+ Hits 5151 5278 +127
+ Misses 1118 1082 -36
- Partials 178 181 +3 Continue to review full report at Codecov.
|
igorbernstein2
force-pushed
the
metrics-rewrite
branch
2 times, most recently
from
cd9d8a5
to
c383131
Compare
igorbernstein2
changed the title
igorbernstein2
changed the title
igorbernstein2
force-pushed
the
metrics-rewrite
branch
from
c383131
to
b0d92dd
Compare
…rpc level metrics This PR refactors opencensus metrics integration to use gax's ApiTracers. Which allows this client to instrument individual attempts and tag everything with bigtable resource ids
igorbernstein2
force-pushed
the
metrics-rewrite
branch
from
b0d92dd
to
7f5df26
Compare
kolea2
reviewed
Apr 2, 2020
...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java
Outdated
Show resolved
Hide resolved
...oud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java
Outdated
Show resolved
Hide resolved
kolea2
approved these changes
Apr 9, 2020
...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java
Show resolved
Hide resolved
kolea2
previously requested changes
Apr 9, 2020
There was a problem hiding this comment.
To get clirr to pass, please add a clirr-ignored-differences.xml file. Example: https://github.com/googleapis/java-trace/blob/master/grpc-google-cloud-trace-v1/clirr-ignored-differences.xml
igorbernstein2
commented
May 13, 2020
|
Adding do not merge - want to go through the merge conflicts that were resolved to ensure I didn't miss anything in the process. |
kolea2
added
the
do not merge
kolea2
reviewed
May 14, 2020
...bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java
Show resolved
Hide resolved
igorbernstein2
commented
May 14, 2020
google-cloud-bigtable/pom.xml
Outdated
| @@ -124,6 +124,10 @@ | |||
| <groupId>io.opencensus</groupId> | |||
| <artifactId>opencensus-api</artifactId> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>io.opencensus</groupId> | |||
| <artifactId>opencensus-impl</artifactId> | |||
There was a problem hiding this comment.
Sorry for being confusing before, but opencensus-impl should stay as a test dep. I just meant that impl-core should be transitive. As it stands endusers should add -impl to activate opencensus (I just wanted to avoid users having to specify both
kolea2
reviewed
May 14, 2020
...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java
Show resolved
Hide resolved
...table/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java
Show resolved
Hide resolved
|
I think this looks good, wanna take it for spin and merge it? |
|
asking @chingor13 for a extra look |
chingor13
approved these changes
May 14, 2020
kolea2
removed
the
do not merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR refactors opencensus metrics integration to use gax's ApiTracers.
Which allows this client to instrument individual attempts and tag everything
with bigtable resource ids
Note: clirr is complaining about removed classes, but they are all marked as InternalApi