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

Breakout ResourceHolder from AwsXrayRemoteSamplerProvider #801

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

thpierce
Copy link
Contributor

Description:

ResourceHolder provides generic utility for other components in the awsxray package, and could be more broadly used. In this commit, we are breaking out the ResourceHolder inner class to it's own class, and exposing the functionality of getResource, so that other classes can rely on it.

Specifically, we are working on new components to meet the needs of #789, which will require the use of this ResourceHolder. Opening up ResourceHolder seems to us to be the best path forward.

Existing Issue(s):

Tangentially related: #789

Testing:

  • Added unit tests, validated they all pass
  • ./gradlew clean assemble succeeds

Documentation:

Unclear if any documentation is required here, can help contribute this as needed. ResourceHolder is a new independent component that can be used to retrieve the Resource from autoconfiguration.

Outstanding items:

None

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 30, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: thpierce / name: Thomas Pierce (1e7883c)

@thpierce
Copy link
Contributor Author

thpierce commented Mar 30, 2023

CLA signed, but can't get the workflow to update. Will just close and re-open.

Update: that worked.

@thpierce thpierce closed this Mar 30, 2023
@thpierce thpierce reopened this Mar 30, 2023
@thpierce thpierce marked this pull request as ready for review March 30, 2023 18:58
@thpierce thpierce requested a review from a team March 30, 2023 18:58
Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

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

lgtm!

ResourceHolder provides generic utility for other components in the awsxray package, and could be more broadly used. In this commit, we are breaking out the ResourceHolder inner class to it's own class, and exposing the functionality of getResource, so that other classes can rely on it. Further, we are deprecating the original ResourceHolder, which we believe was not originally intended to be widely visible/accessible.

Related issue: open-telemetry#789
Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

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

Should be good to merge @trask !

@trask trask merged commit e1a86db into open-telemetry:main Apr 3, 2023
@thpierce thpierce deleted the resourceholder branch April 4, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants