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

Make ECS detector not error when not on platform #1428

Merged
merged 6 commits into from
Nov 23, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Nov 19, 2021

The OpenTelemetry specification states "failure to detect any resource
information MUST NOT be considered an error". Update the ECS detector to
not return an error when not running on the ECS platform.

Additionally, return nil instead of an empty resource to accommodate a
quick merge when this detector is used. A nil value means the other
resource this will be merged with will be returned immediately instead
of working through the actual merge.

Resolve #1426

The OpenTelemetry specification states "failure to detect any resource
information MUST NOT be considered an error". Update the ECS detector to
not return an error when not running on the ECS platform.

Additionally, return nil instead of an empty resource to accommodate a
quick merge when this detector is used. A nil value means the other
resource this will be merged with will be returned immediately instead
of working through the actual merge.
@codecov
Copy link

codecov bot commented Nov 19, 2021

Codecov Report

Merging #1428 (937dc08) into main (1255325) will not change coverage.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #1428   +/-   ##
=====================================
  Coverage   69.2%   69.2%           
=====================================
  Files        127     127           
  Lines       5475    5475           
=====================================
  Hits        3794    3794           
  Misses      1534    1534           
  Partials     147     147           
Impacted Files Coverage Δ
detectors/aws/ecs/ecs.go 54.7% <100.0%> (ø)

@MrAlias MrAlias merged commit bb24bac into open-telemetry:main Nov 23, 2021
@MrAlias MrAlias deleted the ecs-detector-no-platform branch November 23, 2021 15:36
rltoSD referenced this pull request in open-o11y/opentelemetry-go-contrib Dec 21, 2021
* Make ECS detector not error when not on platform

The OpenTelemetry specification states "failure to detect any resource
information MUST NOT be considered an error". Update the ECS detector to
not return an error when not running on the ECS platform.

Additionally, return nil instead of an empty resource to accommodate a
quick merge when this detector is used. A nil value means the other
resource this will be merged with will be returned immediately instead
of working through the actual merge.

* Add changes to changelog

* Match ECS detector tests with expected behavior

* Add PR number to changelog

* Remove unused errNotOnECS
plantfansam referenced this pull request in plantfansam/opentelemetry-go-contrib Mar 18, 2022
…1428)

* adding codeql workfklow

* removing PR and commit triggers

* updating changelog

* removing push trigger

Co-authored-by: Azfaar Qureshi <[email protected]>
@pellared pellared added this to the untracked milestone Nov 8, 2024
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.

ECS detector should not fail with error if not running on ECS
5 participants