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

Update the TracingKernelSubscriber example #76

Merged

Conversation

wadjei
Copy link
Contributor

@wadjei wadjei commented Aug 12, 2022

Since changes in open-telemetry/opentelemetry-php#760,
the original Kernel Listener example no longer works...

Following those changes, the stored Tracer instance gets deleted before the
application terminates so mainSpan can't be ended and none of the collected
trace info gets sent.
By refactoring to use TracerProvider instead, mainSpan's Tracer remains valid
during KernelEvents::TERMINATE so the trace session can be sent successfully.

Since changes in open-telemetry/opentelemetry-php#760,
the original Kernel Listener example no longer works...

Following those changes, the stored Tracer instance gets deleted before the
application terminates so mainSpan can't be ended and none of the collected
trace info gets sent.
By refactoring to use TracerProvider instead, mainSpan's Tracer remains valid
during KernelEvents::TERMINATE so the trace session can be sent successfully.
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #76 (3c8d9c1) into main (7b5c466) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main      #76   +/-   ##
=========================================
  Coverage     95.81%   95.81%           
  Complexity      182      182           
=========================================
  Files            18       18           
  Lines           454      454           
=========================================
  Hits            435      435           
  Misses           19       19           
Flag Coverage Δ
7.4 95.81% <ø> (ø)
8.0 95.81% <ø> (ø)
8.1 95.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out 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 7b5c466...3c8d9c1. Read the comment docs.

@Nevay
Copy link
Contributor

Nevay commented Aug 12, 2022

This should be fixed in the SDK bundle instead. TracerProvider::shutdown() should be called in Bundle::shutdown() and on exit (default Symfony HttpKernelRunner doesn't seem to call ::shutdown()).

Side note: There are some other problems with the example subscriber such as not detaching the created scope. I will see if I can create a PR over the weekend to fix the subscriber / add an actual implementation.

@brettmc brettmc merged commit 9bbdb16 into open-telemetry:main Aug 12, 2022
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