Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Add started_rpcs measures and views to RPC constants.#1230

Merged
songy23 merged 4 commits intocensus-instrumentation:masterfrom
songy23:rpc-constants
Jun 5, 2018
Merged

Add started_rpcs measures and views to RPC constants.#1230
songy23 merged 4 commits intocensus-instrumentation:masterfrom
songy23:rpc-constants

Conversation

@songy23
Copy link
Copy Markdown
Contributor

@songy23 songy23 commented Jun 4, 2018

Related to census-instrumentation/opencensus-specs#120.

Client/server started_rpcs are required by gRPC, add them back to RPC constants.

/cc @zhangkun83

@songy23 songy23 added this to the Release 0.14.0 milestone Jun 4, 2018
@songy23 songy23 requested review from bogdandrutu and sebright2 June 4, 2018 17:24
@songy23 songy23 added the kokoro:run Label to trigger a build for PR label Jun 4, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 4, 2018 •
edited
Loading

Codecov Report

Merging #1230 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1230      +/-   ##
============================================
+ Coverage     82.02%   82.06%   +0.03%     
  Complexity     1241     1241              
============================================
  Files           193      193              
  Lines          6032     6044      +12     
  Branches        560      560              
============================================
+ Hits           4948     4960      +12     
  Misses          935      935              
  Partials        149      149
Impacted Files Coverage Δ Complexity Δ
...a/io/opencensus/contrib/grpc/metrics/RpcViews.java 72.41% <ø> (ø) 8 <0> (ø) ⬇️
...nsus/contrib/grpc/metrics/RpcMeasureConstants.java 100% <100%> (ø) 1 <0> (ø) ⬇️
...ncensus/contrib/grpc/metrics/RpcViewConstants.java 100% <100%> (ø) 1 <0> (ø) ⬇️

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 ff7a991...006ae0b. Read the comment docs.

@kokoro-team kokoro-team removed the kokoro:run Label to trigger a build for PR label Jun 4, 2018
@sebright2
Copy link
Copy Markdown
Contributor

sebright2 commented Jun 4, 2018

I think we should wait to merge this until census-instrumentation/opencensus-specs#120 is merged.

@songy23
Copy link
Copy Markdown
Contributor Author

songy23 commented Jun 4, 2018

Sure, holding off this for now.

Also I would like to include this change in v0.14.0 release so that I can update grpc/grpc-java#4494.

@songy23 songy23 mentioned this pull request Jun 4, 2018
Merged
@songy23
Copy link
Copy Markdown
Contributor Author

songy23 commented Jun 4, 2018

PTAL - I submitted census-instrumentation/opencensus-specs#120 to specs. Will do a minor release after merging this PR.

@songy23 songy23 assigned songy23 and unassigned bogdandrutu Jun 5, 2018
sebright2
sebright2 reviewed Jun 5, 2018
View reviewed changes
Comment thread contrib/grpc_metrics/src/main/java/io/opencensus/contrib/grpc/metrics/RpcMeasureConstants.java Outdated
*
* @since 0.14
*/
public static final MeasureDouble GRPC_CLIENT_STARTED_RPCS =
Copy link
Copy Markdown
Contributor

@sebright2 sebright2 Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the two started_rpcs measures be MeasureLong?

Copy link
Copy Markdown
Contributor Author

@songy23 songy23 Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good catch.

@songy23
Copy link
Copy Markdown
Contributor Author

songy23 commented Jun 5, 2018

Thanks!

@songy23 songy23 merged commit 375907a into census-instrumentation:master Jun 5, 2018
@songy23 songy23 deleted the rpc-constants branch June 5, 2018 00:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@bogdandrutu bogdandrutu bogdandrutu approved these changes

@sebright2 sebright2 sebright2 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@songy23 songy23

Labels

API cla: yes gRPC

Projects

None yet

Milestone

Release 0.14.0

Development

Successfully merging this pull request may close these issues.

6 participants

@songy23 @codecov-io @sebright2 @bogdandrutu @googlebot @kokoro-team