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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Feb 10, 2021
Merged

Fix ECS resource detector bug #569

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
dcc01de
added failure scenario when getting container fails
bhautikpip Feb 5, 2021
1dd54d9
fix test case failure
bhautikpip Feb 5, 2021
03f6fb4
add changelog
bhautikpip Feb 5, 2021
5919be5
Merge branch 'main' into main
bhautikpip Feb 5, 2021
ebd45b4
Merge branch 'main' into main
Aneurysm9 Feb 5, 2021
e1ff7d0
fix ecs resource detector bug
bhautikpip Feb 7, 2021
041d9b8
Merge branch 'main' of https://github.com/bhautikpip/opentelemetry-go…
bhautikpip Feb 7, 2021
e12a4b1
fix struct name as per golint suggestion
bhautikpip Feb 7, 2021
e906d2a
fix merge conflict
bhautikpip Feb 7, 2021
86c04d1
minor changes
bhautikpip Feb 7, 2021
047f5d0
added NewResourceDetector func and interface assertions
bhautikpip Feb 9, 2021
21db8fa
fix golint failure
bhautikpip Feb 9, 2021
5204d27
minor changes to address review comments
bhautikpip Feb 10, 2021
c9c1bca
Merge branch 'main' into main
MrAlias Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions detectors/aws/ecs/ecs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,19 @@ var (
)

// Create interface for methods needing to be mocked
type detectorUtils interface {
type detectorUtilsResources interface {
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
getContainerName() (string, error)
getContainerID() (string, error)
}

// struct implements detectorUtils interface
type ecsDetectorUtils struct{}
// struct implements detectorUtilsResources interface
type DetectorUtils struct{}
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

// resource detector collects resource information from Elastic Container Service environment
type ResourceDetector struct {
XSAM marked this conversation as resolved.
Show resolved Hide resolved
utils detectorUtils
Utils detectorUtilsResources
}

// compile time assertion that ecsDetectorUtils implements detectorUtils interface
bhautikpip marked this conversation as resolved.
Show resolved Hide resolved
var _ detectorUtils = (*ecsDetectorUtils)(nil)

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

// Detect finds associated resources when running on ECS environment.
func (detector *ResourceDetector) Detect(ctx context.Context) (*resource.Resource, error) {
metadataURIV3 := os.Getenv(metadataV3EnvVar)
Expand All @@ -70,11 +64,11 @@ func (detector *ResourceDetector) Detect(ctx context.Context) (*resource.Resourc
if len(metadataURIV3) == 0 && len(metadataURIV4) == 0 {
return empty, errNotOnECS
}
hostName, err := detector.utils.getContainerName()
hostName, err := detector.Utils.getContainerName()
if err != nil {
return empty, err
}
containerID, err := detector.utils.getContainerID()
containerID, err := detector.Utils.getContainerID()
if err != nil {
return empty, err
}
Expand All @@ -87,7 +81,7 @@ func (detector *ResourceDetector) Detect(ctx context.Context) (*resource.Resourc
}

// returns docker container ID from default c group path
func (ecsUtils ecsDetectorUtils) getContainerID() (string, error) {
func (ecsUtils DetectorUtils) getContainerID() (string, error) {
fileData, err := ioutil.ReadFile(defaultCgroupPath)
if err != nil {
return "", errCannotReadCGroupFile
Expand All @@ -102,7 +96,7 @@ func (ecsUtils ecsDetectorUtils) getContainerID() (string, error) {
}

// returns host name reported by the kernel
func (ecsUtils ecsDetectorUtils) getContainerName() (string, error) {
func (ecsUtils DetectorUtils) getContainerName() (string, error) {
hostName, err := os.Hostname()
if err != nil {
return "", errCannotReadContainerName
Expand Down
28 changes: 14 additions & 14 deletions detectors/aws/ecs/ecs_test.go
View file
Open in desktop
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 @@ -59,51 +59,51 @@ func TestDetect(t *testing.T) {
}
expectedResource := resource.NewWithAttributes(labels...)
detector := ResourceDetector{detectorUtils}
resource, _ := detector.Detect(context.Background())
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())
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())
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())
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()))
}