feat(tracing): support Dynatrace OneAgent in-process trace capture

CHANGELOG.md

@@ -16,7 +16,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
 ### Fixed
 
-- No longer crashes on startup when Dynatrace OneAgent is active (`DT_NODE_PRELOAD_OPTIONS` set, kind `*-to-dynatrace`, no `@opentelemetry/exporter-trace-otlp-proto` dependency): the tracing factory no longer builds a tracer provider with an undefined span processor (which crashed on the first span in `MultiSpanProcessor.onStart`). On this path it now sets up no exporter and warns; export traces to Dynatrace via the OTLP exporter (`@opentelemetry/exporter-trace-otlp-proto`)
+- No longer crashes on startup when Dynatrace OneAgent is active (`DT_NODE_PRELOAD_OPTIONS` set, kind `*-to-dynatrace`, no `@opentelemetry/exporter-trace-otlp-proto` dependency): the tracing factory no longer builds a tracer provider with an undefined span processor (which crashed on the first span in `MultiSpanProcessor.onStart`). On this path it now registers a recording tracer provider without an exporter, so the CDS spans exist for OneAgent's in-process OpenTelemetry capture to pick up (no export by us, no duplicates)
 - Logging no longer recurses through `@opentelemetry/sdk-logs` 0.221's export path: the log-processor construction now adapts to the installed sdk-logs version (0.221+ takes an `{ exporter }` options object, earlier versions the positional exporter), and a re-entrancy guard was added to the `cds.log.format` interception
 - Cloud SDK outbound requests are traced again (patch getter-only `@sap-cloud-sdk/http-client` exports via `Object.defineProperty`)
 - Raw SQL no longer leaks into HANA INSERT `prepare` span names (now uses operation + table, matching SELECT)

README.md

