-
Notifications
You must be signed in to change notification settings - Fork 821
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
Feat: Added Amazon EKS Resource Detector #1669
Feat: Added Amazon EKS Resource Detector #1669
Conversation
b1f9ba7
to
7d6a1c0
Compare
Codecov Report
@@ Coverage Diff @@
## master #1669 +/- ##
==========================================
- Coverage 91.38% 91.35% -0.03%
==========================================
Files 164 165 +1
Lines 5059 5138 +79
Branches 1048 1056 +8
==========================================
+ Hits 4623 4694 +71
- Misses 436 444 +8
|
@dyladan Hi Daniel, would really appreciate any comments or review of the code when possible thanks! |
@obecny would really appreciate a review and merge if possible, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
- Suppressing instrumentation
- I see in some places
EKS
and in otherEks
can you please unify it ? - Just a few minor/Nit things
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/test/detectors/AwsEksDetector.test.ts
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Outdated
Show resolved
Hide resolved
@KKelvinLo something wrong either with merge or rebase with latest master I suddenly see here changes which has nothing to do with yr PR. |
63aca8c
to
20d7ea4
Compare
@obecny Consistencies resolved and rebased master, cheers! |
packages/opentelemetry-resource-detector-aws/test/detectors/AwsEksDetector.test.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-resource-detector-aws/src/detectors/AwsEksDetector.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thx for changes
Which problem is this PR solving?
Short description of the changes
If you can review @dyladan, @anuraaga, thanks a lot!