Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid usage of getSpanWithoutDefault when possible. #1690

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

bogdandrutu
Copy link
Member

We check for isValid everywhere so this will not be a performance hit, explicitly because the DefaultSpan.getInvalid().getContext().isValid() will be memoized.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@codecov
Copy link

codecov bot commented Sep 23, 2020

Codecov Report

Merging #1690 into master will increase coverage by 0.10%.
The diff coverage is 73.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1690      +/-   ##
============================================
+ Coverage     85.36%   85.46%   +0.10%     
+ Complexity     1385     1383       -2     
============================================
  Files           164      164              
  Lines          5445     5435      -10     
  Branches        562      560       -2     
============================================
- Hits           4648     4645       -3     
+ Misses          590      587       -3     
+ Partials        207      203       -4     
Impacted Files Coverage Δ Complexity Δ
...xt/src/main/java/io/opentelemetry/Application.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ntelemetry/trace/propagation/HttpTraceContext.java 96.03% <100.00%> (-0.08%) 27.00 <0.00> (-2.00)
...opagation/B3PropagatorInjectorMultipleHeaders.java 100.00% <100.00%> (+8.33%) 4.00 <0.00> (ø)
.../propagation/B3PropagatorInjectorSingleHeader.java 100.00% <100.00%> (+4.34%) 5.00 <0.00> (ø)
...extensions/trace/propagation/JaegerPropagator.java 87.50% <100.00%> (+1.08%) 26.00 <0.00> (ø)
...tensions/trace/propagation/OtTracerPropagator.java 82.75% <100.00%> (+4.63%) 10.00 <0.00> (ø)
...ava/io/opentelemetry/sdk/trace/SpanBuilderSdk.java 95.79% <100.00%> (ø) 39.00 <0.00> (ø)

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 d9bd619...57b1ce9. Read the comment docs.

@jkwatson
Copy link
Contributor

It looks like there's only one usage left after this. Can we get rid of that one, as well, and get rid of the method altogether?

@bogdandrutu
Copy link
Member Author

@jkwatson you read my mind, but that one usage left is a bit different. I will followup on that soon.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Was looking forward to this, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants