-
Notifications
You must be signed in to change notification settings - Fork 544
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
feat: config option to extract sqs context from message payload #737
feat: config option to extract sqs context from message payload #737
Conversation
SNS context propagation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## main #737 +/- ##
==========================================
- Coverage 96.87% 96.37% -0.50%
==========================================
Files 11 26 +15
Lines 640 1767 +1127
Branches 126 231 +105
==========================================
+ Hits 620 1703 +1083
- Misses 20 64 +44
|
Is anyone else planning to review this? if not I'll merge today |
|
||
const responseMockSuccess = { | ||
requestId: '0000000000000', | ||
error: null, | ||
}; | ||
|
||
const extractContextSpy = sinon.spy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a spy on this scope effects all tests. You could move it into the relevant test/testsuite and instead of calling resetHistory()
you could create a new spy for each test and use sinon.restore()
after each test.
|
||
it('should extract from payload', async () => { | ||
const traceparent = { | ||
traceparent: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this object structure documented somewhere? How should it look like for other propagators like e.g. Baggage or B3?
Seems my review was too late :o) but there are anyway no blockers. But I wonder that support for |
Inject is always injecting the propagation headers into the message attributes at time of "Publish" \ "SendMessage" This PR is about support for extracting the context for the second option - taking them from message payload, which is why only extract is relevant |
Which problem is this PR solving?
Short description of the changes