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

Deprecated measures use instance of new measures when 1to1 mapping.#1738

Merged
bogdandrutu merged 5 commits intocensus-instrumentation:masterfrom
bogdandrutu:grpcmeasure
Feb 6, 2019
Merged

Deprecated measures use instance of new measures when 1to1 mapping.#1738
bogdandrutu merged 5 commits intocensus-instrumentation:masterfrom
bogdandrutu:grpcmeasure

Conversation

@bogdandrutu
Copy link
Copy Markdown
Contributor

@bogdandrutu bogdandrutu commented Feb 6, 2019

Intentionally split this into 2 commits to make it easy to see the changes, first commit just moves the code of deprecated measures after the new measures, second commit actually does the main change.

This fixes an issue with registerAllGrpcBasicViews because we had 2 measures (rountrip_latency) with different descriptions and this will fail here https://github.com/census-instrumentation/opencensus-java/blob/master/impl_core/src/main/java/io/opencensus/implcore/stats/MeasureToViewMap.java#L115

This is a breaking change in regard to the measure name/description for the old measures, but nobody should depend on that.

@bogdandrutu
Copy link
Copy Markdown
Contributor Author

To see what changed look at this babf3d3 or the second commit.

Copy link
Copy Markdown
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Please run Java formatter.

@bogdandrutu
Copy link
Copy Markdown
Contributor Author

One more change - removed the old views from the grpc basic views. This method never worked because of the duplicated measure name so this is not a problem.


@VisibleForTesting
static final ImmutableSet<View> RPC_REAL_TIME_METRICS_VIEWS_SET =
static final ImmutableSet<View> GRPC_REAL_TIME_METRICS_VIEWS_SET =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fix the tests.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 6, 2019

Codecov Report

Merging #1738 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1738      +/-   ##
============================================
- Coverage     82.12%   82.07%   -0.05%     
  Complexity     1950     1950              
============================================
  Files           288      288              
  Lines          9061     9048      -13     
  Branches        862      862              
============================================
- Hits           7441     7426      -15     
- Misses         1329     1331       +2     
  Partials        291      291
Impacted Files Coverage Δ Complexity Δ
...a/io/opencensus/contrib/grpc/metrics/RpcViews.java 71.83% <100%> (ø) 18 <2> (ø) ⬇️
...nsus/contrib/grpc/metrics/RpcMeasureConstants.java 100% <100%> (ø) 1 <0> (ø) ⬇️
...e/ocagent/OcAgentTraceServiceConfigRpcHandler.java 75.86% <0%> (-3.45%) 12% <0%> (ø)
...n/java/io/opencensus/implcore/tags/TagMapImpl.java 100% <0%> (ø) 6% <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 1652c73...6cf004c. Read the comment docs.

@bogdandrutu bogdandrutu merged commit 5d85e9e into census-instrumentation:master Feb 6, 2019
@bogdandrutu bogdandrutu deleted the grpcmeasure branch February 6, 2019 17:47
songy23 pushed a commit to songy23/instrumentation-java that referenced this pull request Feb 6, 2019
…ensus-instrumentation#1738)

* Move deprecated measures after non-deprecated once.

* Deprecated measures use instance of new measures when 1to1 mapping.

* Run java format.

* Remove installation of the old views in grpc basic view.

* Fix RpcViewTests.

(cherry picked from commit 5d85e9e)
bogdandrutu pushed a commit that referenced this pull request Feb 6, 2019
…1738) (#1740)

* Move deprecated measures after non-deprecated once.

* Deprecated measures use instance of new measures when 1to1 mapping.

* Run java format.

* Remove installation of the old views in grpc basic view.

* Fix RpcViewTests.

(cherry picked from commit 5d85e9e)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants