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(opentelemetry-resources): add instance type and az to aws detector #1055

Merged

Conversation

justinwalz
Copy link
Contributor

@justinwalz justinwalz commented May 14, 2020

Hi, thanks for the great work on this project. I noticed these two items were missing when implementing a custom detector for our system. Figured I'd add it here as well.

Signed-off-by: Justin Walz [email protected]

Short description of the changes

Added: cloud.zone and host.type to aws ec2 detector.

@codecov-io
Copy link

codecov-io commented May 14, 2020

Codecov Report

Merging #1055 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1055   +/-   ##
=======================================
  Coverage   95.12%   95.12%           
=======================================
  Files         213      213           
  Lines        8950     8950           
  Branches      806      806           
=======================================
  Hits         8514     8514           
  Misses        436      436           
Impacted Files Coverage Δ
...rces/src/platform/node/detectors/AwsEc2Detector.ts 87.09% <ø> (ø)
...ry-resources/test/detectors/AwsEc2Detector.test.ts 100.00% <ø> (ø)
...ntelemetry-resources/test/detect-resources.test.ts 100.00% <100.00%> (ø)

Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and welcome to OpenTelemetry community!

@mayurkale22 mayurkale22 requested a review from mwear May 14, 2020 01:56
@mayurkale22 mayurkale22 added enhancement New feature or request size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 14, 2020
@justinwalz justinwalz force-pushed the feature/update-aws-detector branch from 89e47d4 to f1246dd Compare May 14, 2020 04:19
@justinwalz
Copy link
Contributor Author

Pushed updated commit for added tests.

@mayurkale22
Copy link
Member

Please fix the lint (run: npm run lint:fix) build.

@justinwalz justinwalz force-pushed the feature/update-aws-detector branch from f1246dd to 70242cf Compare May 14, 2020 05:05
@dyladan
Copy link
Member

dyladan commented May 14, 2020

@mayurkale22 if we want this in 0.8.1 we should merge it. It's simple enough that I'm ok to merge it if you are. Or we can wait for another review.

Copy link
Member

@mwear mwear left a comment

Choose a reason for hiding this comment

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

LGTM

@mayurkale22
Copy link
Member

@mayurkale22 if we want this in 0.8.1 we should merge it. It's simple enough that I'm ok to merge it if you are. Or we can wait for another review.

I was waiting for @mwear's approval. Thank you @mwear.

@mayurkale22 mayurkale22 merged commit 2409821 into open-telemetry:master May 14, 2020
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
…pen-telemetry#1055)

Co-authored-by: Rauno Viskus <[email protected]>
Co-authored-by: Rauno Viskus <[email protected]>
Co-authored-by: Valentin Marchaud <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants