-
Notifications
You must be signed in to change notification settings - Fork 487
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(detector-aws): add DetectorSync implementation for EKS detector #2333
feat(detector-aws): add DetectorSync implementation for EKS detector #2333
Conversation
…ce for EKS detector
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2333 +/- ##
==========================================
- Coverage 90.97% 90.42% -0.56%
==========================================
Files 146 150 +4
Lines 7492 7371 -121
Branches 1502 1527 +25
==========================================
- Hits 6816 6665 -151
- Misses 676 706 +30
|
@@ -17,5 +17,6 @@ | |||
export * from './AwsEc2Detector'; | |||
export * from './AwsBeanstalkDetector'; | |||
export * from './AwsEcsDetector'; | |||
export * from './AwsEksDetector'; | |||
export { AwsEksDetector, awsEksDetector } from './AwsEksDetector'; |
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.
note for reviewer: open-telemetry/opentelemetry-js#4186
@@ -18,7 +18,7 @@ import * as nock from 'nock'; | |||
import * as sinon from 'sinon'; | |||
import * as assert from 'assert'; | |||
import { Resource } from '@opentelemetry/resources'; | |||
import { awsEksDetector, AwsEksDetector } from '../../src'; | |||
import { awsEksDetector, awsEksDetectorSync, AwsEksDetectorSync } from '../../src'; |
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.
note for reviewer: now that awsEksDetector
delegates the job to awsEksDetectorSync
we need to apply the stubs in the later, hence the import
closed in favour of #2376 following this suggestion |
Context
Another PR moving away from the deprecated
Detector
interface in favour to theDetectorSync
interface. This time is for AWS EKS detector and it might help a bit in open-telemetry/opentelemetry-js#3582Not doing all detectors at once:
Short description of the changes
AwsEksDetectorSync
class and instanceAwsEksDetector
reuse the new sync instanceNote: since this package is marked as stable (major version is 1) we are doing a non breaking change by adding the new class and mark the current as
deprecated
. The async detector changes behavior, it does not throw anymore, and it was discussed in last SIG meeting (2024-07-31). The outcome was we're okay with that.A couple of PRs will follow: