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(aws-sdk): add http status code attribute to aws sdk span if aws sdk v3 client exception occurs #2344

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

jj22ee
Copy link
Contributor

@jj22ee jj22ee commented Jul 18, 2024

Which problem is this PR solving?

Fixes #2343

  • Upon AWS SDK (v3) Client exceptions, AWS SDK V3 Span will not record http.status.code attribute into the span.

Short description of the changes

  • Record http.status.code attribute into the span upon client exceptions for AWS SDK V3 Spans.

Testing

  • Updated Unit Tests to check for http.status.code attribute

Copy link

@vastin vastin left a comment

Choose a reason for hiding this comment

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

LGTM

@jj22ee
Copy link
Contributor Author

jj22ee commented Jul 19, 2024

Pinging @blumamir for approval and merge.

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.40%. Comparing base (dfb2dff) to head (5eef8ee).
Report is 191 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2344      +/-   ##
==========================================
- Coverage   90.97%   90.40%   -0.57%     
==========================================
  Files         146      149       +3     
  Lines        7492     7370     -122     
  Branches     1502     1532      +30     
==========================================
- Hits         6816     6663     -153     
- Misses        676      707      +31     
Files Coverage Δ
...entelemetry-instrumentation-aws-sdk/src/aws-sdk.ts 95.08% <100.00%> (-0.04%) ⬇️

... and 62 files with indirect coverage changes

@blumamir
Copy link
Member

Thanks you @jj22ee for the contribution.
Please fix the linting so I can merge.

npm run lint:fix

@jj22ee
Copy link
Contributor Author

jj22ee commented Jul 22, 2024

@blumamir done!

@blumamir blumamir merged commit 9a06381 into open-telemetry:main Jul 23, 2024
19 checks passed
@dyladan dyladan mentioned this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants