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

Use default context if unset for builders #401

Merged
merged 1 commit into from Dec 28, 2020

Conversation

jtescher
Copy link
Member

The previous change to the builder actually introduced an issue if a builder was used without explicitly specifying its parent context. This is resolved here by defaulting to the current context if unset.

If you want to create a new root span, you can still explicitly set the builder's parent context to be a new context.

@jtescher jtescher requested a review from a team as a code owner December 28, 2020 01:58
@codecov
Copy link

codecov bot commented Dec 28, 2020

Codecov Report

Merging #401 (fad9603) into master (601d297) will increase coverage by 0.21%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   49.12%   49.34%   +0.21%     
==========================================
  Files          66       66              
  Lines        5400     5409       +9     
==========================================
+ Hits         2653     2669      +16     
+ Misses       2747     2740       -7     
Impacted Files Coverage Δ
opentelemetry/src/sdk/trace/tracer.rs 74.86% <84.61%> (+4.91%) ⬆️
opentelemetry/src/api/trace/tracer.rs 37.83% <0.00%> (+2.62%) ⬆️

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 601d297...fad9603. Read the comment docs.

@jtescher
Copy link
Member Author

@TommyCpp ran through all the examples, and some external crates like tracing-opentelemetry and actix-web-opentelemetry and this was the only issue I found, so should be good to release after this fix 😅

Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

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

Sorry didn't get a chance to look at it earlier. LGTM 👍. Good catch!

@jtescher jtescher merged commit cf2b471 into master Dec 28, 2020
@jtescher jtescher deleted the fix-builder-default-context branch December 28, 2020 23:25
jan-xyz added a commit to jan-xyz/opentelemetry-rust that referenced this pull request Oct 1, 2021
0.13.0

Bug fix: Avoid panics from Instant::elapsed (open-telemetry#406)

Improvement: Allow trailing comma on macros (open-telemetry#390)

Improvement: Add macros for custom registry (open-telemetry#396)

Improvement: Export thread count from process_collector (open-telemetry#401)

Improvement: Add convenience TextEncoder functions to encode directly to string (open-telemetry#402)

Internal change: Clean up the use of macro_use and extern crate (open-telemetry#398)

Internal change: Update dependencies
TommyCpp added a commit that referenced this pull request Oct 5, 2021
* bump prometheus dependency to 0.13

0.13.0

Bug fix: Avoid panics from Instant::elapsed (#406)

Improvement: Allow trailing comma on macros (#390)

Improvement: Add macros for custom registry (#396)

Improvement: Export thread count from process_collector (#401)

Improvement: Add convenience TextEncoder functions to encode directly to string (#402)

Internal change: Clean up the use of macro_use and extern crate (#398)

Internal change: Update dependencies

* also update in example

Co-authored-by: Zhongyang Wu <zhongyang.wu@outlook.com>
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.

None yet

2 participants