From 74be8b820d59c83a25b75d4bddacdb5ca036e078 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Thu, 11 Jun 2026 12:16:34 +0200 Subject: [PATCH 1/2] feat: add startApiSpan helper for centralized dispatch sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complements `instrumentApiMethod` for consumers whose dispatch owns the span lifecycle directly (e.g. a Router method where the start + end points are visible together). `instrumentApiMethod`'s wrap-once- at-module-load shape fits flat dispatch tables (cloudserver `api[name]=handler`, backbeat handlers); for centralized dispatch the wrap costs an extra per-Route cache + a static-this `.bind` dance to amortize a wrap on a single call site. `startApiSpan(action)` returns: - `end(err?)` — reuses the existing exported `endSpan(span, err?)` so the `recordException` + status + `error.type` ceremony stays in one place; same err-as-optional convention. - `withContext(fn)` — runs `fn` with the span set as the active context so child auto-spans (mongo, ioredis, http) nest underneath. When OTEL is off, returns a no-op object; no `@opentelemetry/api` load on the disabled path. Issue: ARSN-592 --- lib/tracing/index.ts | 3 +- lib/tracing/instrumentation.ts | 30 ++++++ tests/unit/tracing/instrumentation.spec.js | 108 ++++++++++++++++++++- 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/lib/tracing/index.ts b/lib/tracing/index.ts index dc22e4dca..2fc598378 100644 --- a/lib/tracing/index.ts +++ b/lib/tracing/index.ts @@ -1,7 +1,8 @@ import * as bootstrap from './bootstrap'; export { makeHttpInstrumentationConfig } from './httpHooks'; -export { instrumentApiMethod, endSpan } from './instrumentation'; +export { instrumentApiMethod, endSpan, startApiSpan } from './instrumentation'; +export type { ApiSpan } from './instrumentation'; export type { InitOptions } from './bootstrap'; export * as kafka from './kafkaTraceContext'; diff --git a/lib/tracing/instrumentation.ts b/lib/tracing/instrumentation.ts index 07d401e05..8f52a2208 100644 --- a/lib/tracing/instrumentation.ts +++ b/lib/tracing/instrumentation.ts @@ -119,3 +119,33 @@ export function instrumentApiMethod any>(apiMethod return instrumentAsyncHandler(this, apiMethod, spanName, args); } as T; } + +// Manual span lifecycle for consumers whose dispatch owns the start + end sites +// directly (e.g. a centralized Router method). Prefer `instrumentApiMethod` for +// flat dispatch tables where wrap-once-at-module-load fits naturally. +export interface ApiSpan { + // `end()` marks the span OK; `end(err)` marks it ERROR + records the + // exception (mirrors the err-as-optional shape of `endSpan` above). + end(err?: any): void; + // Runs `fn` with the span set as the active context so child auto-spans + // (mongo, ioredis, http) nest underneath. + withContext(fn: () => T): T; +} + +const NO_OP_API_SPAN: ApiSpan = { + end: () => {}, + withContext: fn => fn(), +}; + +export function startApiSpan(action: string): ApiSpan { + if (!isEnabled()) { + return NO_OP_API_SPAN; + } + const { trace, context, SpanKind } = getApi(); + const span = getTracer().startSpan(`${SPAN_PREFIX}${action}`, { kind: SpanKind.INTERNAL }); + const ctx = trace.setSpan(context.active(), span); + return { + end: err => endSpan(span, err), + withContext: fn => context.with(ctx, fn), + }; +} diff --git a/tests/unit/tracing/instrumentation.spec.js b/tests/unit/tracing/instrumentation.spec.js index dccfc1f2c..81a59b605 100644 --- a/tests/unit/tracing/instrumentation.spec.js +++ b/tests/unit/tracing/instrumentation.spec.js @@ -1,7 +1,8 @@ 'use strict'; const assert = require('assert'); -const { trace, SpanStatusCode } = require('@opentelemetry/api'); +const { trace, context, SpanStatusCode } = require('@opentelemetry/api'); +const { AsyncLocalStorageContextManager } = require('@opentelemetry/context-async-hooks'); const { BasicTracerProvider, InMemorySpanExporter, @@ -9,7 +10,7 @@ const { AlwaysOnSampler, } = require('@opentelemetry/sdk-trace-base'); -const { instrumentApiMethod, resetTracer } = require('../../../lib/tracing/instrumentation'); +const { instrumentApiMethod, startApiSpan, resetTracer } = require('../../../lib/tracing/instrumentation'); describe('instrumentApiMethod', () => { let exporter; @@ -155,3 +156,106 @@ describe('instrumentApiMethod', () => { }); }); }); + +describe('startApiSpan', () => { + let exporter; + let provider; + let contextManager; + + beforeAll(() => { + process.env.ENABLE_OTEL = 'true'; + exporter = new InMemorySpanExporter(); + provider = new BasicTracerProvider({ + sampler: new AlwaysOnSampler(), + spanProcessors: [new SimpleSpanProcessor(exporter)], + }); + trace.setGlobalTracerProvider(provider); + // Default ContextManager is a no-op (returns ROOT), so context.with + // wouldn't actually propagate. Real-world NodeSDK installs one; + // mirror that for the withContext assertions below. + contextManager = new AsyncLocalStorageContextManager().enable(); + context.setGlobalContextManager(contextManager); + resetTracer(); + }); + + afterAll(async () => { + delete process.env.ENABLE_OTEL; + resetTracer(); + contextManager.disable(); + context.disable(); + await provider.shutdown(); + trace.disable(); + }); + + describe('OTEL on', () => { + afterEach(() => exporter.reset()); + + it('should end with status OK on end() with no error', () => { + const span = startApiSpan('AuthV4'); + span.end(); + + const spans = exporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assert.strictEqual(spans[0].name, 'api.AuthV4'); + assert.strictEqual(spans[0].status.code, SpanStatusCode.OK); + }); + + it('should end with status ERROR + error.type on end(err)', () => { + const span = startApiSpan('AssumeRole'); + const err = Object.assign(new Error('denied'), { code: 'AccessDenied' }); + span.end(err); + + const spans = exporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assert.strictEqual(spans[0].status.code, SpanStatusCode.ERROR); + assert.strictEqual(spans[0].attributes['error.type'], 'AccessDenied'); + }); + + it('should set the started span as the active context inside withContext(fn)', () => { + // Run inside an outer span so the active span is not the same as + // ours before withContext fires — that's how we tell the inner + // span is the one that propagated through context.with. + const outerSpan = trace.getTracer('outer').startSpan('outer'); + context.with(trace.setSpan(context.active(), outerSpan), () => { + const before = trace.getActiveSpan(); + const span = startApiSpan('CheckPolicies'); + let inside; + span.withContext(() => { + inside = trace.getActiveSpan(); + }); + const after = trace.getActiveSpan(); + span.end(); + + assert.strictEqual(before, outerSpan); + assert.ok(inside, 'expected an active span inside withContext'); + assert.notStrictEqual(inside, outerSpan); + assert.strictEqual(after, outerSpan); + }); + outerSpan.end(); + }); + + it('should return the value `fn` returns from withContext', () => { + const span = startApiSpan('GetCallerIdentity'); + const value = span.withContext(() => 42); + span.end(); + assert.strictEqual(value, 42); + }); + }); + + describe('OTEL off', () => { + afterEach(() => { + process.env.ENABLE_OTEL = 'true'; + }); + + it('returns a no-op span — end()/end(err)/withContext do not throw', () => { + process.env.ENABLE_OTEL = 'false'; + const span = startApiSpan('AuthV4'); + assert.doesNotThrow(() => span.end()); + assert.doesNotThrow(() => span.end(new Error('boom'))); + assert.strictEqual( + span.withContext(() => 'value'), + 'value', + ); + }); + }); +}); From bf94ffc8c2608746c03d54e4ee112fac80e9b1c4 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Thu, 11 Jun 2026 12:32:29 +0200 Subject: [PATCH 2/2] Bump arsenal to 8.4.7 Issue: ARSN-592 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a87a9c512..49cc836bb 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "engines": { "node": ">=20" }, - "version": "8.4.6", + "version": "8.4.7", "description": "Common utilities for the S3 project components", "main": "build/index.js", "repository": {