Fix ECS resource detector bug (#569)

* added failure scenario when getting container fails * fix test case failure * add changelog * fix ecs resource detector bug * fix struct name as per golint suggestion * minor changes * added NewResourceDetector func and interface assertions * fix golint failure * minor changes to address review comments Co-authored-by: Anthony Mirabella <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
open-telemetry · Feb 10, 2021 · e532370 · e532370
1 parent b87d221
commit e532370
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 21 deletions.
diff --git a/detectors/aws/ecs/ecs.go b/detectors/aws/ecs/ecs.go
@@ -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)
 

diff --git a/detectors/aws/ecs/ecs_test.go b/detectors/aws/ecs/ecs_test.go
@@ -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)
 
@@ -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()))
 }