Skip to content

Commit

Permalink
Make ECS detector not error when not on platform (#1428)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
MrAlias authored and rltoSD committed Dec 21, 2021
1 parent 0ec3046 commit e8b9e5b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Fixed

- The `"go.opentelemetry.io/contrib/detector/aws/ecs".Detector` no longer errors if not running in ECS. (#1426, #1428)

## [1.2.0/0.27.0] - 2021-11-15

### Changed
Expand Down
3 changes: 1 addition & 2 deletions detectors/aws/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ var (
errCannotReadContainerID = errors.New("failed to read container ID from cGroupFile")
errCannotReadContainerName = errors.New("failed to read hostname")
errCannotReadCGroupFile = errors.New("ECS resource detector failed to read cGroupFile")
errNotOnECS = errors.New("process is not on ECS, cannot detect environment variables from ECS")
)

// Create interface for methods needing to be mocked
Expand Down Expand Up @@ -74,7 +73,7 @@ func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resourc
metadataURIV4 := os.Getenv(metadataV4EnvVar)

if len(metadataURIV3) == 0 && len(metadataURIV4) == 0 {
return empty, errNotOnECS
return nil, nil
}
hostName, err := detector.utils.getContainerName()
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions detectors/aws/ecs/ecs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func TestReturnsIfNoEnvVars(t *testing.T) {
detector := &resourceDetector{utils: nil}
res, err := detector.Detect(context.Background())

assert.Equal(t, errNotOnECS, err)
assert.Equal(t, 0, len(res.Attributes()))
// When not on ECS, the detector should return nil and not error.
assert.NoError(t, err, "failure to detect when not on platform must not be an error")
assert.Nil(t, res, "failure to detect should return a nil Resource to optimize merge")
}

0 comments on commit e8b9e5b

Please sign in to comment.