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

Provide a public API for creating Emitter without exposing implementa… #146

Merged
merged 8 commits into from
May 22, 2020

Conversation

anuraaga
Copy link
Contributor

I noticed that the SDK exposes almost all of its classes to users. For an SDK, where it's important to preserve backwards compatibility, this can make it difficult to improve on the code. An example is #145 where we want to optimize our SDK by changing the implementation of how we request rules, but we can't do so on a minor version release since we expose all of the rule APIs to users. There's a low chance they're using them in their code, but as soon as a method is public, it has to stay that way until the next major version, which cramps evolution of the SDK.

In an aim to improving by 3.0, I'd like to deprecate much of the public API surface to reduce it to only what users / instrumentation writers need. This PR is a small first step to serve as an example of what that may look like.

Let me know your thoughts on this. If it sounds like a good idea (I strongly suggest it), I'll file a tracking issue and we can maybe do a fixit at some point to knock it out.

…tion details and deprecate implementation classes.
@anuraaga anuraaga requested review from shengxil and willarmiros May 15, 2020 09:05
@shengxil
Copy link
Contributor

I noticed that the SDK exposes almost all of its classes to users. For an SDK, where it's important to preserve backwards compatibility, this can make it difficult to improve on the code. An example is #145 where we want to optimize our SDK by changing the implementation of how we request rules, but we can't do so on a minor version release since we expose all of the rule APIs to users. There's a low chance they're using them in their code, but as soon as a method is public, it has to stay that way until the next major version, which cramps evolution of the SDK.

In an aim to improving by 3.0, I'd like to deprecate much of the public API surface to reduce it to only what users / instrumentation writers need. This PR is a small first step to serve as an example of what that may look like.

Let me know your thoughts on this. If it sounds like a good idea (I strongly suggest it), I'll file a tracking issue and we can maybe do a fixit at some point to knock it out.

Great idea! The over exposed implementation has became obstacle of our feature development. There is another plan in 3.0 to isolate X-Ray data models (https://github.com/aws/aws-xray-sdk-java/tree/master/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities) from the core package. Let's sync before merging this.

@anuraaga
Copy link
Contributor Author

Added a DelegatingEmitter to replace inheritance of the implementation classes.

@shengxil
Copy link
Contributor

@anuraaga
Copy link
Contributor Author

Good call thanks!

@anuraaga anuraaga merged commit f393528 into aws:master May 22, 2020
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.

2 participants