Fix parsing of binary datadog headers in SQS

[vandonr]

What Does This Do

The binary data we receive when the data type is marked as binary is not base64, it's the string itself, so we don't need to decode it.

Additional Notes

Jira ticket: AIDM-205

[dougqh]

I'm assuming this change is fine.

But I'm wondering why did we assume that the headers were base64 encoded in the first place?

[vandonr]

But I'm wondering why did we assume that the headers were base64 encoded in the first place?

I don't know, maybe because if you look at the message in json form, the binary value is printed as base64 ?

The PR introducing this was #5920 by @mcculls

[mcculls]

But I'm wondering why did we assume that the headers were base64 encoded in the first place?

IIRC the code example originally shared with us did base64 decoding for the binary payload.

The odd thing is that I can still see base64 decoding used in various places when handling _datadog:

https://github.com/DataDog/datadog-lambda-python/blob/main/datadog_lambda/tracing.py#L263
https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/SQS/AwsSqsHeadersAdapters.cs#L79
https://github.com/DataDog/datadog-lambda-js/blob/main/src/trace/context/extractors/sns-sqs.ts#L22
https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-aws-sdk/src/services/sqs.js#L161

Hopefully we don't have a situation where some tracers are sending the JSON string base64 encoded and others aren't - because then we may have to peek at the string to see if we need to base64 decode it.

Since the practical results in AIDM-205 show base64 decoding is not required we should remove it - but we should keep an eye open for any cases where the opposite is true

[PerfectSlayer]

@vandonr @dougqh Can you set up the PR labels (like bug and the related instrumentation/component)? It will help @nayeem-kamal with the next release 😉

[vandonr]

IIRC the code example originally shared with us did base64 decoding for the binary payload.

The odd thing is that I can still see base64 decoding used in various places when handling _datadog:

https://github.com/DataDog/datadog-lambda-python/blob/main/datadog_lambda/tracing.py#L263 https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/SQS/AwsSqsHeadersAdapters.cs#L79 https://github.com/DataDog/datadog-lambda-js/blob/main/src/trace/context/extractors/sns-sqs.ts#L22 https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-aws-sdk/src/services/sqs.js#L161

I'm not too sure about py and ts, but for dotnet, the base 64 decode is applied when the payload is a String.

Base64 is meant to carry bytes in a String format (in a less efficient way). If we already have a bytes, it really makes no sense to use base64.

Hopefully we don't have a situation where some tracers are sending the JSON string base64 encoded and others aren't - because then we may have to peek at the string to see if we need to base64 decode it.

yeah... thankfully, json strings start with a {, which is an invalid base64 character, so looking at the first character should be enough to discriminate.

Since the practical results in AIDM-205 show base64 decoding is not required we should remove it - but we should keep an eye open for any cases where the opposite is true

:nod:

[vandonr]

Hopefully we don't have a situation where some tracers are sending the JSON string base64 encoded and others aren't - because then we may have to peek at the string to see if we need to base64 decode it.

yeah... thankfully, json strings start with a {, which is an invalid base64 character, so looking at the first character should be enough to discriminate.

Since it's pretty simple, and can help prevent potential uncomfortable escalations with clients, I added that mechanism to support both while we figure out if we actually need it.