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

added new dir for EC2 resource detector #1017

Merged
merged 23 commits into from
Aug 24, 2021

Conversation

bhautikpip
Copy link
Contributor

This PR only changes the location of ec2 detectors and created a dir for ec2 so that ec2 resource detector has parity with other resource detectors directory structure like ecs and eks. Also, importing ec2 resource detector would be more intuitive with this change.

@bhautikpip
Copy link
Contributor Author

@Aneurysm9 @MrAlias Can you give it a review? It's a very small change :)

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This is more than a directory change, it is a package rename. Please be sure to include an entry in the CHANGELOG to track this break change and be sure to check this is a desired enough change for users of the AWS instrumentation to warrant the change.

@bhautikpip
Copy link
Contributor Author

This is more than a directory change, it is a package rename. Please be sure to include an entry in the CHANGELOG to track this break change and be sure to check this is a desired enough change for users of the AWS instrumentation to warrant the change.

I submitted this PR because I myself got confused that we didn't have EC2 resource detector support since I was only looking at ecs and eks modules and not checking code at root of the package. So, I thought of adding separate ec2 module for EC2 resource detector as well to kind of have a parity (with ECS and EKS) and that will probably help causing less confusion. I think this is a good change specifically because this will enable customer to only import EC2 module if they just want to use EC2 resource detector. Added a CHANGELOG entry for this as well and since it's a breaking change it would be good if we can add this before GA release.

@bhautikpip
Copy link
Contributor Author

Checks are failing due to some internal error feel free to re-run again :)

@Aneurysm9 Aneurysm9 merged commit 6d8faab into open-telemetry:main Aug 24, 2021
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.

None yet

3 participants