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

investigate trace-context propagation after transaction_max_spans is hit #3109

Open
trentm opened this issue Jan 18, 2023 · 1 comment
Open
Labels
8.8-candidate agent-nodejs Make available for APM Agents project planning.

Comments

@trentm
Copy link
Member

trentm commented Jan 18, 2023

Before this PR that was part of adding transaction.dropped_spans_stats support, the creation of a span that overshot transactionMaxSpans would return null. After that change, a Span instance is returned (though it has .isRecorded() false -- where "recorded" here corresponds to the sampled flag on that span's traceparent being false).

Looking at trace-context propagation for outgoing HTTP requests:
https://github.com/elastic/apm-agent-nodejs/blob/v3.42.0/lib/instrumentation/http-shared.js#L209-L225
there are two impacts of the above change:

  1. We will be propagating a traceparent with the sampled flag set false.
  2. We will be propagating a traceparent with the parent-id set to the ID of the dropped span.

The latter would cause a broken trace, but given the former, we wouldn't typically expect a downstream service to continue the trace anyway. Before the change (to no longer return null from Transaction.prototype.createSpan()) the APM agent would use the trace-context details from whatever parent span (or the transaction) that was recorded -- so we would get a continued trace.

The same issue will exist for SQS trace-context propagation after the coming change in #3044.

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jan 18, 2023
@trentm
Copy link
Member Author

trentm commented Jan 18, 2023

Note to self: Here is an addition to "sqs.test.js" that starts a test case for this.

sqs.test.js snippet
  test.test('API: sendMessage, hit transactionMaxSpans', function (t) {
    const app = createMockServer(
      getXmlResponse('sendMessage')
    )
    const listener = app.listen(0, function () {
      const oldTransactionMaxSpans = agent._conf.transactionMaxSpans
      agent._conf.transactionMaxSpans = 1
      t.on('end', () => {
        agent._conf.transactionMaxSpans = oldTransactionMaxSpans
      })

      resetAgent(function (data) {
        console.log('XXX data:'); console.dir(data, { depth: 5 })
        // t.equals(data.spans.length, 1, 'generated one span')
        // const spanSqs = data.spans[0]
        // t.equals(spanSqs.name, 'SQS SEND to our-queue', 'SQS span named correctly')
        // t.equals(spanSqs.type, 'messaging', 'span type set to messaging')
        // t.equals(spanSqs.subtype, 'sqs', 'span subtype set to sqs')
        // t.equals(spanSqs.action, 'send', 'span action matches API method called')
        // t.deepEqual(spanSqs.context.service.target,
        //   { type: 'sqs', name: 'our-queue' },
        //   'span.context.service.target')
        // t.deepEqual(spanSqs.context.destination, {
        //   address: 'sqs.us-west.amazonaws.com',
        //   port: 443,
        //   cloud: { region: 'us-west' },
        //   service: { type: '', name: '', resource: 'sqs/our-queue' }
        // }, 'span.context.destination')
        // t.equals(spanSqs.context.message.queue.name, 'our-queue', 'queue name context set')

        // Ensure the request sent to SQS included trace-context in message
        // attributes.  The mock server parses the sent urlencoded body into an
        // object on req.body.
        t.equal(app._receivedReqs.length, 1)
        console.log('XXX _receivedReqs:'); console.dir(app._receivedReqs, { depth: 5 })
        const req = app._receivedReqs[0]
        t.equal(req.body.Action, 'SendMessage', 'req.body.Action')
        // The fixture has 3 message attributes, so traceparent and tracestate
        // should be attributes 4 and 5.
        t.equal(req.body['MessageAttribute.4.Name'], 'traceparent', 'traceparent message attribute Name')
        t.equal(req.body['MessageAttribute.4.Value.DataType'], 'String', 'traceparent message attribute DataType')
        t.equal(req.body['MessageAttribute.4.Value.StringValue'], `00-${spanSqs.trace_id}-${spanSqs.id}-01`, 'traceparent message attribute StringValue')
        t.equal(req.body['MessageAttribute.5.Name'], 'tracestate', 'tracestate message attribute Name')
        t.equal(req.body['MessageAttribute.5.Value.DataType'], 'String', 'tracestate message attribute DataType')
        t.equal(req.body['MessageAttribute.5.Value.StringValue'], 'es=s:1', 'tracestate message attribute StringValue')

        t.end()
      })

      const t0 = agent.startTransaction('t0')
      const s1 = t0.startSpan('s1')
      console.log('XXX s1: ', s1.id, s1.name, s1.ended, s1.traceparent)
      setImmediate(() => {
        const s2 = t0.startSpan('s2')
        console.log('XXX s2: ', s2.id, s2.name, s2.ended, s2.traceparent)
        setImmediate(() => {
          const sqs = new AWS.SQS({ apiVersion: '2012-11-05' })
          const params = getParams('sendMessage', listener.address().port)
          sqs.sendMessage(params, function (err, data) {
            t.error(err)
            s2.end()
            s1.end()
            t0.end()
            listener.close()
          })
        })
      })
    })
  })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.8-candidate agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

No branches or pull requests

1 participant