Skip to content

Fix ECS resource detector bug #569

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

Merged
merged 14 commits into from
Feb 10, 2021
11 changes: 8 additions & 3 deletions detectors/aws/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,23 @@ type detectorUtils interface {
type ecsDetectorUtils struct{}

// resource detector collects resource information from Elastic Container Service environment
type ResourceDetector struct {
type resourceDetector struct {
utils detectorUtils
}

// compile time assertion that ecsDetectorUtils implements detectorUtils interface
var _ detectorUtils = (*ecsDetectorUtils)(nil)

// compile time assertion that resource detector implements the resource.Detector interface.
var _ resource.Detector = (*ResourceDetector)(nil)
var _ resource.Detector = (*resourceDetector)(nil)

// NewResourceDetector returns a resource detector that will detect AWS ECS resources.
func NewResourceDetector() resource.Detector {
return &resourceDetector{utils: ecsDetectorUtils{}}
}

// Detect finds associated resources when running on ECS environment.
func (detector *ResourceDetector) Detect(ctx context.Context) (*resource.Resource, error) {
func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resource, error) {
metadataURIV3 := os.Getenv(metadataV3EnvVar)
metadataURIV4 := os.Getenv(metadataV4EnvVar)

Expand Down
36 changes: 18 additions & 18 deletions detectors/aws/ecs/ecs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func (detectorUtils *MockDetectorUtils) getContainerName() (string, error) {
//succesfully return resource when process is running on Amazon ECS environment
func TestDetect(t *testing.T) {
os.Clearenv()
os.Setenv(metadataV3EnvVar, "3")
os.Setenv(metadataV4EnvVar, "4")
_ = os.Setenv(metadataV3EnvVar, "3")
_ = os.Setenv(metadataV4EnvVar, "4")

detectorUtils := new(MockDetectorUtils)

Expand All @@ -58,52 +58,52 @@ func TestDetect(t *testing.T) {
semconv.ContainerIDKey.String("0123456789A"),
}
expectedResource := resource.NewWithAttributes(labels...)
detector := ResourceDetector{detectorUtils}
resource, _ := detector.Detect(context.Background())
detector := &resourceDetector{utils: detectorUtils}
res, _ := detector.Detect(context.Background())

assert.Equal(t, resource, expectedResource, "Resource returned is incorrect")
assert.Equal(t, res, expectedResource, "Resource returned is incorrect")
}

//returns empty resource when detector cannot read container ID
func TestDetectCannotReadContainerID(t *testing.T) {
os.Clearenv()
os.Setenv(metadataV3EnvVar, "3")
os.Setenv(metadataV4EnvVar, "4")
_ = os.Setenv(metadataV3EnvVar, "3")
_ = os.Setenv(metadataV4EnvVar, "4")
detectorUtils := new(MockDetectorUtils)

detectorUtils.On("getContainerName").Return("container-Name", nil)
detectorUtils.On("getContainerID").Return("", errCannotReadContainerID)

detector := ResourceDetector{detectorUtils}
resource, err := detector.Detect(context.Background())
detector := &resourceDetector{utils: detectorUtils}
res, err := detector.Detect(context.Background())

assert.Equal(t, errCannotReadContainerID, err)
assert.Equal(t, 0, len(resource.Attributes()))
assert.Equal(t, 0, len(res.Attributes()))
}

//returns empty resource when detector cannot read container Name
func TestDetectCannotReadContainerName(t *testing.T) {
os.Clearenv()
os.Setenv(metadataV3EnvVar, "3")
os.Setenv(metadataV4EnvVar, "4")
_ = os.Setenv(metadataV3EnvVar, "3")
_ = os.Setenv(metadataV4EnvVar, "4")
detectorUtils := new(MockDetectorUtils)

detectorUtils.On("getContainerName").Return("", errCannotReadContainerName)
detectorUtils.On("getContainerID").Return("0123456789A", nil)

detector := ResourceDetector{detectorUtils}
resource, err := detector.Detect(context.Background())
detector := &resourceDetector{utils: detectorUtils}
res, err := detector.Detect(context.Background())

assert.Equal(t, errCannotReadContainerName, err)
assert.Equal(t, 0, len(resource.Attributes()))
assert.Equal(t, 0, len(res.Attributes()))
}

//returns empty resource when process is not running ECS
func TestReturnsIfNoEnvVars(t *testing.T) {
os.Clearenv()
detector := ResourceDetector{}
resource, err := detector.Detect(context.Background())
detector := &resourceDetector{utils: nil}
res, err := detector.Detect(context.Background())

assert.Equal(t, errNotOnECS, err)
assert.Equal(t, 0, len(resource.Attributes()))
assert.Equal(t, 0, len(res.Attributes()))
}