opentelemetry-js-contrib icon indicating copy to clipboard operation
opentelemetry-js-contrib copied to clipboard

feat(aws-sdk): add exception hook

Open project0 opened this issue 1 year ago • 3 comments

Which problem is this PR solving?

  • https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1765

Short description of the changes

  • Adds a additional hook for aws sdk exceptions

project0 avatar Aug 22 '24 13:08 project0

Ping to component owners @jj22ee @blumamir

david-luna avatar Nov 25 '24 16:11 david-luna

Codecov Report

:x: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 89.84%. Comparing base (652dbf9) to head (3e7bd85). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/instrumentation-aws-sdk/src/aws-sdk.ts 90.00% 1 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2398   +/-   ##
=======================================
  Coverage   89.84%   89.84%           
=======================================
  Files         188      188           
  Lines        9272     9282   +10     
  Branches     1901     1903    +2     
=======================================
+ Hits         8330     8339    +9     
- Misses        942      943    +1     
Files with missing lines Coverage Δ
packages/instrumentation-aws-sdk/src/aws-sdk.ts 92.77% <90.00%> (-0.17%) :arrow_down:
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 05 '24 16:12 codecov[bot]

@blumamir @jj22ee @trivikr any chance we can move forward? Its already a couple of months without any review...

project0 avatar Feb 18 '25 10:02 project0

@jj22ee can you please take a look at this? I know it was opened before you were made a component owner but it's been sitting for quite a while.

dyladan avatar May 14 '25 16:05 dyladan

I think this feature looks reasonable. The package already has similar hooks defined and it is well documented/tested. If it gets rebased I think we can move forward with it. @jj22ee please comment if you disagree otherwise this will get my 👍

dyladan avatar May 14 '25 16:05 dyladan

Hey, sorry for not taking a look at this, thanks @dyladan for the ping. This provides useful custom span configuration by user in the event of an exception, so this change looks good.

@project0 I suppose this solves #1765 indirectly for you because this allows you to update Span status or unrecord the Span exception via the new exceptionHook?

jj22ee avatar May 15 '25 21:05 jj22ee

Please resolve the conflicts and we can get this merged

dyladan avatar Jul 09 '25 16:07 dyladan

@dyladan sorry for the late response, i rebased everything and addressed the small linting issues.

project0 avatar Jul 30 '25 07:07 project0