feat(Instrumentation/Messenger): implement worker and middleware instrumentation#173
Open
jvocampings wants to merge 27 commits intoFriendsOfOpenTelemetry:mainfrom
Open
feat(Instrumentation/Messenger): implement worker and middleware instrumentation#173jvocampings wants to merge 27 commits intoFriendsOfOpenTelemetry:mainfrom
jvocampings wants to merge 27 commits intoFriendsOfOpenTelemetry:mainfrom
Conversation
gaelreyrol
reviewed
Apr 18, 2025
Contributor
Thanks again for your preliminary work on this, it's not an easy part and it will take some time to get the right implementation! This PR might also close this issue #168. |
…enger-instrumentation
jvocampings
changed the title
Author
Ah I was talking about Instrumentation and not Propagation. |
…n on error and on message handled
Author
|
@gaelreyrol I made some modifications related to what you asked. I'll be on holidays for 2 weeks. After that there are big changes at my work then I don't know if I'll be able to reactive. I'll do my best. |
Contributor
It can't be done from the event and the
Thank you very much, don't worry at all, have a good holiday! I might work on your PR directly if I have enough time. |
gaelreyrol
reviewed
Apr 18, 2025
This was referenced Apr 10, 2026
…n on error and on message handled
Fix several issues in the WorkerMessageEventSubscriber introduced by PR FriendsOfOpenTelemetry#173: - Replace SDK Span import with API Span to respect API/SDK separation - Implement InstrumentationTypeInterface for consistency with other subscribers - Add event priorities (10000/-10000) to wrap all other processing - Add messaging semantic convention attributes (operation.type, destination.name) - Include message class name in span name for better trace readability - Remove stale imports and duplicate propagation middleware service definition - Clean up propagation middleware when messenger tracing is disabled - Add PHPStan baseline entries for untyped $carrier interface params - Add functional tests for worker message handled, failed, and attribute mode - Reorganize messenger tests into Messenger/ subdirectory - Disable retry on test transport to isolate worker span assertions
…leware Cover TraceableMessengerTransport (get/ack/reject spans + TransportException error recording) and AddStampForPropagationMiddleware (stamp skip, no-scope passthrough, active-scope injection).
gaelreyrol
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #173 +/- ##
============================================
+ Coverage 91.21% 92.32% +1.11%
- Complexity 692 741 +49
============================================
Files 116 121 +5
Lines 2789 2971 +182
============================================
+ Hits 2544 2743 +199
+ Misses 245 228 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Align WorkerMessageEventSubscriber.endSpanOnError() with every other subscriber in the codebase (HttpKernel, Console, Doctrine, Mailer, TransportTracer) by using $span->recordException() which creates a proper OTel exception event with exception.type, exception.message, and exception.stacktrace per the OTel semantic conventions. Update tests to assert on span events rather than span attributes.
When Symfony Messenger handlers fail, the exception is wrapped in a HandlerFailedException (or other WrappedExceptionsInterface implementors like DelayedMessageHandlingException). Without unwrapping, the span records the generic wrapper message instead of the actual root causes. Now each nested exception is recorded as a separate span event via recordException(), matching how Sentry handles these wrapped exceptions.
Replace the hardcoded $notSupportedCommands array and isNotSupported() method with a default value on the existing exclude_commands config node. messenger:consume is now the default entry in console.tracing.exclude_commands, following the same pattern as Sentry's configurable excluded_commands. Users can override the list or clear it entirely via configuration.
Cover all branches of set(), get(), and keys() methods: - Valid envelope carrier for set/get/keys - Invalid carrier throws InvalidArgumentException - Non-traceparent key is ignored (set) or returns null (get) - Missing TraceStamp returns null from get() Closes the 34.78% patch coverage gap reported by Codecov.
The W3C Trace Context spec defines two headers: traceparent and tracestate. Previously only traceparent was propagated, silently dropping vendor-specific trace context (Datadog, AWS X-Ray, etc.). Add optional traceState property to TraceStamp and update TraceStampPropagator to handle both keys in set(), get(), and keys(). When tracestate arrives after traceparent, the existing stamp is replaced with one carrying both values.
….name bus.name is not a standard OTel semantic convention. Rename to symfony.messenger.bus.name to follow the existing codebase pattern (symfony.console.*, symfony.kernel.*). Only changes the worker subscriber attribute; the pre-existing bus.name in TraceableMessengerStack is left unchanged (out of scope).
If startSpan fires but neither endSpanWithSuccess nor endSpanOnError fires (e.g. worker killed, unhandled error in another subscriber), the OTel context scope leaks into subsequent messages. Now startSpan checks for a lingering scope at the beginning of each message and cleans it up: detaches the scope, marks the orphaned span as ERROR, and ends it. This prevents context pollution across messages.
…namespace span attributes - Namespace middleware span attributes (bus.name, event.category, event.current) under symfony.messenger.* prefix for consistency with worker subscriber - Add symfony.messenger.will_retry and symfony.messenger.retry_count attributes to failure spans so operators can distinguish retriable from terminal failures - Support #[Traceable] attribute on message classes for attribute-based instrumentation mode, with custom tracer selection via TracerLocatorPass
…erage Mark all new types as final for consistency with existing codebase. Register TraceStampPropagator as a shared DI service instead of inline construction. Default instrumentationType to Auto, add safe tracer locator fallback, nullable logger consistency, debug logging on skipped messages and null scopes, and remove dead commented-out code. Add end-to-end propagation test, WorkerMessageEventSubscriber unit tests, and PropagatorFactory test.
Symfony's getHeaders() normalizes header names to lowercase, so the Content-Length check was unreachable. Access the first array element since headers are returned as arrays.
…leware spans TraceableMessengerStack now tracks its own scope, span, and parent context instead of probing global context storage — fixing scope leaks where earlier middleware spans were never ended. TraceableMessengerMiddleware catches exceptions and records them with STATUS_ERROR on the active span, aligning with every other instrumentation in the bundle.
TraceableMessengerTransportFactory was tagged `kernel.reset` with
method `reset`, but the class has no such method. Symfony's
ServicesResetter invokes the declared method on every tagged service
between worker iterations, so `messenger:consume` threw as soon as a
retry (or any post-handler reset) ran:
Attempted to call an undefined method named "reset" of class
"...\TraceableMessengerTransportFactory".
The tag was introduced with the original messenger tracing (6f3c032)
and never had a corresponding method — likely speculative, possibly
confused with messenger transports that sometimes implement
ResetInterface.
Removal is preferred over adding an empty reset(): the factory's
dependencies (inner TransportFactory, TracerInterface, LoggerInterface)
are immutable DI services, createTransport() returns a fresh
TraceableMessengerTransport without retaining a reference, and
supports() is pure — there is no per-message state to clear. An empty
reset() would silence the error but imply a ResetInterface-style
contract the class does not fulfill and mislead anyone later adding
mutable state.
Contributor
|
Hi @jvocampings — quick update on where the PR stands. cc @kochen (ref #190) — this should cover your use case too; would love your feedback. Review feedback addressed
Middleware & propagation
Attributes, config, tests
@jvocampings — could you give the branch a spin on your setup to confirm nothing regressed vs. your original intent? And @kochen, if you're able to test against your PDF-generator flow, that would be a great real-world validation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP.
Related to issue #172
Encapsulate consumption of message in a Span. Based on Messenger events.
Missing instrumentation when a message is dispatched. Not sure how to implement it.
edit: I assume we could reuse TraceableMessengerTransport to decorate the method
send.