Bugfix/set default context in pika #719
Conversation
|
The patch looks fine but I'm now a bit confused about the overall design of the instrumentation. It looks like we are trying to extract span from properties.headers and then immediately after that are trying inject context again into the same thing. Generally we'd want to instrument two sets of functions. Functions that get/retrieve a message from some bus/queue (receivers) and functions that would send messages to a bus (senders). Generally, receivers should extract context from incoming messages, start a span and set it in the current context. The senders should just start a span and inject its context into the outgoing message. In this implementation it looks like we are extracting from and injecting into the same message. |
| @@ -41,15 +41,20 @@ def decorated_callback( | |||
| ) -> Any: | |||
There was a problem hiding this comment.
So _decorate_callback wraps the "receive" functionality only?
There was a problem hiding this comment.
Yes, but it wraps all the callbacks in the _consumers dict
|
Skipped changelog as this component has not been released yet so the first release should only contain one changelog entry for this. |
|
Docker tests are failing for unrelated reasons and need to be looked at separately. This PR has approvals and passes the relevant checks so merging it. |
Description
This PR fixes a bug in the pika instrumentation library. When the propagator could not detect a context from the message headers (for example in the sending action
properties.headers) the context would be set to{}instead of the current context - thus breaking the trace chain.Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.