feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics by igorbernstein2 · Pull Request #214 · googleapis/java-bigtable

igorbernstein2 · 2020-03-05T23:53:39Z

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

codecov · 2020-03-18T23:15:06Z

Codecov Report

Merging #214 into master will increase coverage by 0.79%.
The diff coverage is 92.27%.

@@             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

Impacted Files	Coverage Δ	Complexity Δ
...ble/data/v2/stub/EnhancedBigtableStubSettings.java	`95.04% <ø> (-0.11%)`	`20.00 <0.00> (ø)`
...d/bigtable/data/v2/stub/metrics/MetricsTracer.java	`78.48% <78.48%> (ø)`	`14.00 <14.00> (?)`
...able/data/v2/stub/metrics/RpcMeasureConstants.java	`92.85% <92.85%> (+1.94%)`	`1.00 <1.00> (ø)`
...ud/bigtable/data/v2/stub/EnhancedBigtableStub.java	`95.74% <100.00%> (+0.47%)`	`21.00 <0.00> (ø)`
...bigtable/data/v2/stub/metrics/CompositeTracer.java	`100.00% <100.00%> (ø)`	`33.00 <33.00> (?)`
...e/data/v2/stub/metrics/CompositeTracerFactory.java	`100.00% <100.00%> (ø)`	`3.00 <3.00> (?)`
...ble/data/v2/stub/metrics/MetricsTracerFactory.java	`100.00% <100.00%> (ø)`	`3.00 <3.00> (?)`
...igtable/data/v2/stub/metrics/RpcViewConstants.java	`97.67% <100.00%> (+97.67%)`	`1.00 <0.00> (+1.00)`
.../cloud/bigtable/data/v2/stub/metrics/RpcViews.java	`66.66% <100.00%> (+66.66%)`	`3.00 <1.00> (+3.00)`
...le/cloud/bigtable/data/v2/models/BulkMutation.java	`97.56% <0.00%> (-2.44%)`	`10.00% <0.00%> (ø%)`
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc215c5...787bc59. Read the comment docs.

…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

...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java

...oud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java

kolea2

LGTM, though I think this needs a rebase and code format fixes

...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java

kolea2

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

…rics-rewrite

google-cloud-bigtable/pom.xml

kolea2 · 2020-05-13T17:40:06Z

Adding do not merge - want to go through the merge conflicts that were resolved to ensure I didn't miss anything in the process.

...bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java

igorbernstein2 · 2020-05-14T14:27:30Z

google-cloud-bigtable/pom.xml

    </dependency>
+    <dependency>
+      <groupId>io.opencensus</groupId>
+      <artifactId>opencensus-impl</artifactId>


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

...loud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java

...table/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java

igorbernstein2 · 2020-05-14T19:36:56Z

I think this looks good, wanna take it for spin and merge it?

kolea2 · 2020-05-14T19:53:34Z

asking @chingor13 for a extra look

stale

chingor13

LGTM

Is the CompositeTracer something we should look to upstream into gax that would be useful to others?

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 5, 2020

kolea2 added the release-please:force-run To run release-please label Mar 16, 2020

release-please bot removed the release-please:force-run To run release-please label Mar 16, 2020

igorbernstein2 force-pushed the metrics-rewrite branch from 1e414f4 to 0afccbb Compare March 18, 2020 23:15

igorbernstein2 force-pushed the metrics-rewrite branch 2 times, most recently from cd9d8a5 to c383131 Compare April 1, 2020 23:14

igorbernstein2 changed the title ~~Metrics rewrite~~ feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics Apr 1, 2020

igorbernstein2 changed the title ~~feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics~~ feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics Apr 1, 2020

igorbernstein2 marked this pull request as ready for review April 1, 2020 23:16

igorbernstein2 requested a review from kolea2 April 1, 2020 23:16

igorbernstein2 force-pushed the metrics-rewrite branch from c383131 to b0d92dd Compare April 1, 2020 23:17

feat: Update opencensus metrics to include bigtable resource ids and …

7f5df26

…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 April 2, 2020 14:46

fix deps

78fcc4d

kolea2 reviewed Apr 2, 2020

View reviewed changes

...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

igorbernstein2 added 2 commits April 2, 2020 11:22

typo

1a109a4

remove unused param

b39dc5a

kolea2 approved these changes Apr 9, 2020

View reviewed changes

...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

View reviewed changes

kolea2 added 2 commits May 13, 2020 11:51

Merge branch 'master' of github.com:googleapis/java-bigtable into met…

71ea401

…rics-rewrite

add clirr

8153020

igorbernstein2 commented May 13, 2020

View reviewed changes

google-cloud-bigtable/pom.xml Show resolved Hide resolved

kolea2 added 2 commits May 13, 2020 13:38

review feedback

94a0141

code formatting

f8779ae

kolea2 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 13, 2020

kolea2 reviewed May 14, 2020

View reviewed changes

...bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java Show resolved Hide resolved

igorbernstein2 commented May 14, 2020

View reviewed changes

fix deps

787bc59

kolea2 reviewed May 14, 2020

View reviewed changes

...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

chingor13 approved these changes May 14, 2020

View reviewed changes

kolea2 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 18, 2020

kolea2 merged commit 7214ef6 into googleapis:master May 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics#214

feat: Update opencensus metrics to include bigtable resource ids and rpc level metrics#214
kolea2 merged 9 commits intogoogleapis:masterfrom
igorbernstein2:metrics-rewrite

igorbernstein2 commented Mar 5, 2020 •

edited

Loading

Uh oh!

codecov bot commented Mar 18, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

kolea2 left a comment

Uh oh!

Uh oh!

kolea2 left a comment

Uh oh!

Uh oh!

kolea2 commented May 13, 2020

Uh oh!

Uh oh!

igorbernstein2 May 14, 2020

Uh oh!

Uh oh!

Uh oh!

igorbernstein2 commented May 14, 2020

Uh oh!

kolea2 commented May 14, 2020

Uh oh!

chingor13 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

igorbernstein2 commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

kolea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kolea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kolea2 commented May 13, 2020

Uh oh!

Uh oh!

igorbernstein2 May 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

igorbernstein2 commented May 14, 2020

Uh oh!

kolea2 commented May 14, 2020

Uh oh!

chingor13 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

igorbernstein2 commented Mar 5, 2020 •

edited

Loading

codecov bot commented Mar 18, 2020 •

edited

Loading