[awsxrayexporter] Allow sending spans in otlp format with a new config #236

wyTrivail · 2024-10-02T00:03:42Z

Description:

This PR adds a configuration transit_spans_in_otlp_format in X-Ray Exporter to allow customers to choose to send spans in otlp format.

Testing:

unit-tests done.
tested with cloudwatch agent, by running a sample app in EKS and verified the data received in X-Ray contains aws.xray.exporter.config.index_all_attributes and aws.xray.exporter.config.indexed_attributes.

{
    "resource": {
        "attributes": {
            "service.name": "nginx-proxy",
            "cloud.region": "sa-east-1",
            "host.image.id": "ami-05bf16341c7ebb2e4",
            "host.type": "m5.large",
            "ec2.tag.aws:autoscaling:groupName": "eks-ng-23d4a773-82c9130c-e74b-0945-e092-86ea527a4e35",
            "cloud.availability_zone": "sa-east-1a",
            "telemetry.sdk.name": "opentelemetry",
            "aws.xray.exporter.config.index_all_attributes": false,
            "aws.xray.exporter.config.indexed_attributes": ["aws.local.service", "aws.local.operation", "aws.local.environment", "aws.remote.service", "aws.remote.operation", "aws.remote.environment", "aws.remote.resource.identifier", "aws.remote.resource.type"],
            "telemetry.sdk.language": "cpp",
            "cloud.provider": "aws",
            "cloud.account.id": "045274034697",
            "aws.log.group.names": "/aws/containerinsights/yingying2/application",
            "telemetry.sdk.version": "1.8.1",
            "host.name": "ip-192-168-44-197.sa-east-1.compute.internal",
            "ec2.tag.kubernetes.io/cluster/yingying2": "owned",
            "cloud.platform": "aws_eks",
            "host.id": "i-047a17e9780258540"
        }
    },
    "scope": {
        "name": "nginx",
        "version": ""
    },
    "traceId": "dcba8a4a111f2bdec9053b2d4be421c0",
    "spanId": "c85649574f8d75b5",
    "flags": 0,
    "name": "HTTP GET pet-clinic-frontend-java",
    "kind": "SERVER",
    "startTimeUnixNano": 1727825661174485223,
    "endTimeUnixNano": 1727825661177039102,
    "durationNano": 2553879,
    "attributes": {
        "EC2.AutoScalingGroup": "eks-ng-23d4a773-82c9130c-e74b-0945-e092-86ea527a4e35",
        "net.peer.port": 28459,
        "http.target": "/scripts/visits/visits.js",
        "http.flavor": "1.1",
        "Host": "ip-192-168-44-197.sa-east-1.compute.internal",
        "net.peer.ip": "192.168.66.126",
        "http.host": "a9b4d587d6cc9486ea60d63807c00fd7-421249312.sa-east-1.elb.amazonaws.com",
        "aws.local.environment": "eks:yingying2/UnknownNamespace",
        "K8s.Namespace": "UnknownNamespace",
        "http.status_code": 200,
        "http.server_name": "_",
        "http.user_agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/111.0.5563.146 Safari/537.36 CloudWatchSynthetics/arn:aws:synthetics:sa-east-1:045274034697:canary:pc-visit-billings",
        "net.host.port": 80,
        "EC2.InstanceId": "i-047a17e9780258540",
        "EKS.Cluster": "yingying2",
        "PlatformType": "AWS::EKS",
        "http.method": "GET",
        "http.scheme": "http"
    }
}

…ed spans to X-Ray Service

wangzlei · 2024-10-02T01:01:53Z

exporter/awsxrayexporter/awsxray.go

