aws / aws-xray-sdk-node

The official AWS X-Ray SDK for Node.js.
Apache License 2.0
271 stars 155 forks source link

Stop throwing unnecessary errors with adding annotations, metadata, errors #467

Closed wparad closed 2 years ago

wparad commented 3 years ago

Description of changes: Adding annotations, metadata, and errors with malformed data, will be logged as an error instead of hard crashing the caller. XRay Segments are not production critical code and therefore should never throw an exception when it can be avoided. This changes default values and keys to something xray can handle, but won't crash the caller.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

NathanielRN commented 2 years ago

Thanks for this change @wparad! It looks very useful, and I agree with you that the X-Ray SDK should not throw exceptions to the user's app after the initial initialization stage. This is the same philosophy follows with the OTel JS Contrib project which supports X-Ray :)

Would you have bandwidth to fix the tests so we can get this merged?

codecov-commenter commented 2 years ago

Codecov Report

Merging #467 (99b25dd) into master (7283182) will decrease coverage by 0.15%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
- Coverage   82.74%   82.58%   -0.16%     
==========================================
  Files          36       36              
  Lines        1744     1751       +7     
==========================================
+ Hits         1443     1446       +3     
- Misses        301      305       +4     
Impacted Files Coverage Δ
lib/segments/segment.js 81.13% <0.00%> (-0.92%) :arrow_down:
lib/segments/attributes/subsegment.js 78.80% <0.00%> (-0.79%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7283182...99b25dd. Read the comment docs.

wparad commented 2 years ago

@NathanielRN done!

willarmiros commented 2 years ago

Please note that my comments still apply as well :)

wparad commented 2 years ago

Please note that my comments still apply as well :)

@willarmiros, there are no comments on this PR that I can see.

willarmiros commented 2 years ago

Ah! That's because they were still pending :)