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

return no-op span builder after shutdown #765

Merged
merged 2 commits into from
Jul 17, 2022
Merged

return no-op span builder after shutdown #765

merged 2 commits into from
Jul 17, 2022

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented Jul 14, 2022

this completes a todo in code comments: when tracer shared state has shutdown, getting a span builder should return a no-op implementation

this completes a todo in code comments: when tracer shared state has shutdown, getting a span builder should return a no-op implementation
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #765 (93728ad) into main (f33b623) will decrease coverage by 4.34%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #765      +/-   ##
============================================
- Coverage     82.86%   78.51%   -4.35%     
- Complexity     1295     1337      +42     
============================================
  Files           150      154       +4     
  Lines          3192     3291      +99     
============================================
- Hits           2645     2584      -61     
- Misses          547      707     +160     
Flag Coverage Δ
7.4 78.51% <100.00%> (-4.36%) ⬇️
8.0 78.59% <100.00%> (-4.34%) ⬇️
8.1 78.59% <100.00%> (-4.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SDK/Trace/Tracer.php 100.00% <100.00%> (ø)
src/SDK/Trace/Span.php 12.50% <0.00%> (-81.74%) ⬇️
src/SDK/Trace/SpanLimits.php 0.00% <0.00%> (-75.00%) ⬇️
src/API/Trace/AbstractSpan.php 0.00% <0.00%> (-38.89%) ⬇️
src/SDK/Trace/Behavior/HttpSpanExporterTrait.php 93.47% <0.00%> (-1.88%) ⬇️
src/SDK/Trace/ExporterFactory.php 100.00% <0.00%> (ø)
src/SDK/Trace/SpanExporter/InMemoryExporter.php 100.00% <0.00%> (ø)
src/SDK/Common/Util/ShutdownHandler.php 63.63% <0.00%> (ø)
src/SDK/Common/Util/WeakMap.php 0.00% <0.00%> (ø)
src/SDK/Common/Util/functions.php 100.00% <0.00%> (ø)
... and 1 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 f33b623...93728ad. Read the comment docs.

@brettmc
Copy link
Collaborator Author

brettmc commented Jul 14, 2022

NB that I needed to adjust a bunch of tests, as tracer shared state was immediately shutting down due to their not maintaining a reference to the tracer provider. Should be fixed by #760

@tidal
Copy link
Member

tidal commented Jul 17, 2022

There is a huge drop in code-coverage, which is unrelated. However SpanTest has only a @coversDefaultClass annotation, which is never used, so all those tests are completely ignored by code-coverage.

@tidal tidal merged commit ea5b7c1 into open-telemetry:main Jul 17, 2022
@brettmc brettmc deleted the noop-span-builder-after-shutdown branch October 25, 2022 06:57
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