@@ -96,7 +96,7 @@ Example trace in Dynatrace:
 
 An example trace printed to the console can be found in [`telemetry-to-console`](#telemetry-to-console).
 
-To export traces to Dynatrace, use the predefined kind [`telemetry-to-dynatrace`](#telemetry-to-dynatrace) with the OTLP trace exporter — see [Dynatrace OneAgent](#dynatrace-oneagent) for why this is required even when OneAgent is present.
+To export traces to Dynatrace, use the predefined kind [`telemetry-to-dynatrace`](#telemetry-to-dynatrace). If [Dynatrace OneAgent](#dynatrace-oneagent) is present, the CDS spans can alternatively be captured in-process by OneAgent, without the OTLP trace exporter.
 
 
 ### Metrics
@@ -251,10 +251,9 @@ In Dynatrace itself, you need to ensure that the following two features are enab
 
 #### Dynatrace OneAgent
 
-If [Dynatrace OneAgent](https://www.dynatrace.com/platform/oneagent) is present, for example on SAP BTP CF, do **not** rely on it to transport the spans created by `@cap-js/telemetry`.
-**Until further notice, traces must be exported to Dynatrace via the OTLP exporter**: `@opentelemetry/exporter-trace-otlp-proto` must be a dependency (as listed under [Required additional dependencies](#telemetry-to-dynatrace) above) and the `openTelemetryTrace.ingest` scope must be granted.
+If [Dynatrace OneAgent](https://www.dynatrace.com/platform/oneagent) is present, for example on SAP BTP CF, and the OTLP trace exporter is *not* installed, `@cap-js/telemetry` does not export traces itself. Instead it registers a recording tracer provider without an exporter and relies on OneAgent's in-process OpenTelemetry capture to pick up the CDS spans — no OTLP export, no duplicate spans.
 
-If OneAgent is active but the OTLP trace exporter is *not* installed, `@cap-js/telemetry` exports no traces and logs a warning at startup — only OneAgent's own instrumentation is captured, not the CDS spans.
+Installing the OTLP trace exporter (`@opentelemetry/exporter-trace-otlp-proto`, see [Required additional dependencies](#telemetry-to-dynatrace) above) takes precedence: `@cap-js/telemetry` then exports the spans itself instead of relying on in-process capture, and the `openTelemetryTrace.ingest` scope must be granted.
 
 
 ### `telemetry-to-cloud-logging`

lib/tracing/index.js

@@ -160,53 +160,56 @@ module.exports = resource => {
   require('./cds')()
   require('./cloud_sdk')()
 
-  // Dynatrace OneAgent present, kind targets Dynatrace, and no OTLP trace exporter is installed.
+  // On the OneAgent path — Dynatrace OneAgent present (DT_NODE_PRELOAD_OPTIONS), kind *-to-dynatrace,
+  // no OTLP trace exporter installed — we don't export spans ourselves: OneAgent captures them
+  // in-process, and exporting would only duplicate them. So no span processor is built on this path;
+  // everywhere else we build one around the configured exporter.
   const via_one_agent =
     process.env.DT_NODE_PRELOAD_OPTIONS &&
     cds.env.requires.telemetry.kind.match(/to-dynatrace$/) &&
     !hasDependency('@opentelemetry/exporter-trace-otlp-proto')
-  if (via_one_agent) {
-    // No exporter is configured on this path, so there is nothing to build a span processor
-    // around — passing an undefined processor to the provider would crash on the first span.
-    // Set up nothing and return; @cap-js/telemetry does not export traces here. To get traces
-    // into Dynatrace, install @opentelemetry/exporter-trace-otlp-proto, which flips this flag
-    // off and takes the OTLP export path below.
-    LOG.warn(
-      'Dynatrace OneAgent detected without an OTLP trace exporter: @cap-js/telemetry will not export traces. ' +
-        'Add @opentelemetry/exporter-trace-otlp-proto to export traces to Dynatrace.'
-    )
-    return
-  }
+
+  if (!resource && via_one_agent)
+    throw new Error('Dynatrace OneAgent with @sap/xotel-agent-ext-js (CALM) is not supported')
 
   /*
    * create processor
    */
-  const exporter = _getExporter()
-  const { kind: processorKind, config: processorConfig = {} } = cds.env.requires.telemetry.tracing.processor || {}
-  const processors = { BatchSpanProcessor, SimpleSpanProcessor }
-  if (!processors[processorKind]) throw new Error(`Unknown span processor ${processorKind}`)
-  const processor = new processors[processorKind](exporter, processorConfig)
+  let processor
+  if (via_one_agent) {
+    LOG._info && LOG.info('Dynatrace OneAgent detected: relying on in-process trace capture')
+  } else {
+    const exporter = _getExporter()
+    const { kind: processorKind, config: processorConfig = {} } = cds.env.requires.telemetry.tracing.processor || {}
+    const processors = { BatchSpanProcessor, SimpleSpanProcessor }
+    if (!processors[processorKind]) throw new Error(`Unknown span processor ${processorKind}`)
+    processor = new processors[processorKind](exporter, processorConfig)
+  }
 
   /*
    * either add processor as delegate in CALM...
    */
   if (!resource) {
-    LOG.warn("@sap/xotel-agent-ext-js found, adding @cap-js/telemetry's span processor as delegate")
+    LOG._info && LOG.info("@sap/xotel-agent-ext-js found, adding @cap-js/telemetry's span processor as delegate")
     try {
       const { getCompositeSpanProcessor } = require('@sap/xotel-agent-ext-js')
       getCompositeSpanProcessor().addDelegate(processor)
-      return
     } catch (error) {
       LOG.error('Failed to add span processor as delegate:', error)
       throw error
     }
+    return
   }
 
   /*
    * ... or initialize and return provider
    */
   resource = resourceFromAttributes({}).merge(resource).merge(getDynatraceMetadata())
-  const tracerProvider = new NodeTracerProvider({ resource, spanProcessors: [processor], sampler: _getSampler() })
+  const tracerProvider = new NodeTracerProvider({
+    resource,
+    spanProcessors: processor ? [processor] : [],
+    sampler: _getSampler()
+  })
   tracerProvider.register({ propagator: _getPropagator() })
   return tracerProvider
 }

test/tracing-one-agent.test.js

@@ -1,24 +1,23 @@
-// Regression test: when Dynatrace OneAgent is active (`via_one_agent`), the tracing factory must
-// NOT build a tracer provider with an undefined span processor.
+// When Dynatrace OneAgent is active (`via_one_agent`), OneAgent captures OpenTelemetry spans
+// in-process — so instead of exporting traces ourselves (which would duplicate them), the tracing
+// factory registers a real, recording tracer provider with an EMPTY span-processor list. That
+// provider records spans (so the global tracer used by lib/tracing/trace.js is no longer a no-op)
+// but adds no export path of our own; OneAgent's preload hooks observe the recorded spans.
 //
-// 2.x constructs the provider via the OTel 2.0 constructor `new NodeTracerProvider({ spanProcessors })`.
-// On the OneAgent path no exporter/processor is created, so `processor` stayed `undefined` and
-// `[undefined]` was handed to the provider — MultiSpanProcessor.onStart then dereferenced undefined
-// on the FIRST span and the app crashed on startup (CF exit 137, crash loop).
-//
-// The fix: on the OneAgent path the factory sets up nothing and returns — no exporter, no processor,
-// no provider, nothing to crash on. It deliberately does not export traces here (export to Dynatrace
-// goes via the OTLP exporter instead) and must not register or clobber a global tracer provider.
+// Two properties must hold and are asserted here without a live Dynatrace tenant:
+//   1. crash-safety — an EMPTY processor list `[]` must not crash on the first span, unlike the
+//      `[undefined]` a naive "no exporter -> no processor" path would have produced (which crashed
+//      in MultiSpanProcessor.onStart);
+//   2. the provider is real and RECORDING — a provider that doesn't record would leave CDS spans
+//      non-existent, which is the regression this path must avoid.
 //
 // We drive the factory directly rather than through a full boot: exercising `via_one_agent` needs
-// kind `*-to-dynatrace` (whose metrics/tracing exporters would otherwise demand real Dynatrace
-// credentials at boot). Booting once with the in-memory tracing profile populates `cds.env` and
-// caches lib/tracing; the tests then flip `cds.env.requires.telemetry.kind` + the env flag and call
-// the factory in isolation.
+// kind `*-to-dynatrace` (whose exporters would otherwise demand real Dynatrace credentials at boot).
+// Booting once with the in-memory tracing profile populates `cds.env` and caches lib/tracing; the
+// tests then flip `cds.env.requires.telemetry.kind` + the env flag and call the factory in isolation.
 const cds = require('@sap/cds')
 const { expect } = cds.test(__dirname + '/bookshop', '--profile', 'tracing-in-memory')
 
-const otel = require('@opentelemetry/api')
 const { resourceFromAttributes } = require('@opentelemetry/resources')
 const { NodeTracerProvider } = require('@opentelemetry/sdk-trace-node')
 
@@ -28,49 +27,64 @@ const setupTracing = require('../lib/tracing')
 describe('tracing setup with Dynatrace OneAgent', () => {
   const OTLP_PROTO = '@opentelemetry/exporter-trace-otlp-proto'
 
+  // Guard on the illustrative failure mode: a provider whose only span processor is `undefined`
+  // builds fine but crashes on the first span in onStart. It is the reason the OneAgent path passes
+  // an empty list `[]` rather than `[processor]` with a missing processor.
   test('a provider with an undefined span processor crashes on the first span', () => {
-    // This is the failure mode the OneAgent path must avoid: a provider whose only span processor
-    // is `undefined`. It builds fine, but the first span crashes in onStart.
     const provider = new NodeTracerProvider({
       resource: resourceFromAttributes({}),
       spanProcessors: [undefined]
     })
     expect(() => provider.getTracer('probe').startSpan('boom')).toThrow(/onStart/)
   })
 
-  test('OneAgent path: sets up nothing, returns, and never touches an undefined processor', () => {
-    // Precondition for `via_one_agent`: the otlp-proto exporter must NOT be a (production) dependency
-    // — it is only a devDependency here, and hasDependency() checks `dependencies` only.
+  // Precondition for `via_one_agent`: the otlp-proto exporter must NOT be a (production) dependency
+  // — it is only a devDependency here, and hasDependency() checks `dependencies` only. With it
+  // present, `via_one_agent` would be false and the normal OTLP export path taken instead.
+  test('otlp-proto exporter is not a production dependency (so via_one_agent can be true)', () => {
     expect(hasDependency(OTLP_PROTO)).toBe(false)
+  })
 
-    const proxy = otel.trace.getTracerProvider() // the process-global ProxyTracerProvider
-    const originalDelegate = proxy.getDelegate()
+  describe('OneAgent path', () => {
+    let savedKind, savedEnv
+
+    beforeEach(() => {
+      savedKind = cds.env.requires.telemetry.kind
+      savedEnv = process.env.DT_NODE_PRELOAD_OPTIONS
+      process.env.DT_NODE_PRELOAD_OPTIONS = '{}'
+      cds.env.requires.telemetry.kind = 'telemetry-to-dynatrace'
+    })
 
-    const savedKind = cds.env.requires.telemetry.kind
-    const savedEnv = process.env.DT_NODE_PRELOAD_OPTIONS
-    process.env.DT_NODE_PRELOAD_OPTIONS = '{}'
-    cds.env.requires.telemetry.kind = 'telemetry-to-dynatrace'
+    afterEach(() => {
+      cds.env.requires.telemetry.kind = savedKind
+      if (savedEnv === undefined) delete process.env.DT_NODE_PRELOAD_OPTIONS
+      else process.env.DT_NODE_PRELOAD_OPTIONS = savedEnv
+    })
 
-    try {
-      // Must not throw. We pass a resource so it is the standalone path (a falsy resource is the
-      // CALM path).
-      let returned
+    test('standalone: registers a real, recording provider with no export path, and does not crash', () => {
+      // A truthy resource is the standalone path (a falsy resource is the CALM path).
+      let provider
       expect(() => {
-        returned = setupTracing(resourceFromAttributes({}))
+        provider = setupTracing(resourceFromAttributes({}))
       }).not.toThrow()
 
-      // The factory returns nothing and registers no provider of its own — the global delegate is
-      // left exactly as it was.
-      expect(returned).toBeUndefined()
-      expect(proxy.getDelegate()).toBe(originalDelegate)
+      // A real provider is returned (not undefined as on the pre-fix regression) ...
+      expect(provider).toBeInstanceOf(NodeTracerProvider)
 
-      // And a span created via the global API (as lib/tracing/trace.js does for every CDS span)
-      // does not crash — with no real provider registered it is simply a non-recording span.
-      expect(() => otel.trace.getTracer('@cap-js/telemetry').startSpan('cds-span').end()).not.toThrow()
-    } finally {
-      cds.env.requires.telemetry.kind = savedKind
-      if (savedEnv === undefined) delete process.env.DT_NODE_PRELOAD_OPTIONS
-      else process.env.DT_NODE_PRELOAD_OPTIONS = savedEnv
-    }
+      // ... and it records: a span created the way lib/tracing/trace.js creates them is a real,
+      // recording span — not a NonRecordingSpan — so CDS spans actually come into existence for
+      // OneAgent to capture. The empty processor list means creating and ending it never crashes.
+      const span = provider.getTracer('@cap-js/telemetry').startSpan('cds-span')
+      expect(span.constructor.name).not.toBe('NonRecordingSpan')
+      expect(span.isRecording()).toBe(true)
+      expect(() => span.end()).not.toThrow()
+    })
+
+    test('CALM + OneAgent: rejected as an unsupported combination', () => {
+      // A falsy resource is the CALM path (@sap/xotel-agent-ext-js owns the provider). Combined with
+      // OneAgent, two agents would own tracing at once — a contradictory, unverified setup — so the
+      // factory rejects it rather than silently doing nothing.
+      expect(() => setupTracing(undefined)).toThrow(/OneAgent with @sap\/xotel-agent-ext-js .* not supported/)
+    })
   })
 })