DataDog / datadog-lambda-java

The Datadog AWS Lambda package for Java
Apache License 2.0
14 stars 14 forks source link

Malformed _X_AMZN_TRACE_ID value #89

Closed robturner-kaluza closed 1 year ago

robturner-kaluza commented 1 year ago

Overnight we've suddently started to see Malformed _X_AMZN_TRACE_ID errors.

As an example the value we're seeing is: Root=1-46105bdf-04c13a9504458ebc539f5fba;Parent=240a548a42a88af4;Sampled=0;Lineage=12326a9d:0

Looks like lineage is valid (https://docs.aws.amazon.com/xray/latest/devguide/xray-concepts.html#xray-concepts-tracingheader) however this library is throwing an error when it splits the traceId by ;. See here: https://github.com/DataDog/datadog-lambda-java/blob/7f1c919c8979502fc7d072f7aee18e01d2d635b5/src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java#L397

Not sure I can prevent the lineage property appearing in traceIds but should this library handle this case?

DReigada commented 1 year ago

We're also experiencing this.

Seems like AWS added a new field overnight to the tracing environment variable. It is a valid value for the variable though, according to their docs

astuyve commented 1 year ago

Hi, thanks for reaching out!

It does look like this is a change to AWS X-Ray.

That said, this library is now deprecated, and no longer needed.

There is an upgrade guide available in the documentation.

Please let me know if you have issues upgrading.

Thanks!

GitMarco commented 1 year ago

I have a fix, sadly issues pushing a branch gives me a 403, tried many things. And since there's an upgrade possible, I'll just post the .patch file here, should anybody need it. This code requires at least 3 parts in the trace, any higher number is allowed too.

------- Patch starts below this line ------------------------------


Index: build.gradle
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/build.gradle b/build.gradle
--- a/build.gradle  (revision d8ef9596fe8430acd6117f640a4a40addc1fc3f1)
+++ b/build.gradle  (revision 5f2304c164ab7571e211352d90670cbe756e3ec2)
@@ -54,7 +54,7 @@
 targetCompatibility = 1.8

 group = 'com.datadoghq'
-version= '1.4.5'
+version= '1.4.10'
 archivesBaseName = "datadog-lambda-java"
 description = "datadog-lambda-java"

Index: src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java b/src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java
--- a/src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java  (revision d8ef9596fe8430acd6117f640a4a40addc1fc3f1)
+++ b/src/main/java/com/datadoghq/datadog_lambda_java/Tracing.java  (revision 5f2304c164ab7571e211352d90670cbe756e3ec2)
@@ -366,14 +366,15 @@
     String parentId;

     public XRayTraceContext(){
-        //Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510;Sampled=1
+        // Example: Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510;Sampled=1
+        // Example: Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510;Sampled=0;Lineage=f627d631:0
         String traceId = System.getenv("_X_AMZN_TRACE_ID");
         if (traceId == null || traceId.equals("")){
             DDLogger.getLoggerImpl().debug("Unable to find _X_AMZN_TRACE_ID");
             return;
         }
         String[] traceParts = traceId.split(";");
-        if(traceParts.length != 3){
+        if(hasValidNumberOfParts(traceParts)){
             DDLogger.getLoggerImpl().error ("Malformed _X_AMZN_TRACE_ID value: "+ traceId);
             return;
         }
@@ -388,13 +389,17 @@
         this.traceIdHeader = traceId;
     }

+    private static boolean hasValidNumberOfParts(String[] traceParts) {
+        return traceParts.length < 3;
+    }
+
     /**
      * Test constructor that can take a dummy _X_AMZN_TRACE_ID value rather than reading from env vars
      * @param traceId
      */
     protected XRayTraceContext(String traceId){
         String[] traceParts = traceId.split(";");
-        if(traceParts.length != 3){
+        if(hasValidNumberOfParts(traceParts)){
             DDLogger.getLoggerImpl().error("Malformed _X_AMZN_TRACE_ID value: "+ traceId);
             return;
         }
Index: src/test/java/com/datadoghq/datadog_lambda_java/TracingTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/test/java/com/datadoghq/datadog_lambda_java/TracingTest.java b/src/test/java/com/datadoghq/datadog_lambda_java/TracingTest.java
--- a/src/test/java/com/datadoghq/datadog_lambda_java/TracingTest.java  (revision d8ef9596fe8430acd6117f640a4a40addc1fc3f1)
+++ b/src/test/java/com/datadoghq/datadog_lambda_java/TracingTest.java  (revision 5f2304c164ab7571e211352d90670cbe756e3ec2)
@@ -4,11 +4,15 @@
 import com.google.gson.reflect.TypeToken;
 import java.lang.reflect.Type;
 import java.util.HashMap;
-import org.junit.Assert;
+
 import org.junit.Test;

 import java.util.Map;

+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
 public class TracingTest {

     @Test
@@ -29,10 +33,10 @@

         Map<String, String> headers = t.makeOutboundHttpTraceHeaders();

-        Assert.assertEquals(3, headers.size());
-        Assert.assertEquals("2", headers.get(cxt.ddSamplingKey));
-        Assert.assertEquals("12344567890", headers.get(cxt.ddTraceKey));
-        Assert.assertEquals("6023947403358210776", headers.get(cxt.ddParentKey));
+        assertEquals(3, headers.size());
+        assertEquals("2", headers.get(cxt.ddSamplingKey));
+        assertEquals("12344567890", headers.get(cxt.ddTraceKey));
+        assertEquals("6023947403358210776", headers.get(cxt.ddParentKey));
     }

     @Test
@@ -53,7 +57,7 @@

         Map<String, String> headers = t.makeOutboundHttpTraceHeaders();

-        Assert.assertEquals(0, headers.size());
+        assertEquals(0, headers.size());
     }

     @Test
@@ -83,6 +87,24 @@
         expectedTracingInfo.put(cxt.ddParentKey, "6023947403358210776");
         expectedTracingInfo.put(cxt.ddSamplingKey, cxt.samplingPriority);

-        Assert.assertEquals(expectedTracingInfo, tracingInfo);
+        assertEquals(expectedTracingInfo, tracingInfo);
+    }
+
+    @Test
+    public void shouldSupportAtLeastThreePartsIn_X_AMZN_TRACE_ID() {
+        XRayTraceContext xRayTraceContextBefore2023March14 = new XRayTraceContext("Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510;Sampled=1");
+        assertEquals("1-5e41a79d-e6a0db584029dba86a594b7e", xRayTraceContextBefore2023March14.getTraceId());
+        assertEquals("8c34f5ad8f92d510", xRayTraceContextBefore2023March14.getParentId());
+
+        XRayTraceContext xRayTraceContextSince2023March14 = new XRayTraceContext("Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510;Sampled=0;Lineage=f627d631:0");
+        assertEquals("1-5e41a79d-e6a0db584029dba86a594b7e", xRayTraceContextSince2023March14.getTraceId());
+        assertEquals("8c34f5ad8f92d510", xRayTraceContextSince2023March14.getParentId());
+    }
+
+    @Test
+    public void shouldIgnoreLessThanThreePartsIn_X_AMZN_TRACE_ID() {
+        XRayTraceContext xRayTraceContextLessThanThreeParts = new XRayTraceContext("Root=1-5e41a79d-e6a0db584029dba86a594b7e;Parent=8c34f5ad8f92d510");
+        assertNull(xRayTraceContextLessThanThreeParts.getTraceId());
+        assertNull(xRayTraceContextLessThanThreeParts.getParentId());
     }
 }
dataGeeek commented 1 year ago

Hi astuyve,

we have currently a setup that prevents us from migrating to the new lambda extension solution.

We therefore like to know, if there is an alternative way to obtain the files we need to install to /opts/? E.g. by downloading a .tar file?

astuyve commented 1 year ago

Hi @dataGeeek!

Thanks for the question. For container-image based services, we have a different documentation page which should handle your use case.

Thanks!

dataGeeek commented 1 year ago

Hi @astuyve,

unfortunately the proposed solution for container-image based services does not work out-of-the-box for us, because jib does not support the COPY --from=public.ecr.aws/datadog/lambda-extension:<TAG> /opt/. /opt/ command.

Therefore I'm asking if there is a nother way to fetch the necessary files and place them in the docker container?

Thanks for your support!

astuyve commented 1 year ago

Given a user cannot upgrade, we've decided it's best to backport a fix to this library.

I'll update this ticket when we perform the release, thanks everyone!

astuyve commented 1 year ago

Hi folks,

This is fix is released in 1.4.10.

Thanks!