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

fix(instrumentation-aws-sdk): Add traceparent to AWS SDK unsignableHeaders #1813

Conversation

martinkuba
Copy link
Contributor

Which problem is this PR solving?

This is an attempt to address the issue of a signature mismatch when a request fails and is retried. This happens because the instrumentation adds the traceparent header to outgoing headers. When a request fails, the value of the header changes, but the signature does not; and this results in an error.

Fixes #1609 and open-telemetry/opentelemetry-js#3922

Short description of the changes

This change adds the traceparent header to the list of unsignable headers, so that it is excluded from creating the signature.

@martinkuba martinkuba requested a review from a team as a code owner November 18, 2023 01:39
Copy link

codecov bot commented Nov 18, 2023

Codecov Report

Merging #1813 (deedbcf) into main (5c1050b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1813   +/-   ##
=======================================
  Coverage   91.42%   91.43%           
=======================================
  Files         143      143           
  Lines        7303     7309    +6     
  Branches     1461     1465    +4     
=======================================
+ Hits         6677     6683    +6     
  Misses        626      626           
Files Coverage Δ
...entelemetry-instrumentation-aws-sdk/src/aws-sdk.ts 97.55% <100.00%> (+0.06%) ⬆️

@pichlermarc pichlermarc added bug Something isn't working pkg:instrumentation-aws-sdk priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies labels Nov 22, 2023
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I like this solution.
@blumamir would you mind also taking a look to ensure I'm not missing anything (I'm not too familiar with the aws sdk) 🙂

@trentm
Copy link
Contributor

trentm commented Nov 25, 2023

I mentioned a possible alternative fix at #1609 (comment)

@trentm
Copy link
Contributor

trentm commented Feb 7, 2024

I think this can be closed now that #1609 is in.

Update: The alternative fix was open-telemetry/opentelemetry-js#4346

@trentm trentm closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:instrumentation-aws-sdk priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS S3 behavior changes when using the auto instrumentation
4 participants