DataDog / datadog-lambda-js

The Datadog AWS Lambda Library for Node
Apache License 2.0
107 stars 35 forks source link

"Cannot read properties of undefined (reading 'toString')" when calling getTraceHeaders() #482

Closed midu-git closed 6 months ago

midu-git commented 6 months ago

Expected Behavior

Calling getTraceHeaders() should not error out and provide trace headers correctly.

Actual Behavior

We are using this guy from a lambda called on the s3 middleware. At least in some situations, we encounter this error

{
    errorType: "TypeError",
    errorMessage: "Cannot read properties of undefined (reading 'toString')",
    stack: [
        "TypeError: Cannot read properties of undefined (reading 'toString')",
        "    at TraceContextService.get (/opt/nodejs/node_modules/datadog-lambda-js/trace/trace-context-service.js:87:93)",
        "    at TraceListener.get (/opt/nodejs/node_modules/datadog-lambda-js/trace/listener.js:76:40)",
        "    at getTraceHeaders (/opt/nodejs/node_modules/datadog-lambda-js/index.js:363:33)",
        "    at injectDatadogHeadersIntoS3 (/shared/src/utils/tracing-utils.ts:71:28)",
        "    at next (/shared/src/utils/s3-utils.ts:219:17)",
        "    at <anonymous> (/node_modules/@smithy/middleware-apply-body-checksum/dist-cjs/applyMd5BodyChecksumMiddleware.js:29:12)",
        "    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)",
    ],
};

We have identified the error to this code

public sampleMode(): number {
  return this.spanContext._sampling?.priority;
}

Basically, you allow a number to be returned, but it should be number | undefined.

Steps to Reproduce the Problem

1. 1. 1.

Specifications

Stacktrace

        "TypeError: Cannot read properties of undefined (reading 'toString')",
        "    at TraceContextService.get (/opt/nodejs/node_modules/datadog-lambda-js/trace/trace-context-service.js:87:93)",
        "    at TraceListener.get (/opt/nodejs/node_modules/datadog-lambda-js/trace/listener.js:76:40)",
        "    at getTraceHeaders (/opt/nodejs/node_modules/datadog-lambda-js/index.js:363:33)",
        "    at injectDatadogHeadersIntoS3 (/shared/src/utils/tracing-utils.ts:71:28)",
        "    at next (/shared/src/utils/s3-utils.ts:219:17)",
        "    at <anonymous> (/node_modules/@smithy/middleware-apply-body-checksum/dist-cjs/applyMd5BodyChecksumMiddleware.js:29:12)",
        "    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)",
duncanista commented 6 months ago

Hey @midu-git,

Thanks for reporting this! It seems that back in the past, we were ALWAYS setting AUTO_KEEP (1), after we added W3C support, we started using whatever the user had. So now, we'll just mark to AUTO_KEEP when no sampling is found.

I created a PR for this, and will follow up as things progress!

astuyve commented 6 months ago

This should be fixed in v8.106.