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

chore: Fix Max depth serialization bug when using Batch RecordHanderResult and Tracing #632

Conversation

hjgraca
Copy link
Contributor

@hjgraca hjgraca commented Aug 21, 2024

Please provide the issue number

Issue number: #630

Summary

Changes

  • Update RecordHanderResult to avoid object cycle on static property, this was breaking LitJson parser on X-Ray SDK with Max allowed object depth reached.
  • Tracing now takes into account the CaptureMode set for each method
  • XRayRecorder now captures the exceptions from EndSubsegment call and does not surface exception to the client
  • Update nuget packages for X-Ray SDK

User experience

Please share what the user experience looks like before and after this change

Now when exceptions occour inside X-Ray SDK EndSubsegment we catch those and create a new SubSegment in CloudWatch Traces, this prevents the app from crashing.

image

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

…operty, this was breaking LitJson parser on X-Ray SDK with Max allowed object depth reached.

Tracing  now takes into account the CaptureMode set for each method
XRayRecorder now captures the exceptions from EndSubsegment call and does not surface exception to the client
Update nuget packages for X-Ray SDK
@auto-assign auto-assign bot requested review from amirkaws and sliedig August 21, 2024 11:54
@boring-cyborg boring-cyborg bot added area/tracing Core tracing utility tests labels Aug 21, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 21, 2024
@hjgraca hjgraca added the area/batch Batch utility label Aug 21, 2024
@github-actions github-actions bot added the internal Maintenance changes label Aug 21, 2024
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.76%. Comparing base (2ed95ef) to head (5a4dd56).
Report is 11 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #632      +/-   ##
===========================================
+ Coverage    72.72%   72.76%   +0.04%     
===========================================
  Files          190      190              
  Lines         7874     7886      +12     
  Branches       850      850              
===========================================
+ Hits          5726     5738      +12     
  Misses        1858     1858              
  Partials       290      290              
Flag Coverage Δ
unittests 72.76% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@hjgraca hjgraca linked an issue Aug 21, 2024 that may be closed by this pull request
Copy link
Contributor

@amirkaws amirkaws left a comment

Choose a reason for hiding this comment

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

LGTM

@hjgraca hjgraca merged commit 871f18b into aws-powertools:develop Aug 26, 2024
8 checks passed
@hjgraca hjgraca deleted the fix(tracing)-batch-handler-result-null-reference branch August 26, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/batch Batch utility area/tracing Core tracing utility internal Maintenance changes size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Batch Processing breaks Tracing
2 participants