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

Improved ECS attribute and origin translation in awsxrayexporter #1428

Merged
merged 6 commits into from
Nov 5, 2020
Merged

Improved ECS attribute and origin translation in awsxrayexporter #1428

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dc73a66
initial commit of ecs translation
Oct 29, 2020
d2bfc7e
reverted trace ID change
Oct 29, 2020
ed27d55
added unit tests
Oct 30, 2020
f03b0df
addressed comments and unit test issues
Nov 3, 2020
8923b17
Merge branch 'master' into xray-translate
willarmiros Nov 4, 2020
c31c6bf
fixed breaking API changes
Nov 5, 2020
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
2 changes: 1 addition & 1 deletion exporter/awsxrayexporter/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsxra
go 1.14

require (
github.com/aws/aws-sdk-go v1.35.10
github.com/aws/aws-sdk-go v1.35.14
github.com/open-telemetry/opentelemetry-collector-contrib/internal/awsxray v0.0.0-00010101000000-000000000000
github.com/stretchr/testify v1.6.1
go.opentelemetry.io/collector v0.13.1-0.20201020175630-99cb5b244aad
Expand Down
5 changes: 3 additions & 2 deletions exporter/awsxrayexporter/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:o
github.com/aws/aws-lambda-go v1.13.3/go.mod h1:4UKl9IzQMoD+QF79YdCuzCwp8VbmG4VAQwij/eHl5CU=
github.com/aws/aws-sdk-go v1.27.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
github.com/aws/aws-sdk-go v1.34.9/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0=
github.com/aws/aws-sdk-go v1.35.10 h1:FsJtrOS7P+Qmq1rPTGgS/+qC1Y9eGuAJHvAZpZlhmb4=
github.com/aws/aws-sdk-go v1.35.10/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
github.com/aws/aws-sdk-go v1.35.14 h1:nucVVXXjAr9UkmYCBWxQWRuYa5KOlaXjuJGg2ulW0K0=
github.com/aws/aws-sdk-go v1.35.14/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
Expand Down Expand Up @@ -1120,6 +1120,7 @@ go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
go.opencensus.io v0.22.4 h1:LYy1Hy3MJdrCdMwwzxA/dRok4ejH+RwNGbuoD9fCjto=
go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
go.opentelemetry.io/collector v0.13.0/go.mod h1:UB7wWD7RrEx8GFSaUR47TO1GAqxSi5+Kq68tI1icwJk=
go.opentelemetry.io/collector v0.13.1-0.20201020175630-99cb5b244aad h1:rg1WNNah58R1WbtoNFqfLB3yuD8uWGTWkdc1U293CU4=
go.opentelemetry.io/collector v0.13.1-0.20201020175630-99cb5b244aad/go.mod h1:UB7wWD7RrEx8GFSaUR47TO1GAqxSi5+Kq68tI1icwJk=
go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
Expand Down
83 changes: 77 additions & 6 deletions exporter/awsxrayexporter/translator/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package translator

import (
"bytes"
"strconv"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -27,6 +28,7 @@ import (
func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]string, *awsxray.AWSData) {
var (
cloud string
service string
account string
zone string
hostID string
Expand All @@ -49,6 +51,14 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
containerID string
clusterName string
podUID string
clusterArn string
containerArn string
taskArn string
taskFamily string
launchType string
logGroups pdata.AnyValueArray
logGroupArns pdata.AnyValueArray
cwl []awsxray.LogGroupMetadata
ec2 *awsxray.EC2Metadata
ecs *awsxray.ECSMetadata
ebs *awsxray.BeanstalkMetadata
Expand All @@ -61,6 +71,8 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
switch key {
case semconventions.AttributeCloudProvider:
cloud = value.StringVal()
case "cloud.infrastructure_service":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As these are destined to be replaced by constants anyways, I'd go ahead and have private constants like AttributeCloudInfrastructureService defined in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, will replace

service = value.StringVal()
case semconventions.AttributeCloudAccount:
account = value.StringVal()
case semconventions.AttributeCloudZone:
Expand Down Expand Up @@ -95,6 +107,20 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
containerID = value.StringVal()
case semconventions.AttributeK8sCluster:
clusterName = value.StringVal()
case "aws.ecs.cluster.arn":
clusterArn = value.StringVal()
case "aws.ecs.container.arn":
containerArn = value.StringVal()
case "aws.ecs.task.arn":
taskArn = value.StringVal()
case "aws.ecs.task.family":
taskFamily = value.StringVal()
case "aws.ecs.launchtype":
launchType = value.StringVal()
case "aws.log.group.names":
logGroups = value.ArrayVal()
case "aws.log.group.arns":
logGroupArns = value.ArrayVal()
}
})
}
Expand Down Expand Up @@ -128,22 +154,29 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
if cloud != "aws" && cloud != "" {
return filtered, nil // not AWS so return nil
}
// progress from least specific to most specific origin so most specific ends up as origin
// as per X-Ray docs
if hostID != "" {

if service == "EC2" || hostID != "" {
ec2 = &awsxray.EC2Metadata{
InstanceID: awsxray.String(hostID),
AvailabilityZone: awsxray.String(zone),
InstanceSize: awsxray.String(hostType),
AmiID: awsxray.String(amiID),
}
}
if container != "" {
if service == "ECS" || container != "" {
ecs = &awsxray.ECSMetadata{
ContainerName: awsxray.String(container),
ContainerID: awsxray.String(containerID),
ContainerName: awsxray.String(container),
ContainerID: awsxray.String(containerID),
AvailabilityZone: awsxray.String(zone),
ContainerArn: awsxray.String(containerArn),
ClusterArn: awsxray.String(clusterArn),
TaskArn: awsxray.String(taskArn),
TaskFamily: awsxray.String(taskFamily),
LaunchType: awsxray.String(launchType),
}
}

// TODO(willarmiros): Add infrastructure_service checks once their resource detectors are implemented
if deployID != "" {
deployNum, err := strconv.ParseInt(deployID, 10, 64)
if err != nil {
Expand All @@ -163,6 +196,14 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
}
}

// Since we must couple log group ARNs and Log Group Names in the same CWLogs object, we first try to derive the
// names from the ARN, then fall back to just recording the names
if logGroupArns != (pdata.AnyValueArray{}) && logGroupArns.Len() > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this != even though we have a len check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I initially didn't have it and it turns out the Len() method relies on an underlying pointer orig in the AnyValueArray struct, so if we don't initialize logGroupArns we get a nil pointer exception. I could check for nilness of orig, but it seemed like an implementation detail that I shouldn't be referencing directly.

cwl = getLogGroupMetadata(logGroupArns, true)
} else if logGroups != (pdata.AnyValueArray{}) && logGroups.Len() > 0 {
cwl = getLogGroupMetadata(logGroups, false)
}

if sdkName != "" && sdkLanguage != "" {
// Convention for SDK name for xray SDK information is e.g., `X-Ray SDK for Java`, `X-Ray for Go`.
// We fill in with e.g, `opentelemetry for java` by using the conventions
Expand All @@ -180,6 +221,7 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
awsData := &awsxray.AWSData{
AccountID: awsxray.String(account),
Beanstalk: ebs,
CWLogs: cwl,
ECS: ecs,
EC2: ec2,
EKS: eks,
Expand All @@ -192,3 +234,32 @@ func makeAws(attributes map[string]string, resource pdata.Resource) (map[string]
}
return filtered, awsData
}

// Given an array of log group ARNs, create a corresponding amount of LogGroupMetadata objects with log_group and arn
// populated, or given an array of just log group names, create the LogGroupMetadata objects with arn omitted
func getLogGroupMetadata(logGroups pdata.AnyValueArray, isArn bool) []awsxray.LogGroupMetadata {
var lgm []awsxray.LogGroupMetadata
for i := 0; i < logGroups.Len(); i++ {
if isArn {
lgm = append(lgm, awsxray.LogGroupMetadata{
Arn: awsxray.String(logGroups.At(i).StringVal()),
LogGroup: awsxray.String(parseLogGroup(logGroups.At(i).StringVal())),
})
} else {
lgm = append(lgm, awsxray.LogGroupMetadata{
LogGroup: awsxray.String(logGroups.At(i).StringVal()),
})
}
}

return lgm
}

func parseLogGroup(arn string) string {
i := bytes.LastIndexByte([]byte(arn), byte(':'))
if i != -1 {
return arn[i+1:]
}

return arn
}
26 changes: 23 additions & 3 deletions exporter/awsxrayexporter/translator/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestAwsFromEc2Resource(t *testing.T) {
resource.InitEmpty()
attrs := pdata.NewAttributeMap()
attrs.InsertString(semconventions.AttributeCloudProvider, semconventions.AttributeCloudProviderAWS)
attrs.InsertString("cloud.infrastructure_service", "EC2")
attrs.InsertString(semconventions.AttributeCloudAccount, "123456789")
attrs.InsertString(semconventions.AttributeCloudZone, "us-east-1c")
attrs.InsertString(semconventions.AttributeHostID, instanceID)
Expand Down Expand Up @@ -63,12 +64,19 @@ func TestAwsFromEcsResource(t *testing.T) {
instanceID := "i-00f7c0bcb26da2a99"
containerName := "signup_aggregator-x82ufje83"
containerID := "0123456789A"
az := "us-east-1c"
launchType := "fargate"
family := "family"
taskArn := "arn:aws:ecs:us-west-2:123456789123:task/123"
clusterArn := "arn:aws:ecs:us-west-2:123456789123:cluster/my-cluster"
containerArn := "arn:aws:ecs:us-west-2:123456789123:container-instance/123"
resource := pdata.NewResource()
resource.InitEmpty()
attrs := pdata.NewAttributeMap()
attrs.InsertString(semconventions.AttributeCloudProvider, semconventions.AttributeCloudProviderAWS)
attrs.InsertString("cloud.infrastructure_service", "ECS")
attrs.InsertString(semconventions.AttributeCloudAccount, "123456789")
attrs.InsertString(semconventions.AttributeCloudZone, "us-east-1c")
attrs.InsertString(semconventions.AttributeCloudZone, az)
attrs.InsertString(semconventions.AttributeContainerImage, "otel/signupaggregator")
attrs.InsertString(semconventions.AttributeContainerTag, "v1")
attrs.InsertString(semconventions.AttributeK8sCluster, "production")
Expand All @@ -78,7 +86,13 @@ func TestAwsFromEcsResource(t *testing.T) {
attrs.InsertString(semconventions.AttributeContainerName, containerName)
attrs.InsertString(semconventions.AttributeContainerID, containerID)
attrs.InsertString(semconventions.AttributeHostID, instanceID)
attrs.InsertString("aws.ecs.cluster.arn", clusterArn)
attrs.InsertString("aws.ecs.container.arn", containerArn)
attrs.InsertString("aws.ecs.task.arn", taskArn)
attrs.InsertString("aws.ecs.task.family", family)
attrs.InsertString("aws.ecs.launchtype", launchType)
attrs.InsertString(semconventions.AttributeHostType, "m5.xlarge")

attrs.CopyTo(resource.Attributes())

attributes := make(map[string]string)
Expand All @@ -92,8 +106,14 @@ func TestAwsFromEcsResource(t *testing.T) {
assert.Nil(t, awsData.Beanstalk)
assert.NotNil(t, awsData.EKS)
assert.Equal(t, &awsxray.ECSMetadata{
ContainerName: aws.String(containerName),
ContainerID: aws.String(containerID),
ContainerName: aws.String(containerName),
ContainerID: aws.String(containerID),
AvailabilityZone: aws.String(az),
ClusterArn: aws.String(clusterArn),
ContainerArn: aws.String(containerArn),
TaskArn: aws.String(taskArn),
TaskFamily: aws.String(family),
LaunchType: aws.String(launchType),
}, awsData.ECS)
}

Expand Down
48 changes: 43 additions & 5 deletions exporter/awsxrayexporter/translator/segment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ import (

// AWS X-Ray acceptable values for origin field.
const (
OriginEC2 = "AWS::EC2::Instance"
OriginECS = "AWS::ECS::Container"
OriginEB = "AWS::ElasticBeanstalk::Environment"
OriginEKS = "AWS::EKS::Container"
OriginEC2 = "AWS::EC2::Instance"
OriginECS = "AWS::ECS::Container"
OriginECSEC2 = "AWS::ECS::EC2"
OriginECSFargate = "AWS::ECS::Fargate"
OriginEB = "AWS::ElasticBeanstalk::Environment"
OriginEKS = "AWS::EKS::Container"
)

var (
Expand Down Expand Up @@ -235,6 +237,38 @@ func determineAwsOrigin(resource pdata.Resource) string {
return ""
}
}

// TODO(willarmiros): Only use infrastructure_service for origin resolution once detectors for all AWS environments are
// implemented for robustness
is, present := resource.Attributes().Get("cloud.infrastructure_service")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can inline, e.g., if is, ok := resource...; ok {

if present {
switch is.StringVal() {
case "EKS":
return OriginEKS
case "ElasticBeanstalk":
return OriginEB
case "ECS":
lt, present := resource.Attributes().Get("aws.ecs.launchtype")
if !present {
return OriginECS
}
switch lt.StringVal() {
case "ec2":
return OriginECSEC2
case "fargate":
return OriginECSFargate
default:
return OriginECS
}
case "EC2":
return OriginEC2

// If infrastructure_service is defined with a non-AWS value, we should not assign it an AWS origin
default:
return ""
}
}

// EKS > EB > ECS > EC2
_, eks := resource.Attributes().Get(semconventions.AttributeK8sCluster)
if eks {
Expand All @@ -248,7 +282,11 @@ func determineAwsOrigin(resource pdata.Resource) string {
if ecs {
return OriginECS
}
return OriginEC2
_, ec2 := resource.Attributes().Get(semconventions.AttributeHostID)
if ec2 {
return OriginEC2
}
return ""
}

// convertToAmazonTraceID converts a trace ID to the Amazon format.
Expand Down
Loading