elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

Span start/end after parent and trans end should be allowed #635

Closed trentm closed 2 years ago

trentm commented 2 years ago

An APM agent shouldn't prevent the creation/start of a span after its parent or its containing transaction has ended.

FWIW, the Java APM agent (I'm told) already allows span creation and end after its parent and transaction have ended.

Motivation

It is possible (at least in Node.js) to have a span creation/start happen after its containing transaction has already ended. With the OTel API this can happen like this:

const otel = require('@opentelemetry/api')
const tracer = otel.trace.getTracer('play')

// Create an OTel span "s1" with the OTel API. Here we create Transaction "s1".
const s1 = tracer.startSpan('s1')
const ctx1 = otel.trace.setSpan(otel.context.active(), s1)

// The edge case: Create a child of "s1" after s1 has *ended*.
// - When using the OTel SDK: This works. s4 is a child of s1.
// - When using the Elastic OTel Bridge: This results in an *attempt* to create
//   a child Span of s1, but that returns `null` with the debug log:
//        DEBUG: transaction already ended - cannot build new span
s1.end()
const s4 = tracer.startSpan('s4', {}, ctx1)
s4.end()

This "transaction already ended - cannot build new span" limitation in the current Node.js APM agent dates back to v1 of the APM Server intake API when all of a transaction's spans had to be reported with the transaction -- and that was done on transaction end. The v2 intake API no longer requires this. https://github.com/elastic/apm-agent-nodejs/pull/2653 will be removing this restriction in the Node.js APM agent.

checklist

apmmachine commented 2 years ago

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

#### Build stats * Start Time: 2022-05-02T04:01:00.079+0000 * Duration: 3 min 1 sec

trentm commented 2 years ago

This could also happen in the case of e.g. a delayed promise that starts a span, right?

Yes. Or any async task like a setTimeout, as well.