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

Span processor exporter updates #439

Conversation

Blacksmoke16
Copy link
Contributor

@Blacksmoke16 Blacksmoke16 commented Oct 7, 2021

  • Update docs on various types/methods to point to related spec sections
  • Rename Exporter to SpanExporterInterface
  • Update various shutdown and forceFlush methods to return bool
    • Spec says they should provide a way to differentiate between pass/fail
    • The spec also says to support a timeout, but I'm not sure how you'd do that in PHP given it's single threaded and doesn't have async stuff like other langs
  • Implemented NoopTracer and NoopSpanBuilder
    • Is returned when you call #getTracer on a shutdown TracerProvider
  • Ensure #shutdown also calls #forceFlush as per the spec
  • Rename SpanMultiProcessor to MultiSpanProcessor to better match naming of other processors

TODO:

  • Want to update some more tests, so starting this off as a draft

Resolves #270

Rename `Exporter` to `SpanExporterInterface`
Update docs to include links to spec
Revamp `SimpleSpanProcessorTest`
@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

Merging #439 (9c6147c) into main (c8756f1) will decrease coverage by 1.69%.
The diff coverage is 54.23%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #439      +/-   ##
============================================
- Coverage     92.84%   91.14%   -1.70%     
- Complexity      781      804      +23     
============================================
  Files            74       76       +2     
  Lines          1929     1977      +48     
============================================
+ Hits           1791     1802      +11     
- Misses          138      175      +37     
Impacted Files Coverage Δ
src/SDK/Trace/NoopSpanBuilder.php 0.00% <0.00%> (ø)
src/SDK/Trace/NoopTracer.php 0.00% <0.00%> (ø)
src/SDK/Trace/SpanProcessor/NoopSpanProcessor.php 60.00% <0.00%> (-6.67%) ⬇️
src/SDK/Trace/TracerProvider.php 67.85% <12.50%> (-7.15%) ⬇️
src/SDK/Trace/SpanProcessor/MultiSpanProcessor.php 20.00% <16.66%> (ø)
src/SDK/Trace/TracerSharedState.php 92.85% <50.00%> (+3.20%) ⬆️
src/Contrib/ZipkinToNewrelic/Exporter.php 90.38% <75.00%> (+0.18%) ⬆️
...rc/SDK/Trace/SpanProcessor/SimpleSpanProcessor.php 94.11% <88.88%> (-5.89%) ⬇️
src/Contrib/OtlpHttp/Exporter.php 97.14% <90.00%> (+0.04%) ⬆️
src/SDK/Trace/SpanProcessor/BatchSpanProcessor.php 95.55% <90.90%> (-1.95%) ⬇️
... and 3 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 c8756f1...9c6147c. Read the comment docs.

@bobstrecansky bobstrecansky merged commit 8d61da2 into open-telemetry:main Oct 9, 2021
@Blacksmoke16 Blacksmoke16 deleted the span-processor-exporter-updates branch October 9, 2021 18:37
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.

Update SDK TracerProvider shutdown behavior to conform to spec
2 participants