From 172066c13e69b8a00389bbf3f261ebb3dc546634 Mon Sep 17 00:00:00 2001 From: Jay Camp Date: Wed, 24 Mar 2021 22:48:49 -0400 Subject: [PATCH] review updates --- .../resourcedetectionprocessor/README.md | 16 ++++++- .../internal/aws/eks/detector.go | 46 +++++++++---------- .../internal/aws/eks/detector_test.go | 13 +++--- 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/processor/resourcedetectionprocessor/README.md b/processor/resourcedetectionprocessor/README.md index 1627a23eb9b5..ea1bfd1f6bf4 100644 --- a/processor/resourcedetectionprocessor/README.md +++ b/processor/resourcedetectionprocessor/README.md @@ -112,7 +112,21 @@ detectors: [ ] override: ``` -Note that if multiple detectors are inserting the same attribute name, the first detector to insert wins. For example if you had `detectors: [eks, ec2]` then `cloud.infrastructure_service` will be `aws_eks` instead of `ec2`. +## Ordering + +Note that if multiple detectors are inserting the same attribute name, the first detector to insert wins. For example if you had `detectors: [eks, ec2]` then `cloud.infrastructure_service` will be `aws_eks` instead of `ec2`. The below ordering is recommended. + +### GCP + +* gke +* gce + +### AWS + +* elastic_beanstalk +* eks +* ecs +* ec2 The full list of settings exposed for this extension are documented [here](./config.go) with detailed sample configurations [here](./testdata/config.yaml). diff --git a/processor/resourcedetectionprocessor/internal/aws/eks/detector.go b/processor/resourcedetectionprocessor/internal/aws/eks/detector.go index be505c5fbae8..769c2338feaf 100644 --- a/processor/resourcedetectionprocessor/internal/aws/eks/detector.go +++ b/processor/resourcedetectionprocessor/internal/aws/eks/detector.go @@ -23,7 +23,6 @@ import ( "io/ioutil" "net/http" "os" - "time" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/consumer/pdata" @@ -36,19 +35,20 @@ const ( // TypeStr is type of detector. TypeStr = "eks" - k8sSvcURL = "https://kubernetes.default.svc" /* #nosec */ k8sTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" k8sCertPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" - authConfigmapPath = "/api/v1/namespaces/kube-system/configmaps/aws-auth" - cwConfigmapPath = "/api/v1/namespaces/amazon-cloudwatch/configmaps/cluster-info" - timeoutMillis = 2000 + authConfigmapPath = "https://kubernetes.default.svc/api/v1/namespaces/kube-system/configmaps/aws-auth" + cwConfigmapPath = "https://kubernetes.default.svc/api/v1/namespaces/amazon-cloudwatch/configmaps/cluster-info" ) // detectorUtils is used for testing the resourceDetector by abstracting functions that rely on external systems. type detectorUtils interface { + // fileExists returns true if the file exists, otherwise false. fileExists(filename string) bool - fetchString(httpMethod string, URL string) (string, error) + // fetchString executes an HTTP request with a given HTTP Method and URL string returning + // the content body or an error. + fetchString(ctx context.Context, httpMethod string, URL string) (string, error) } // This struct will implement the detectorUtils interface @@ -77,7 +77,7 @@ func NewDetector(_ component.ProcessorCreateParams, _ internal.DetectorConfig) ( // Detect returns a Resource describing the Amazon EKS environment being run in. func (detector *Detector) Detect(ctx context.Context) (pdata.Resource, error) { res := pdata.NewResource() - isEks, err := isEKS(detector.utils) + isEks, err := isEKS(ctx, detector.utils) if err != nil { return res, err } @@ -92,7 +92,7 @@ func (detector *Detector) Detect(ctx context.Context) (pdata.Resource, error) { attr.InsertString(conventions.AttributeCloudInfrastructureService, conventions.AttributeCloudProviderAWSEKS) // Get clusterName and append to attributes - clusterName, err := getClusterName(detector.utils) + clusterName, err := getClusterName(ctx, detector.utils) if err != nil { return res, err } @@ -104,13 +104,13 @@ func (detector *Detector) Detect(ctx context.Context) (pdata.Resource, error) { } // isEKS checks if the current environment is running in EKS. -func isEKS(utils detectorUtils) (bool, error) { +func isEKS(ctx context.Context, utils detectorUtils) (bool, error) { if !isK8s(utils) { return false, nil } // Make HTTP GET request - awsAuth, err := utils.fetchString(http.MethodGet, k8sSvcURL+authConfigmapPath) + awsAuth, err := utils.fetchString(ctx, http.MethodGet, authConfigmapPath) if err != nil { return false, fmt.Errorf("isEks() error retrieving auth configmap: %w", err) } @@ -123,25 +123,26 @@ func isK8s(utils detectorUtils) bool { return utils.fileExists(k8sTokenPath) && utils.fileExists(k8sCertPath) } -// fileExists checks if a file with a given filename exists. +// fileExists returns true if the file exists, otherwise false. func (eksUtils eksDetectorUtils) fileExists(filename string) bool { info, err := os.Stat(filename) return err == nil && !info.IsDir() } -// fetchString executes an HTTP request with a given HTTP Method and URL string. -func (eksUtils eksDetectorUtils) fetchString(httpMethod string, URL string) (string, error) { - request, err := http.NewRequest(httpMethod, URL, nil) +// fetchString executes an HTTP request with a given HTTP Method and URL string returning +// the content body or an error. +func (eksUtils eksDetectorUtils) fetchString(ctx context.Context, httpMethod string, URL string) (string, error) { + request, err := http.NewRequestWithContext(ctx, httpMethod, URL, nil) if err != nil { return "", fmt.Errorf("failed to create new HTTP request with method=%s, URL=%s: %w", httpMethod, URL, err) } // Set HTTP request header with authentication credentials - authHeader, err := getK8sCredHeader() + k8sToken, err := getK8sToken() if err != nil { return "", err } - request.Header.Set("Authorization", authHeader) + request.Header.Set("Authorization", "Bearer "+k8sToken) // Get certificate caCert, err := ioutil.ReadFile(k8sCertPath) @@ -153,7 +154,6 @@ func (eksUtils eksDetectorUtils) fetchString(httpMethod string, URL string) (str // Set HTTP request timeout and add certificate client := &http.Client{ - Timeout: timeoutMillis * time.Millisecond, Transport: &http.Transport{ TLSClientConfig: &tls.Config{ RootCAs: caCertPool, @@ -175,19 +175,19 @@ func (eksUtils eksDetectorUtils) fetchString(httpMethod string, URL string) (str return string(body), nil } -// getK8sCredHeader retrieves the kubernetes credential information. -func getK8sCredHeader() (string, error) { +// getK8sToken retrieves the kubernetes credential information. +func getK8sToken() (string, error) { content, err := ioutil.ReadFile(k8sTokenPath) if err != nil { - return "", fmt.Errorf("getK8sCredHeader() error: cannot read file with path %s", k8sTokenPath) + return "", fmt.Errorf("getK8sToken() error: cannot read file with path %s", k8sTokenPath) } - return "Bearer " + string(content), nil + return string(content), nil } // getClusterName retrieves the clusterName resource attribute -func getClusterName(utils detectorUtils) (string, error) { - resp, err := utils.fetchString("GET", k8sSvcURL+cwConfigmapPath) +func getClusterName(ctx context.Context, utils detectorUtils) (string, error) { + resp, err := utils.fetchString(ctx, "GET", cwConfigmapPath) if err != nil { return "", fmt.Errorf("getClusterName() error: %w", err) } diff --git a/processor/resourcedetectionprocessor/internal/aws/eks/detector_test.go b/processor/resourcedetectionprocessor/internal/aws/eks/detector_test.go index 9bb473655a95..2a191f1636b7 100644 --- a/processor/resourcedetectionprocessor/internal/aws/eks/detector_test.go +++ b/processor/resourcedetectionprocessor/internal/aws/eks/detector_test.go @@ -38,8 +38,8 @@ func (detectorUtils *MockDetectorUtils) fileExists(filename string) bool { } // Mock function for fetchString() -func (detectorUtils *MockDetectorUtils) fetchString(httpMethod string, URL string) (string, error) { - args := detectorUtils.Called(httpMethod, URL) +func (detectorUtils *MockDetectorUtils) fetchString(ctx context.Context, httpMethod string, URL string) (string, error) { + args := detectorUtils.Called(ctx, httpMethod, URL) return args.String(0), args.Error(1) } @@ -51,17 +51,18 @@ func TestNewDetector(t *testing.T) { // Tests EKS resource detector running in EKS environment func TestEks(t *testing.T) { - detectorUtils := new(MockDetectorUtils) + detectorUtils := &MockDetectorUtils{} + ctx := context.Background() // Mock functions and set expectations detectorUtils.On("fileExists", k8sTokenPath).Return(true) detectorUtils.On("fileExists", k8sCertPath).Return(true) - detectorUtils.On("fetchString", "GET", k8sSvcURL+authConfigmapPath).Return("not empty", nil) - detectorUtils.On("fetchString", "GET", k8sSvcURL+cwConfigmapPath).Return(`{"data":{"cluster.name":"my-cluster"}}`, nil) + detectorUtils.On("fetchString", ctx, "GET", authConfigmapPath).Return("not empty", nil) + detectorUtils.On("fetchString", ctx, "GET", cwConfigmapPath).Return(`{"data":{"cluster.name":"my-cluster"}}`, nil) // Call EKS Resource detector to detect resources eksResourceDetector := &Detector{utils: detectorUtils} - res, err := eksResourceDetector.Detect(context.Background()) + res, err := eksResourceDetector.Detect(ctx) require.NoError(t, err) assert.Equal(t, map[string]interface{}{