-
Notifications
You must be signed in to change notification settings - Fork 576
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
Fix ECS resource detector bug #569
Conversation
Codecov Report
@@ Coverage Diff @@
## main #569 +/- ##
=======================================
- Coverage 77.9% 77.9% -0.1%
=======================================
Files 55 55
Lines 2593 2594 +1
=======================================
Hits 2022 2022
- Misses 441 442 +1
Partials 130 130
|
Following the issue I've opened here, I went ahead and did a test with this implementation and it working very great for me on ECS with the following modifications
ecsResourceDetector := ecs.ResourceDetector{Utils: new(ecs.DetectorUtils)}
ecsResource, err := ecsResourceDetector.Detect(ctx) |
@MrAlias addressed your comments! |
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
* Update go-sdk.mdx 1. Updated creation of OTEL exporter as per upstream `0.16.0` 2. Updated ECS and EKS resource detector initialization due to recent bug fix - open-telemetry/opentelemetry-go-contrib#569 open-telemetry/opentelemetry-go-contrib#575 However, we should wait for upstream to release `0.17.0` which will have these changes released. * Added `otelgrpc` dependency
This PR changes the circleci build config to build the project in both go-1.13 and go-1.14 versions. Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com> Co-authored-by: Liz Fong-Jones <lizf@honeycomb.io>
probably now with the fix we can initialize resource detector as below:
Reference Issue: ECS resource detector crash aws-observability/aws-otel-go#16