DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
646 stars 306 forks source link

SNS->SQS->JMS causes error "JMSException: Binary is not a supported JMS property type" #3549

Open falken opened 1 year ago

falken commented 1 year ago

Expected behaviour 1) A message is sent from a nodejs application to an SNS topic with _datadog context propagation header set. 2) A messages is placed on an SQS queue based on a subscription to the SNS topic 3) A JMS consumer consumes and processes the message

Actual behaviour 1 & 2 function properly Step 3 fails with an error "JMSException: Binary is not a supported JMS property type" thrown by this code in the aws library https://github.com/awslabs/amazon-sqs-java-messaging-lib/blob/4cb91355cb92d9361a2179233c9db89383b1299e/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java#L1057

The issue appears to be that when a message is sent to an SNS topic the DataType is set to "Binary"

https://github.com/DataDog/dd-trace-js/blob/da4b19df5e5b7f85f56873372869d677d60d389e/packages/datadog-plugin-aws-sdk/src/services/sns.js#L76C26-L76C26

Which causes the error in the previous code link.

In the SQS version of the code it uses a DataType of "String": https://github.com/DataDog/dd-trace-js/blob/da4b19df5e5b7f85f56873372869d677d60d389e/packages/datadog-plugin-aws-sdk/src/services/sqs.js#L164

It looks like it's using Binary because of some sort of Message Attribute filtering issue https://github.com/DataDog/serverless-plugin-datadog/issues/232#issuecomment-1050372590

Steps to reproduce

Environment Linux AWS ECS Fargate

falken commented 1 year ago

This change appears to have changed it to a Binary value https://github.com/DataDog/dd-trace-js/commit/2a3bb58efac4c5790a4e1f49b355f44f4e9f0027

astuyve commented 1 year ago

Hi @falken, thanks for reaching out!

We need to use a binary type attribute because even a well-formatted JSON string breaks message attribute filtering in AWS SNS, and AWS has no plans to change this (please feel free to open a support ticket in AWS and mention me by name, maybe that will spur their motivation?). The reason is that SNS message filtering is not supported for Binary typed message attributes, so these are passed with no issues.

Unfortunately the library you've linked to has had an open bug ticket for this since 2018.

I'm not familiar with this library, is there a way to ask it not to deserialize this payload? Alternatively you can try disabling raw message delivery, which should strip out the sns metadata, or disable the aws sdk upstream with DD_TRACE_DISABLED_INSTRUMENTATIONS=aws-sdk. This will prevent trace injection into these messages at all.

If you still want trace context passed through SNS and SQS and to use the amazon-sqs-java-messaging-lib, you may need to inject and extract trace context yourself.

We're exploring other methods of trace context injection, but don't have immediate plans at the moment.

Thanks!