+	for i := 0; i < td.ResourceSpans().Len(); i++ {
+		// 1. build a new trace with one resource span
+		singleTrace := ptrace.NewTraces()
+		td.ResourceSpans().At(i).CopyTo(singleTrace.ResourceSpans().AppendEmpty())


Any reason here copy to a new trace?

because I didn't find a function that takes resourcespans as the input and produce bytes. The only function i found is this MarshalTraces, which takes trace as the input.

I think you need to put them under traces, this is the format used by the export request protobuf.
Basically the protobuf schema has same structure for TracesDatat and ExportTraceServiceRequest

https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto#L38

https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/collector/trace/v1/trace_service.proto#L36

Can you try using ExportTraceServiceRequest https://opentelemetry.io/docs/specs/otlp/#binary-protobuf-encoding I think it should work as well and is closer to otlp implementation, which may have some other information in the future.

ExportTraceServiceRequest is the Orig of Trace, however Orig is not being exposed publicly. there's a function as below, but as you can see, it's a private function.

func (ms Traces) getOrig() *otlpcollectortrace.ExportTraceServiceRequest { return internal.GetOrigTraces(internal.Traces(ms)) }

pingleig · 2024-10-02T01:21:27Z

exporter/awsxrayexporter/config.go

@@ -30,6 +30,9 @@ type Config struct {
 	// AWS client.
 	MiddlewareID *component.ID `mapstructure:"middleware,omitempty"`

+	// X-Ray Export sends spans in its original otlp format to X-Ray Service when this flag is on
+	TransitSpanInOtlpFormat bool `mapstructure:"transit_spans_in_otlp_format,omitempty"`


I am not sure this config name is accurate because strictly speaking we are not using otlp format. Server side is accepting base64 encoded protobuf binary where OTLP is passing the raw bytes are request body. I suggest we call it something like encode_to_otel_binary_base64

I agree with the intention but I don't think exposing too much implementation details to users is helpful. All users need to know is that this flag enable them to send complete otlp format span to X-Ray, which contains more information than X-Ray format. Hence, i would still prefer keeping the name.

pingleig · 2024-10-02T01:30:14Z

exporter/awsxrayexporter/awsxray.go

+		}
+
+		// 4. build bytes into base64 and append with PROTOCOL HEADER at the beginning
+		base64Str := otlpFormatPrefix + base64.StdEncoding.EncodeToString(bytes)


I suppose we are not checking the size of encoded document? Does cwagent config has translator on the batching processor config? Though I think number of spans in one resource span is configured by the batch processor inside application for instrumentation SDK https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#batch-span-processor @mxiamxia Batch processor in agent/collector only has impact on how many resource spans (which equals to documents) and exporter is already handling more than 50 documents.

Yeah we don't check the size, would this cause any issue? I think as long as X-Ray Service can accept such size, we should be good.

Does cwagent config has translator on the batching processor config?

I'm not aware of that.

We do have a batch for xray: https://github.com/aws/amazon-cloudwatch-agent/blob/main/translator/tocwconfig/sampleConfig/trace_config_linux.yaml#L33

pingleig · 2024-10-02T17:19:10Z

exporter/awsxrayexporter/awsxray.go

+		injectIndexConfigIntoOtlpPayload(singleTrace.ResourceSpans().At(0), cfg)
+
+		// 3. Marshal single trace into proto bytes
+		bytes, err := ptraceotlp.NewExportRequestFromTraces(singleTrace).MarshalProto()


nit: we can call it b instead of bytes, bytes is often https://pkg.go.dev/bytes

wangzlei

LGTM

wyTrivail · 2024-10-02T20:10:56Z

I'm not sure if those failures in workflow are related with the change? Seems like they are related to other plugins.

exporter/awsxrayexporter/awsxray.go

[awsxrayexporter] add new configuration to allow sending otlp formatt…

a804a73

…ed spans to X-Ray Service

wyTrivail requested a review from wangzlei as a code owner October 2, 2024 00:03

wangzlei reviewed Oct 2, 2024

View reviewed changes

wangzlei previously approved these changes Oct 2, 2024

View reviewed changes

pingleig reviewed Oct 2, 2024

View reviewed changes

[xray exporter] use underlying proto function

fec30e6

wyTrivail dismissed wangzlei’s stale review via fec30e6 October 2, 2024 17:14

pingleig reviewed Oct 2, 2024

View reviewed changes

pingleig approved these changes Oct 2, 2024

View reviewed changes

[xray exporter] change config name to TransitSpan[s]InOtlpFormat

2cb93e3

wangzlei approved these changes Oct 2, 2024

View reviewed changes

lisguo approved these changes Oct 3, 2024

View reviewed changes

jefchien reviewed Oct 4, 2024

View reviewed changes

exporter/awsxrayexporter/awsxray.go Show resolved Hide resolved

jefchien approved these changes Oct 4, 2024

View reviewed changes

jefchien merged commit 90ec962 into amazon-contributing:aws-cwa-dev Oct 4, 2024
142 of 146 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[awsxrayexporter] Allow sending spans in otlp format with a new config #236

[awsxrayexporter] Allow sending spans in otlp format with a new config #236

wyTrivail commented Oct 2, 2024

wangzlei Oct 2, 2024 •

edited

Loading

wyTrivail Oct 2, 2024

pingleig Oct 2, 2024

pingleig Oct 2, 2024

wyTrivail Oct 2, 2024

pingleig Oct 2, 2024

wyTrivail Oct 2, 2024

pingleig Oct 2, 2024

wyTrivail Oct 2, 2024

lisguo Oct 3, 2024

pingleig Oct 2, 2024

wangzlei left a comment

wyTrivail commented Oct 2, 2024

[awsxrayexporter] Allow sending spans in otlp format with a new config #236

[awsxrayexporter] Allow sending spans in otlp format with a new config #236

Conversation

wyTrivail commented Oct 2, 2024

wangzlei Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangzlei left a comment

Choose a reason for hiding this comment

wyTrivail commented Oct 2, 2024

wangzlei Oct 2, 2024 •

edited

Loading