Skip to content

Conversation

JonkaSusaki
Copy link
Contributor

Issue number: #603

Summary

Changes

This PR creates a wrapper to prevent exposing internal lib Amazon.XRayRecorder.

  • Created libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingSubsegment.cs to place the wrapper;
  • Changed libraries/src/AWS.Lambda.Powertools.Tracing/Tracing.cs to include the wrapper;

User experience

Before the changes, users had to include the reference to Amazon.XRay.Recorder.Core.Internal.Entities to use the Subsegment class. Now, the TracingSubsegment was created and users can now import them using the AWS.Lambda.Powertools.Tracing.Internal reference.

Checklist

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

Is this a breaking change?

NO

Acknowledgment

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

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 4, 2025
@boring-cyborg boring-cyborg bot added the area/tracing Core tracing utility label Aug 4, 2025
Copy link

boring-cyborg bot commented Aug 4, 2025

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #dotnet channel on our Powertools for AWS Lambda Discord: Invite link

@JonkaSusaki
Copy link
Contributor Author

Hey, @hjgraca! I tried implementing it this way.

Please, tell me your thoughts! If there's anything you think might need changes or you believe another approach is better, let me know!

@leandrodamascena
Copy link
Contributor

Hi @JonkaSusaki, thank you so much for working on this PR! We will review this PR as soon as possible and provide feedback.

@github-actions github-actions bot added the feature New features or minor changes label Aug 4, 2025
Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.01%. Comparing base (7f033da) to head (07ac2e4).
⚠️ Report is 12 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #950      +/-   ##
===========================================
+ Coverage    77.80%   78.01%   +0.21%     
===========================================
  Files          285      286       +1     
  Lines        11402    11404       +2     
  Branches      1341     1341              
===========================================
+ Hits          8871     8897      +26     
+ Misses        2100     2078      -22     
+ Partials       431      429       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hjgraca
Copy link
Contributor

hjgraca commented Aug 18, 2025

Hi @JonkaSusaki , thank you so much for the pull request, and sorry for the slow response I have been away for the last 2 weeks.
The pull request looks good, but is missing the tests, If you can add them that would be great, if not I will do the work to add to the pull request.
Again thanks and really appreciate your contribution!

@JonkaSusaki
Copy link
Contributor Author

Hey @hjgraca . Thanks for the response!
I'll add the tests for sure. Going to commit soon.

@hjgraca
Copy link
Contributor

hjgraca commented Sep 1, 2025

Hey @hjgraca . Thanks for the response! I'll add the tests for sure. Going to commit soon.

Hi @JonkaSusaki hope you are well, do you have an idea of when you have time to add the tests?
Thank you!

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 3, 2025
@boring-cyborg boring-cyborg bot added the tests label Sep 3, 2025
@JonkaSusaki
Copy link
Contributor Author

Hello @hjgraca ,sorry for the delay. I added the tests.
Please, tell me your thoughts on what do you think about the implementation and the test suite.

Thanks, and sorry again for the delay.

@hjgraca hjgraca self-requested a review September 9, 2025 14:13
@hjgraca
Copy link
Contributor

hjgraca commented Sep 9, 2025

Thank you @JonkaSusaki I just added a couple more tests to improve coverage. I will merge soon and this will be on the next release!
Really appreciate your contribution and look forward to the next!!! Just ping me on an issue you want to work next

Copy link
Contributor

@hjgraca hjgraca left a comment

Choose a reason for hiding this comment

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

GTM

Copy link

@hjgraca hjgraca merged commit cbf75a4 into aws-powertools:develop Sep 12, 2025
9 checks passed
Copy link

boring-cyborg bot commented Sep 12, 2025

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing Core tracing utility feature New features or minor 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.

Maintenance: Bring Amazon.XRay.Recorder.Core.Internal.Entities into PT Tracing
3 participants