Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting TraceID from an OpenTracing DatadogSpanContext can throw "Cannot read properties of undefined (reading 'toString') #4987

Open
davidalpert opened this issue Dec 10, 2024 · 4 comments

Comments

@davidalpert
Copy link

davidalpert commented Dec 10, 2024

Calling this DatadogSpanContext.toTraceId() method on what appears to be a valid span can sometimes error out with an undefined inner this._traceId value:

toTraceId (get128bitId = false) {
if (get128bitId) {
return this._traceId.toBuffer().length <= 8 && this._trace.tags[TRACE_ID_128]
? this._trace.tags[TRACE_ID_128] + this._traceId.toString(16).padStart(16, '0')
: this._traceId.toString(16).padStart(32, '0')
}
return this._traceId.toString(10)
}

Suggested Solution

  • While I am not sure what might cause this scenario, a quick fix would be to make the dot property access of this._traceId a bit more defensive:

     toTraceId (get128bitId = false) {
        if (this._traceId && get128bitId) {
          return this._traceId.toBuffer().length <= 8 && this._trace.tags[TRACE_ID_128]
            ? this._trace.tags[TRACE_ID_128] + this._traceId.toString(16).padStart(16, '0')
            : this._traceId.toString(16).padStart(32, '0')
        }
        return this._traceId?.toString(10)
      }
    

Replication Steps

  • Using:

        "dd-trace": "5.25.0",
    
  • Initialize the tracer on startup:

    import tracer from 'dd-trace';
    import { version } from '../../package.json';
    import { buildVars } from '#root/.build/buildVars';
    
    tracer.init({
      service: process.env.DD_SERVICE || 'my-fe-app',
      version: version ? `${version}-${buildVars.GIT_SHA}` : '0.0.0',
      tags: {
        pid: process.pid,
      },
      runtimeMetrics: true,
      logInjection: true,
    });
    
  • Try to defensively get the active span's traceID at runtime

    import tracer from 'dd-trace';
    
    ...
    
        const span = tracer.scope().active();
        const traceID = span?.context()?.toTraceId() ?? 'n/a';
    
    ...
    

Expected result

  • traceID is either a valid Trace ID from the current span or undefined in which case we default to 'n/a/ based on the code above

Actual result

  • We regularly encounter an error whereby we seem to have a defined span (i.e. not null or undefined) with an empty or missing DatadogSpanContext._traceID property such that when we call toTraceId() on it we get the following error:

    .../my_app/node_modules/dd-trace/packages/dd-trace/src/opentracing/span_context.js:39
         return this._traceId.toString(10)
                                          ^
    TypeError: Cannot read properties of undefined (reading 'toString')
         at DatadogSpanContext.toTraceId (.../my_app/packages/dd-trace/src/opentracing/span_context.js:39:26)
    

Unit Tests

  • Here is a failing test we can add to packages/dd-trace/test/opentracing/span_context.spec.js to demonstrate the issue

    'use strict'
    
    require('../setup/tap')
    
    const { expect } = require('chai')
    const id = require('../../src/id')
    
    describe('SpanContext', () => {
      let SpanContext
      let TraceState
    
      beforeEach(() => {
        SpanContext = require('../../src/opentracing/span_context')
        TraceState = require('../../src/opentracing/propagation/tracestate')
      })
     
      ...
      describe('toTraceparent()', () => {
      ...
    
        it('should allow nullsafe access to trace ID', () => {
          const spanContext = new SpanContext({
            traceId: undefined,
            spanId: id('456', 16)
          })
    
          spanContext._trace.tags['_dd.p.tid'] = null
    
          expect(spanContext.toTraceId()).to.equal(undefined)
        })
      })
    })
    
@mtgto
Copy link

mtgto commented Dec 20, 2024

We too are having trouble with this error sometimes after we upgraded from v5.24.0 to v5.25.0.
Here are a portion of our code.

import tracer from 'dd-trace'

const serviceName = process.env['DD_SERVICE']

if (serviceName) {
  tracer.init({
    logInjection: true,
    service: serviceName,
  })
}

...

export function logger() {
  // Set req.traceId to http access log
  return (req: Request, res: Response, next: NextFunction) => {
    // dd-trace v5.25.0 sometimes raise error `Cannot read properties of undefined (reading 'toString')`
    // in https://github.com/DataDog/dd-trace-js/blob/497c0b0d4333b6a7e7a434fb4de275595e5950c9/packages/dd-trace/src/opentracing/span_context.js#L39
    req.traceId = tracer?.scope().active()?.context().toTraceId()
    delegate(req, res, next)
  }
}

const app = express()
app.use(logger())

...

@rdsedmundo
Copy link

This set of libraries is honestly disappointingly unstable. I feel like every time we upgrade another random issue like this appears. We had it pinned since May 2024 to circumvent another bug, but yesterday I was forced to unpin to fix another problem, and now I wake up to several reports of this error reported here.

You seriously need to invest in better integration tests.

@rdsedmundo
Copy link

@rochdev @uurien @watson any feedback here? This seems to be affecting all installations for a month now.

@rdsedmundo
Copy link

We're using serverless-plugin-datadog and getting this library through the layer that is published in datadog-lambda-js, the dd-trace version there is not even v5 yet for some reason, but it also has this same problem here.

I've downgraded the layer to 112, which for us is the LKGC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants