From e99cb5206f01f0562a0eb3ee87cd112e6c6149a6 Mon Sep 17 00:00:00 2001 From: Nikhil Pareek Date: Mon, 2 Nov 2026 02:45:59 -0800 Subject: [PATCH 1/4] fix(openai): repair recalled TypeScript SDK contracts Handle custom tools and optional Responses inputs, protect duplicate private payloads, and preserve stream abort, failure and usage semantics. Add floor/latest SDK and shared-core regressions. Co-Authored-By: Claude Code --- .github/workflows/openai-sdk-contracts.yml | 24 +- .../__tests__/duplicatePayloadPrivacy.test.ts | 50 ++++ .../src/trace/trace-config/maskingRules.ts | 19 +- typescript/packages/traceai_openai/README.md | 8 +- .../src/__tests__/openai.test.ts | 10 +- .../src/__tests__/providerHostsWire.test.ts | 6 +- .../src/__tests__/responseAttributes.test.ts | 4 +- .../traceai_openai/src/instrumentation.ts | 141 ++++++++++-- .../traceai_openai/src/responseAttributes.ts | 6 +- .../traceai_openai/test/currentSdk.cjs | 2 +- .../traceai_openai/test/currentSdkShapes.mts | 21 ++ .../traceai_openai/test/privacyContext.cjs | 67 ++++++ .../traceai_openai/test/recallRegressions.cjs | 213 ++++++++++++++++++ 13 files changed, 521 insertions(+), 50 deletions(-) create mode 100644 typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts create mode 100644 typescript/packages/traceai_openai/test/currentSdkShapes.mts create mode 100644 typescript/packages/traceai_openai/test/privacyContext.cjs create mode 100644 typescript/packages/traceai_openai/test/recallRegressions.cjs diff --git a/.github/workflows/openai-sdk-contracts.yml b/.github/workflows/openai-sdk-contracts.yml index 08453fd5c..fdac066ea 100644 --- a/.github/workflows/openai-sdk-contracts.yml +++ b/.github/workflows/openai-sdk-contracts.yml @@ -46,10 +46,12 @@ jobs: pnpm run build:deps pnpm --filter @traceai/openai run build pnpm --dir packages/traceai_openai pack --out "$RUNNER_TEMP/openai.tgz" + pnpm --dir packages/fi-core pack --out "$RUNNER_TEMP/core.tgz" npm install --prefix "$RUNNER_TEMP/consumer" --ignore-scripts --no-audit --no-fund \ "$RUNNER_TEMP/openai.tgz" "openai@${{ matrix.sdk }}" \ - @traceai/fi-core@1.1.0 @traceai/fi-semantic-conventions@1.0.0 \ - @opentelemetry/sdk-trace-base@2.0.1 @opentelemetry/exporter-trace-otlp-http@0.202.0 \ + "$RUNNER_TEMP/core.tgz" @traceai/fi-semantic-conventions@1.0.0 \ + @opentelemetry/sdk-trace-base@2.0.1 @opentelemetry/context-async-hooks@2.0.1 \ + @opentelemetry/exporter-trace-otlp-http@0.202.0 \ @opentelemetry/exporter-trace-otlp-proto@0.202.0 @opentelemetry/otlp-transformer@0.202.0 \ typescript@5.8.3 @types/node@22.15.29 - name: Network-isolated automatic and manual SDK contracts @@ -66,6 +68,22 @@ jobs: done done done + - name: Network-isolated recall regressions + env: + OPENAI_CONSUMER_DIR: ${{ runner.temp }}/consumer + run: | + sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ + env PATH="$PATH" OPENAI_CONSUMER_DIR="$OPENAI_CONSUMER_DIR" \ + node --test packages/traceai_openai/test/recallRegressions.cjs \ + packages/traceai_openai/test/privacyContext.cjs + - name: Typecheck current SDK request contracts + if: matrix.sdk == '7.47.0' + env: + OPENAI_CONSUMER_DIR: ${{ runner.temp }}/consumer + run: | + cp packages/traceai_openai/test/currentSdkShapes.mts "$OPENAI_CONSUMER_DIR/" + "$OPENAI_CONSUMER_DIR/node_modules/.bin/tsc" --noEmit --module nodenext --target es2022 \ + "$OPENAI_CONSUMER_DIR/currentSdkShapes.mts" - name: Network-isolated promise and provider regressions env: OPENAI_SDK_PATH: ${{ runner.temp }}/consumer/node_modules/openai @@ -97,3 +115,5 @@ jobs: sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ env PATH="$PATH" node packages/traceai_openai/node_modules/jest/bin/jest.js \ --config packages/traceai_openai/jest.config.js --runInBand + sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ + env PATH="$PATH" pnpm --filter @traceai/fi-core test --runInBand diff --git a/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts b/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts new file mode 100644 index 000000000..4ab7bc7b8 --- /dev/null +++ b/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts @@ -0,0 +1,50 @@ +import { BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor } from "@opentelemetry/sdk-trace-base"; +import { SemanticConventions as SC } from "@traceai/fi-semantic-conventions"; +import { FITracer } from "../trace/trace-config/FITracer"; +import { TraceConfigOptions } from "../trace/trace-config/types"; + +const cases: [keyof TraceConfigOptions, string[]][] = [ + ["hideInputs", [`${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`, `${SC.EMBEDDING_EMBEDDINGS}.12.${SC.EMBEDDING_TEXT}`]], + ["hideInputText", [SC.INPUT_VALUE, SC.RAW_INPUT, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`]], + ["hideInputImages", [SC.INPUT_VALUE, SC.RAW_INPUT]], + ["hideInputMessages", [SC.INPUT_VALUE, SC.RAW_INPUT]], + ["hideOutputText", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT]], + ["hideOutputMessages", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT]], + ["hideEmbeddingVectors", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_VECTOR}`]], +]; + +describe("privacy covers duplicate payload attributes", () => { + const exporter = new InMemorySpanExporter(); + const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); + afterEach(() => exporter.reset()); + afterAll(() => provider.shutdown()); + + for (const [flag, keys] of cases) { + for (const placement of ["creation", "setter", "link"] as const) { + it(`${flag} protects ${placement} attributes without removing safe metadata`, () => { + const attributes = Object.fromEntries(keys.map(key => [key, "SYNTHETIC-PRIVATE-PAYLOAD"])); + const tracer = new FITracer({ tracer: provider.getTracer("privacy"), traceConfig: { [flag]: true } }); + const span = tracer.startSpan("test", { + attributes: { [SC.LLM_MODEL_NAME]: "synthetic-model", ...(placement === "creation" ? attributes : {}) }, + links: placement === "link" ? [{ context: { traceId: "1".repeat(32), spanId: "2".repeat(16), traceFlags: 1 }, attributes }] : [], + }); + if (placement === "setter") { + span.setAttributes(attributes); + for (const [key, value] of Object.entries(attributes)) span.setAttribute(key, value); + } + span.end(); + const finished = exporter.getFinishedSpans()[0]; + expect(JSON.stringify({ attributes: finished.attributes, links: finished.links })).not.toContain("SYNTHETIC-PRIVATE-PAYLOAD"); + expect(finished.attributes[SC.LLM_MODEL_NAME]).toBe("synthetic-model"); + }); + } + } + + it("preserves all payloads with hiding disabled", () => { + const attributes = Object.fromEntries(cases.flatMap(([, keys]) => keys.map(key => [key, "SYNTHETIC-PUBLIC-PAYLOAD"]))); + const tracer = new FITracer({ tracer: provider.getTracer("privacy"), traceConfig: {} }); + const span = tracer.startSpan("test", { attributes }); + span.end(); + expect(exporter.getFinishedSpans()[0].attributes).toEqual(attributes); + }); +}); diff --git a/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts b/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts index 488d07dfa..d20b64677 100644 --- a/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts +++ b/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts @@ -109,14 +109,18 @@ const maskingRules: MaskingRule[] = [ action: () => undefined, }, { + // Raw/value payloads are opaque duplicates. Do not attempt provider-specific + // JSON parsing here: granular hiding must also cover every copied payload. condition: ({ config, key }) => - config.hideInputs && key === SemanticConventions.INPUT_VALUE, + (config.hideInputs || config.hideInputText || config.hideInputImages || config.hideInputMessages) && + (key === SemanticConventions.INPUT_VALUE || key === SemanticConventions.RAW_INPUT), action: () => REDACTED_VALUE as AttributeValue, }, { - // The raw request body (e.g. the full OpenAI request, messages included), like input.value condition: ({ config, key }) => - config.hideInputs && key === SemanticConventions.RAW_INPUT, + (config.hideInputs || config.hideInputText) && + key.startsWith(`${SemanticConventions.EMBEDDING_EMBEDDINGS}.`) && + key.endsWith(`.${SemanticConventions.EMBEDDING_TEXT}`), action: () => REDACTED_VALUE as AttributeValue, }, { @@ -132,13 +136,8 @@ const maskingRules: MaskingRule[] = [ }, { condition: ({ config, key }) => - config.hideOutputs && key === SemanticConventions.OUTPUT_VALUE, - action: () => REDACTED_VALUE as AttributeValue, - }, - { - // The raw response body, like output.value - condition: ({ config, key }) => - config.hideOutputs && key === SemanticConventions.RAW_OUTPUT, + (config.hideOutputs || config.hideOutputText || config.hideOutputMessages || config.hideEmbeddingVectors) && + (key === SemanticConventions.OUTPUT_VALUE || key === SemanticConventions.RAW_OUTPUT), action: () => REDACTED_VALUE as AttributeValue, }, { diff --git a/typescript/packages/traceai_openai/README.md b/typescript/packages/traceai_openai/README.md index cf35b28bc..9459b06a1 100644 --- a/typescript/packages/traceai_openai/README.md +++ b/typescript/packages/traceai_openai/README.md @@ -98,7 +98,7 @@ try { Use Perplexity's [OpenAI-compatible Responses API](https://docs.perplexity.ai/docs/agent-api/openai-compatibility) with the shared instrumentor. This route does not instrument the native Perplexity SDK or its Search and Embeddings resources. ```bash -npm install @traceai/openai @traceai/fi-core@1.1.0 openai@7.47.0 +npm install @traceai/openai @traceai/fi-core openai@7.47.0 export PERPLEXITY_API_KEY="your-perplexity-api-key" export PERPLEXITY_MODEL="your-perplexity-model-id" export FI_API_KEY="your-futureagi-api-key" @@ -154,7 +154,7 @@ For ESM, explicitly instrument the imported SDK as above. For CommonJS automatic Use the same instrumentor for the gateway's OpenAI-compatible client; do not add a second gateway instrumentor. This non-streaming Chat Completions recipe uses `openai@5.23.2`. The instrumentor supports OpenAI 4.x through 7.x. For applications using `ai` instead of `openai`, use the [Vercel AI SDK integration](https://docs.futureagi.com/docs/integrations/traceai/vercel). ```bash -npm install @traceai/openai @traceai/fi-core@1.1.0 openai@5.23.2 +npm install @traceai/openai @traceai/fi-core openai@5.23.2 export AI_GATEWAY_API_KEY="your-vercel-gateway-key" export AI_GATEWAY_MODEL="provider/model" export FI_API_KEY="your-futureagi-api-key" @@ -221,7 +221,9 @@ const response = await client.chat.completions.create({ Choose a model available to your gateway account. `gen_ai.provider.name` comes from the configured API hostname, not the model name: `api.anannas.ai` and `anannas.ai` produce `anannas`, including failed and streamed requests. The gateway's downstream routing is not inferred. Unknown proxies and local servers retain the historical `openai` fallback; this does not establish which provider they route to. URL credentials, paths and query strings are not added as provider attributes. -Streaming spans finish when you consume the stream, encounter an iteration error or leave the loop early. Consume or close streams before shutting down the tracer provider. Request and response content is captured by default; use the instrumentation's `traceConfig` input/output and error-detail hiding options for sensitive data. +Streaming spans finish when you consume the stream, encounter an iteration error, leave the loop early or abort the stream's controller—even before iteration starts. A request signal aborted with a `TimeoutError` records an error without changing the SDK's cancellation behavior. An intentional early exit is not recorded as a successful generation. Consume or abort streams before shutting down the tracer provider. Reported streaming usage is recorded when the stream completes; absent usage is not zero. + +Request and response content is captured by default. Upgrade `@traceai/openai` and `@traceai/fi-core` together for shared privacy controls. `FI_HIDE_INPUTS` also hides embedding input text. Granular input text/image/message flags redact the opaque `input.value` and `raw.input` copies; output text/message and embedding-vector flags redact `output.value` and `raw.output`. Structured attributes retain the fields allowed by their respective settings. Responses instructions are treated as input, not invocation parameters, and echoed request fields are excluded from output payloads. Tool definitions and custom attributes still require their own privacy review. ## Fireworks AI diff --git a/typescript/packages/traceai_openai/src/__tests__/openai.test.ts b/typescript/packages/traceai_openai/src/__tests__/openai.test.ts index b7cd1bd69..944fb6b42 100644 --- a/typescript/packages/traceai_openai/src/__tests__/openai.test.ts +++ b/typescript/packages/traceai_openai/src/__tests__/openai.test.ts @@ -755,8 +755,11 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"","tool_calls":[{"id":"call_PGkcUg2u6vYrCpTn0e9ofykY","function":{"name":"getWeather","arguments":"{}"}}]}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo", + "gen_ai.request.model": "gpt-3.5-turbo-0613", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"stream":true}", + "gen_ai.response.finish_reasons": "["tool_calls"]", + "gen_ai.response.id": "chatcmpl-8iA39kCtuVHIVDr9AnBdJZjgSjNWL", + "gen_ai.response.model": "gpt-3.5-turbo-0613", "gen_ai.tool.definitions": "[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}]", "input.mime_type": "application/json", "input.value": "{"messages":[{"role":"user","content":"What's the weather today?"}],"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"stream":true}", @@ -868,8 +871,11 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"","function_call_name":"getWeather","function_call_arguments":"{}"}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo", + "gen_ai.request.model": "gpt-3.5-turbo-0613", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","functions":[{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}},{"name":"getCurrentLocation","description":"Get the current location of the user.","parameters":{"type":"object","properties":{}}}],"stream":true}", + "gen_ai.response.finish_reasons": "["function_call"]", + "gen_ai.response.id": "chatcmpl-8iA39kCtuVHIVDr9AnBdJZjgSjNWL", + "gen_ai.response.model": "gpt-3.5-turbo-0613", "input.mime_type": "application/json", "input.value": "{"messages":[{"role":"user","content":"What's the weather today?"}],"model":"gpt-3.5-turbo","functions":[{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}},{"name":"getCurrentLocation","description":"Get the current location of the user.","parameters":{"type":"object","properties":{}}}],"stream":true}", "output.mime_type": "text/plain", diff --git a/typescript/packages/traceai_openai/src/__tests__/providerHostsWire.test.ts b/typescript/packages/traceai_openai/src/__tests__/providerHostsWire.test.ts index 524ad40dd..4dac1119e 100644 --- a/typescript/packages/traceai_openai/src/__tests__/providerHostsWire.test.ts +++ b/typescript/packages/traceai_openai/src/__tests__/providerHostsWire.test.ts @@ -114,11 +114,7 @@ describe("provider attribution on the OTLP wire", () => { expect(spans[0].attributes.find(({ key }) => key === "gen_ai.provider.name")?.value.stringValue).toBe(expected); if (error) expect(spans[0].status.code).toBe(2); expect(JSON.stringify(spans)).not.toMatch(/synthetic failure|synthetic-unused-key|url-user|url-password|query-secret/); - // Embedding text has a separate, pre-existing fi-core masking gap. This - // change does not claim hideInputs covers embedding.embeddings.*.text. - if (spans[0].name !== "OpenAI Embeddings") { - expect(JSON.stringify(spans)).not.toContain("Hello"); - } + expect(JSON.stringify(spans)).not.toContain("Hello"); } for (const operation of operations) { diff --git a/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts b/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts index d0fef0b6c..ed2dbc585 100644 --- a/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts +++ b/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts @@ -249,7 +249,9 @@ describe('Response Attributes Functions', () => { it('should handle missing input field', () => { const body = {}; - expect(() => getResponsesInputMessagesAttributes(body as any)).toThrow(); + expect(getResponsesInputMessagesAttributes(body as any)).toEqual({ + [SemanticConventions.LLM_INPUT_MESSAGES]: "[]", + }); }); }); diff --git a/typescript/packages/traceai_openai/src/instrumentation.ts b/typescript/packages/traceai_openai/src/instrumentation.ts index d2ccb8d71..98f3d2f95 100644 --- a/typescript/packages/traceai_openai/src/instrumentation.ts +++ b/typescript/packages/traceai_openai/src/instrumentation.ts @@ -299,6 +299,7 @@ export class OpenAIInstrumentation extends InstrumentationBase{ } else { result = observeStream(result, span, (chunks) => consumeChatCompletionStreamChunks(chunks, span), + args[1]?.signal, ); } return result; @@ -383,12 +384,13 @@ export class OpenAIInstrumentation extends InstrumentationBase{ } else { result = observeStream(result, span, (chunks) => { span.setAttributes({ + ...getStreamMetadataAttributes(chunks), [SemanticConventions.OUTPUT_VALUE]: chunks.map((chunk) => chunk.choices[0]?.text ?? "").join(""), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.TEXT, [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(chunks) ?? "", }); span.setStatus({ code: SpanStatusCode.OK }); - }); + }, args[1]?.signal); } return result; }, (error) => endSpanWithError(span, error), (response) => endRawResponseSpan(span, response)); @@ -485,7 +487,7 @@ export class OpenAIInstrumentation extends InstrumentationBase{ ...args: Parameters ) { const body = args[0]; - const { input: _messages, ...invocationParameters } = body; + const { input: _messages, instructions: _instructions, ...invocationParameters } = body; const span = instrumentation.fiTracer.startSpan( `OpenAI Responses`, { @@ -535,17 +537,27 @@ export class OpenAIInstrumentation extends InstrumentationBase{ if (endSpan) span.end(); return; } + const output = { + id: result.id, object: result.object, status: result.status, + model: result.model, output: result.output, usage: result.usage, + }; span.setAttributes({ - [SemanticConventions.OUTPUT_VALUE]: JSON.stringify(result), + [SemanticConventions.OUTPUT_VALUE]: JSON.stringify(output), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.JSON, [SemanticConventions.LLM_MODEL_NAME]: result.model, [SemanticConventions.GEN_AI_RESPONSE_MODEL]: result.model, [SemanticConventions.GEN_AI_RESPONSE_ID]: result.id, ...getResponsesOutputMessagesAttributes(result), ...getResponsesUsageAttributes(result), - [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(result) ?? "", + [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(output) ?? "", }); - span.setStatus({ code: SpanStatusCode.OK }); + if (["failed", "incomplete", "cancelled"].includes(result.status ?? "") || result.error) { + const message = result.error?.message ?? `Response ${result.status}`; + span.recordException(new Error(message)); + span.setStatus({ code: SpanStatusCode.ERROR, message }); + } else { + span.setStatus({ code: SpanStatusCode.OK }); + } if (endSpan) span.end(); }; if (isResponseCreateResponse(result)) { @@ -557,7 +569,8 @@ export class OpenAIInstrumentation extends InstrumentationBase{ async (chunks) => { recordSpan(await consumeResponseStreamEvents(chunks), false); }, - (event) => event.type === "response.completed", + args[1]?.signal, + (event) => event.type === "response.completed" || event.type === "response.failed" || event.type === "response.incomplete", ); } @@ -615,6 +628,7 @@ function observeStream( stream: Stream, span: Span, onComplete: (chunks: T[]) => void | Promise, + requestSignal?: AbortSignal | null, retainChunk: (chunk: T) => boolean = () => true, ): Stream { // Modern SDK helpers (notably tee) call the iterator factory directly. @@ -624,21 +638,67 @@ function observeStream( const originalIterator = hasIteratorFactory ? sdkStream.iterator!.bind(stream) : stream[Symbol.asyncIterator].bind(stream); + const signals = [stream.controller?.signal, requestSignal].filter( + (signal): signal is AbortSignal => signal != null, + ); + let ended = false; + let awaitingNext = false; + const finish = () => { + if (ended) return; + ended = true; + for (const signal of signals) signal.removeEventListener("abort", onAbort); + span.end(); + }; + const onAbort = () => { + if (ended) return; + // The SDK swallows AbortError and does not forward the request's reason to + // its own controller. Inspect the original signal before treating EOF as OK. + const deadline = signals.find(signal => signal.aborted && signal.reason?.name === "TimeoutError"); + if (deadline) { + span.recordException(deadline.reason); + span.setStatus({ code: SpanStatusCode.ERROR, message: deadline.reason.message }); + } + // An SDK parser/transport error can abort its controller in finally before + // next() rejects. Let that rejection reach our catch before ending the span. + if (!deadline && awaitingNext) return; + // Other aborts include the SDK's automatic cancellation on a consumer's + // early return. End them as UNSET, not successful generations or failures. + finish(); + }; + for (const signal of signals) signal.addEventListener("abort", onAbort, { once: true }); + if (signals.some(signal => signal.aborted)) onAbort(); const observedIterator = async function* () { const chunks: T[] = []; try { - for await (const chunk of { [Symbol.asyncIterator]: originalIterator }) { - if (retainChunk(chunk)) chunks.push(chunk); + const source = { + [Symbol.asyncIterator]: () => { + const iterator = originalIterator(); + return { + next: async () => { + awaitingNext = true; + try { return await iterator.next(); } + finally { awaitingNext = false; } + }, + return: iterator.return?.bind(iterator), + throw: iterator.throw?.bind(iterator), + }; + }, + }; + for await (const chunk of source) { + if (!ended && retainChunk(chunk)) chunks.push(chunk); yield chunk; } - await onComplete(chunks); + if (signals.some(signal => signal.aborted)) onAbort(); + if (!ended) await onComplete(chunks); } catch (error) { - const exception = error instanceof Error ? error : new Error(String(error)); - span.recordException(exception); - span.setStatus({ code: SpanStatusCode.ERROR, message: exception.message }); + if (!ended) { + const exception = error instanceof Error ? error : new Error(String(error)); + span.recordException(exception); + span.setStatus({ code: SpanStatusCode.ERROR, message: exception.message }); + } throw error; } finally { - span.end(); + finish(); } }; if (hasIteratorFactory) sdkStream.iterator = observedIterator; @@ -729,6 +789,24 @@ function getLLMToolsJSONSchema( }; } +// Structural shape keeps the build compatible with SDK 4 while accepting the +// discriminated custom-tool union introduced in later SDKs. Unknown variants +// retain identity only; tracing must not make a valid SDK request throw. +function serializeToolCall(tc: { + id: string; + type: string; + function?: { name: string; arguments: string }; + custom?: { name: string; input: string }; +}): Record { + const result: Record = { id: tc.id, type: tc.type }; + if (tc.type === "function" && tc.function) { + result.function = { name: tc.function.name, arguments: tc.function.arguments }; + } else if (tc.type === "custom" && tc.custom) { + result.custom = { name: tc.custom.name, input: tc.custom.input }; + } + return result; +} + /** * Serializes a ChatCompletionMessageParam into a plain object for JSON blob format. */ @@ -754,11 +832,7 @@ function serializeChatCompletionInputMessage( switch (role) { case "assistant": if (message.tool_calls) { - obj.tool_calls = message.tool_calls.map((tc) => ({ - id: tc.id, - type: tc.type, - function: { name: tc.function.name, arguments: tc.function.arguments }, - })); + obj.tool_calls = message.tool_calls.map(serializeToolCall); } break; case "function": @@ -809,7 +883,7 @@ function getCompletionInputValueAndMimeType( * Get usage attributes */ function getUsageAttributes( - completion: ChatCompletion | Completion, + completion: Pick, ): Attributes { if (completion.usage) { const usageAttributes: Attributes = { @@ -833,6 +907,27 @@ function getUsageAttributes( return {}; } +/** Usage-only final chunks have no choices. Collect metadata independently of deltas. */ +function getStreamMetadataAttributes(chunks: (ChatCompletionChunk | Completion)[]): Attributes { + const attributes: Attributes = {}; + const finishReasons = new Map(); + for (const chunk of chunks) { + if (chunk.model) { + attributes[SemanticConventions.LLM_MODEL_NAME] = chunk.model; + attributes[SemanticConventions.GEN_AI_RESPONSE_MODEL] = chunk.model; + } + if (chunk.id) attributes[SemanticConventions.GEN_AI_RESPONSE_ID] = chunk.id; + if (chunk.usage) Object.assign(attributes, getUsageAttributes(chunk)); + for (const choice of chunk.choices) { + if (choice.finish_reason) finishReasons.set(choice.index, choice.finish_reason); + } + } + if (finishReasons.size) { + attributes[SemanticConventions.GEN_AI_RESPONSE_FINISH_REASONS] = JSON.stringify([...finishReasons.values()]); + } + return attributes; +} + /** * Converts the chat completion result to LLM output messages (JSON blob) */ @@ -860,11 +955,7 @@ function serializeChatCompletionOutputMessage( obj.content = message.content; } if (message.tool_calls) { - obj.tool_calls = message.tool_calls.map((tc) => ({ - id: tc.id, - type: tc.type, - function: { name: tc.function.name, arguments: tc.function.arguments }, - })); + obj.tool_calls = message.tool_calls.map(serializeToolCall); } if (message.function_call) { obj.function_call = { @@ -1008,12 +1099,14 @@ async function consumeChatCompletionStreamChunks( } const attributes: Attributes = { + ...getStreamMetadataAttributes(allChunks), [SemanticConventions.OUTPUT_VALUE]: streamResponse, [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.TEXT, [SemanticConventions.LLM_OUTPUT_MESSAGES]: safelyJSONStringify([outputMessage]) ?? "[]", [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(allChunks) ?? "", }; span.setAttributes(attributes); + span.setStatus({ code: SpanStatusCode.OK }); // --- ADD LOG --- // diag.debug(`@traceai/openai: consumeChatCompletionStreamChunks span ENDED: ${span.spanContext().spanId}`); } diff --git a/typescript/packages/traceai_openai/src/responseAttributes.ts b/typescript/packages/traceai_openai/src/responseAttributes.ts index c03a38fb1..3d1bf2793 100644 --- a/typescript/packages/traceai_openai/src/responseAttributes.ts +++ b/typescript/packages/traceai_openai/src/responseAttributes.ts @@ -93,7 +93,7 @@ export function getResponsesInputMessagesAttributes( } if (typeof body.input === "string") { items.push({ content: body.input, role: "user" }); - } else { + } else if (Array.isArray(body.input)) { items.push(...body.input); } const messages = items.map((item) => serializeResponseItemMessage(item)); @@ -136,7 +136,9 @@ export async function consumeResponseStreamEvents( for await (const event of stream) { switch (event.type) { - case "response.completed": { + case "response.completed": + case "response.failed": + case "response.incomplete": { response = event.response; break; } diff --git a/typescript/packages/traceai_openai/test/currentSdk.cjs b/typescript/packages/traceai_openai/test/currentSdk.cjs index 7cff1002d..e0407818d 100644 --- a/typescript/packages/traceai_openai/test/currentSdk.cjs +++ b/typescript/packages/traceai_openai/test/currentSdk.cjs @@ -43,7 +43,7 @@ function call(operation = 'responses', stream = false) { const args = { model: 'requested-model', ...(stream ? { stream: true } : {}) }; if (operation === 'chat') return client.chat.completions.create({ ...args, messages: [{ role: 'user', content: bodyMarker }] }); if (operation === 'completions') return client.completions.create({ ...args, prompt: bodyMarker }); - if (operation === 'embeddings') return client.embeddings.create({ ...args, input: 'synthetic embedding', encoding_format: 'float' }); + if (operation === 'embeddings') return client.embeddings.create({ ...args, input: bodyMarker, encoding_format: 'float' }); return client.responses.create({ ...args, input: bodyMarker }); } async function spans(count = 1, expectedProvider = 'perplexity') { diff --git a/typescript/packages/traceai_openai/test/currentSdkShapes.mts b/typescript/packages/traceai_openai/test/currentSdkShapes.mts new file mode 100644 index 000000000..d3ca01d82 --- /dev/null +++ b/typescript/packages/traceai_openai/test/currentSdkShapes.mts @@ -0,0 +1,21 @@ +import OpenAI from 'openai'; +// Compile-only witnesses against the installed released SDK. No vendor calls. +declare const client: OpenAI; +const storedPrompt: OpenAI.Responses.ResponseCreateParamsNonStreaming = { + model: 'synthetic', prompt: { id: 'pmpt_synthetic' }, +}; +const custom: OpenAI.Chat.Completions.ChatCompletionMessageCustomToolCall = { + id: 'call-synthetic', type: 'custom', custom: { name: 'code_exec', input: 'print(1)' }, +}; +const request: OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming = { + model: 'synthetic', + messages: [ + { role: 'assistant', content: null, tool_calls: [custom] }, + { role: 'tool', tool_call_id: custom.id, content: '1' }, + ], + tools: [{ type: 'custom', custom: { name: 'code_exec' } }], +}; +const stream: OpenAI.Completions.CompletionCreateParamsStreaming = { + model: 'synthetic', prompt: 'synthetic', stream: true, stream_options: { include_usage: true }, +}; +void [client.responses.create(storedPrompt), client.chat.completions.create(request), client.completions.create(stream)]; diff --git a/typescript/packages/traceai_openai/test/privacyContext.cjs b/typescript/packages/traceai_openai/test/privacyContext.cjs new file mode 100644 index 000000000..5a272c0a4 --- /dev/null +++ b/typescript/packages/traceai_openai/test/privacyContext.cjs @@ -0,0 +1,67 @@ +// Synthetic SDK calls only; run under the network-denying review sandbox. +const { test, after, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const path = require('node:path'); +const req = createRequire(path.resolve(process.env.OPENAI_CONSUMER_DIR, 'package.json')); +const OpenAI = req('openai'); +const { OpenAIInstrumentation } = req('@traceai/openai'); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req('@opentelemetry/sdk-trace-base'); +const { AsyncLocalStorageContextManager } = req('@opentelemetry/context-async-hooks'); +const { trace, context } = req('@opentelemetry/api'); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); +const tracer = provider.getTracer('recall'); +context.setGlobalContextManager(new AsyncLocalStorageContextManager().enable()); +let instrumentation; +function instrument(traceConfig = {}) { + instrumentation = new OpenAIInstrumentation({ instrumentationConfig: { enabled: false }, traceConfig }); + instrumentation.setTracerProvider(provider); instrumentation.manuallyInstrument(OpenAI); +} +const fixture = { id: 'chat-synthetic', object: 'chat.completion', model: 'synthetic', choices: [{ index: 0, message: { role: 'assistant', content: 'SYNTHETIC-PRIVATE-ANSWER' }, finish_reason: 'stop' }] }; +function client(body = fixture, child = false) { + return new OpenAI({ apiKey: 'synthetic-private-key', maxRetries: 0, fetch: async () => { + if (child) { const span = tracer.startSpan('synthetic-transport'); span.end(); } + return new Response(JSON.stringify(body), { headers: { 'content-type': 'application/json' } }); + } }); +} +function attrs() { return exporter.getFinishedSpans()[0].attributes; } +afterEach(async () => { instrumentation?.disable(); instrumentation = undefined; exporter.reset(); }); +after(async () => { context.disable(); await provider.shutdown(); }); +for (const [flag, token, messages] of [ + ['FI_HIDE_INPUT_TEXT', 'SYNTHETIC-PRIVATE-PROMPT', [{ role: 'user', content: 'SYNTHETIC-PRIVATE-PROMPT' }]], + ['FI_HIDE_OUTPUT_TEXT', 'SYNTHETIC-PRIVATE-ANSWER', []], + ['FI_HIDE_INPUT_IMAGES', 'SYNTHETIC-PRIVATE-IMAGE', [{ role: 'user', content: [{ type: 'image_url', image_url: { url: 'https://image.example.invalid/SYNTHETIC-PRIVATE-IMAGE' } }] }]], +]) { + test(`${flag} covers copied raw/value payloads`, async () => { + process.env[flag] = 'true'; + try { + instrument(); await client().chat.completions.create({ model: 'synthetic', messages }); + const leakingKeys = Object.entries(attrs()).filter(([k, v]) => String(v).includes(token)).map(([k]) => k); + assert.deepEqual(leakingKeys, [], `sensitive marker survived in ${leakingKeys.join(', ')}`); + } finally { delete process.env[flag]; } + }); +} +if (OpenAI.Responses) test('FI_HIDE_INPUTS covers Responses system instructions', async () => { + process.env.FI_HIDE_INPUTS = 'true'; + try { + instrument(); await client({ id: 'resp-synthetic', object: 'response', status: 'completed', model: 'synthetic', output: [], instructions: 'SYNTHETIC-PRIVATE-SYSTEM' }).responses.create({ model: 'synthetic', input: 'synthetic', instructions: 'SYNTHETIC-PRIVATE-SYSTEM' }); + const leakingKeys = Object.entries(attrs()).filter(([k, v]) => String(v).includes('SYNTHETIC-PRIVATE-SYSTEM')).map(([k]) => k); + assert.deepEqual(leakingKeys, [], `system prompt survived in ${leakingKeys.join(', ')}`); + } finally { delete process.env.FI_HIDE_INPUTS; } +}); +test('concurrent calls preserve parents and transport child context', async () => { + instrument(); const c = client(fixture, true); + await Promise.all([1, 2].map(async n => { + const parent = tracer.startSpan(`parent-${n}`); + try { await context.with(trace.setSpan(context.active(), parent), () => c.chat.completions.create({ model: 'synthetic', messages: [] })); } + finally { parent.end(); } + })); + const spans = exporter.getFinishedSpans(); + assert.equal(spans.length, 6); + const requests = spans.filter(s => s.name === 'OpenAI Chat Completions'); + assert.equal(new Set(requests.map(s => s.parentSpanContext.spanId)).size, 2); + const requestIds = requests.map(s => s.spanContext().spanId); + for (const child of spans.filter(s => s.name === 'synthetic-transport')) assert.ok(requestIds.includes(child.parentSpanContext.spanId)); + assert.ok(!JSON.stringify(spans.map(s => s.attributes)).includes('synthetic-private-key')); +}); diff --git a/typescript/packages/traceai_openai/test/recallRegressions.cjs b/typescript/packages/traceai_openai/test/recallRegressions.cjs new file mode 100644 index 000000000..820adee4d --- /dev/null +++ b/typescript/packages/traceai_openai/test/recallRegressions.cjs @@ -0,0 +1,213 @@ +// Real installed SDK + packed current integration. Synthetic fetch only; run under sandbox-exec. +const { test, after, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const { getEventListeners } = require('node:events'); +const path = require('node:path'); +const req = createRequire(path.resolve(process.env.OPENAI_CONSUMER_DIR, 'package.json')); +const OpenAI = req('openai'); +const sdkVersion = req(path.resolve(process.env.OPENAI_CONSUMER_DIR, 'node_modules/openai/package.json')).version; +const { OpenAIInstrumentation } = req('@traceai/openai'); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req('@opentelemetry/sdk-trace-base'); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); +let instrumentation; +function instrument(config = {}) { + instrumentation = new OpenAIInstrumentation({ instrumentationConfig: { enabled: false }, traceConfig: config }); + instrumentation.setTracerProvider(provider); + instrumentation.manuallyInstrument(OpenAI); +} +afterEach(async () => { instrumentation?.disable(); instrumentation = undefined; await provider.forceFlush(); exporter.reset(); }); +after(async () => { await provider.shutdown(); }); +const tick = () => new Promise(resolve => setImmediate(resolve)); +const chat = { id: 'chat-offline', object: 'chat.completion', model: 'returned-model', choices: [{ index: 0, message: { role: 'assistant', content: 'SYNTHETIC-ANSWER' }, finish_reason: 'stop' }], usage: { prompt_tokens: 2, completion_tokens: 1, total_tokens: 3 } }; +const response = { id: 'resp-offline', object: 'response', status: 'completed', model: 'returned-model', output: [], usage: { input_tokens: 2, output_tokens: 1, total_tokens: 3 } }; +const toolCall = { id: 'call-synthetic', type: 'custom', custom: { name: 'code_exec', input: 'print(1)' } }; +function client(body, metrics = {}) { + return new OpenAI({ apiKey: 'synthetic-not-a-key', maxRetries: 0, fetch: async (_url, options) => { + metrics.requests = (metrics.requests || 0) + 1; + metrics.body = JSON.parse(options.body); + return new Response(JSON.stringify(body), { headers: { 'content-type': 'application/json' } }); + } }); +} +function span() { const spans = exporter.getFinishedSpans(); assert.equal(spans.length, 1); return spans[0]; } +function streamingClient(operation, { unfinished = false, usage = false } = {}) { + const chunk = operation === 'chat' + ? { id: 'chat-synthetic', object: 'chat.completion.chunk', model: 'returned-model', choices: [{ index: 0, delta: { content: 'SYNTHETIC-ANSWER' }, finish_reason: null }] } + : { id: 'cmpl-synthetic', object: 'text_completion', model: 'returned-model', choices: [{ index: 0, text: 'SYNTHETIC-ANSWER', finish_reason: null }] }; + return new OpenAI({ apiKey: 'synthetic-not-a-key', maxRetries: 0, fetch: async (_url, options) => { + const encoder = new TextEncoder(); + const body = new ReadableStream({ start(controller) { + controller.enqueue(encoder.encode(`data: ${JSON.stringify(chunk)}\n\n`)); + if (unfinished) options.signal.addEventListener('abort', () => controller.error(new DOMException('Synthetic cancellation', 'AbortError')), { once: true }); + else { + if (usage) { + const terminal = { ...chunk, choices: [{ index: 0, ...(operation === 'chat' ? { delta: {} } : { text: '' }), finish_reason: 'stop' }] }; + controller.enqueue(encoder.encode(`data: ${JSON.stringify(terminal)}\n\n`)); + controller.enqueue(encoder.encode(`data: ${JSON.stringify({ ...chunk, choices: [], usage: { prompt_tokens: 12, completion_tokens: 7, total_tokens: 19 } })}\n\n`)); + } + controller.enqueue(encoder.encode('data: [DONE]\n\n')); controller.close(); + } + } }); + return new Response(body, { headers: { 'content-type': 'text/event-stream' } }); + } }); +} +function callStream(c, operation) { + return operation === 'chat' + ? c.chat.completions.create({ model: 'requested-model', messages: [], stream: true, stream_options: { include_usage: true } }) + : c.completions.create({ model: 'requested-model', prompt: 'synthetic', stream: true, stream_options: { include_usage: true } }); +} +test(`${sdkVersion}: baseline normal promise helpers and disable`, async () => { + instrument(); const c = client(chat); const p = c.chat.completions.create({ model: 'synthetic', messages: [] }); + assert.equal((await p.withResponse()).data, await p); assert.equal(span().status.code, 1); + instrumentation.disable(); await c.chat.completions.create({ model: 'synthetic', messages: [] }); assert.equal(exporter.getFinishedSpans().length, 1); +}); +for (const operation of ['chat', 'completions']) { + test(`${sdkVersion}: ${operation} controller abort is not successful`, async () => { + instrument(); const stream = await callStream(streamingClient(operation, { unfinished: true }), operation); + let chunks = 0; for await (const chunk of stream) { chunks++; stream.controller.abort(); } + assert.equal(chunks, 1); assert.notEqual(span().status.code, 1, 'aborted request must not be explicitly OK'); + }); + test(`${sdkVersion}: ${operation} early exit is not successful`, async () => { + instrument(); const stream = await callStream(streamingClient(operation, { unfinished: true }), operation); + for await (const chunk of stream) break; + assert.notEqual(span().status.code, 1, 'early exit aborted request must not be explicitly OK'); + }); + test(`${sdkVersion}: ${operation} abort before iteration finishes span`, async () => { + instrument(); const stream = await callStream(streamingClient(operation, { unfinished: true }), operation); + stream.controller.abort(); stream.controller.abort(); await tick(); await provider.forceFlush(); + // Starting an already aborted iterator must neither finish twice nor turn it OK. + for await (const chunk of stream) {} + assert.equal(exporter.getFinishedSpans().length, 1, 'a closed/aborted stream must finish even without first next()'); + }); + test(`${sdkVersion}: ${operation} streaming usage survives aggregation`, async () => { + instrument(); for await (const chunk of await callStream(streamingClient(operation, { usage: true }), operation)) {} + assert.equal(span().attributes['gen_ai.usage.input_tokens'], 12); + assert.equal(span().attributes['gen_ai.usage.output_tokens'], 7); + assert.equal(span().attributes['gen_ai.usage.total_tokens'], 19); + assert.equal(span().attributes['gen_ai.response.model'], 'returned-model'); + assert.equal(span().attributes['gen_ai.response.id'], operation === 'chat' ? 'chat-synthetic' : 'cmpl-synthetic'); + assert.deepEqual(JSON.parse(span().attributes['gen_ai.response.finish_reasons']), ['stop']); + assert.equal(span().status.code, 1); + }); +} +if (OpenAI.Responses) { + test(`${sdkVersion}: valid stored-prompt response without input reaches transport`, async () => { + const body = { model: 'synthetic', prompt: { id: 'pmpt_synthetic' } }; const metrics = {}; const c = client(response, metrics); + assert.equal((await c.responses.create(body)).id, response.id); assert.equal(metrics.requests, 1); + instrument(); assert.equal((await c.responses.create(body)).id, response.id); assert.equal(metrics.requests, 2); + }); + test(`${sdkVersion}: custom tool output does not reject a successful SDK response`, async () => { + const fixture = { ...chat, choices: [{ index: 0, finish_reason: 'tool_calls', message: { role: 'assistant', content: null, tool_calls: [toolCall] } }] }; + const c = client(fixture); const args = { model: 'synthetic', messages: [], tools: [{ type: 'custom', custom: { name: 'code_exec' } }] }; + assert.equal((await c.chat.completions.create(args)).choices[0].message.tool_calls[0].type, 'custom'); + instrument(); const promise = c.chat.completions.create(args); + const actual = await promise; + assert.equal((await promise.withResponse()).data, actual); + assert.deepEqual(actual.choices[0].message.tool_calls[0], toolCall); + assert.deepEqual(JSON.parse(span().attributes['gen_ai.output.messages'])[0].tool_calls[0], toolCall); + }); + test(`${sdkVersion}: custom tool history does not block the next request`, async () => { + const c = client(chat); const args = { model: 'synthetic', messages: [{ role: 'assistant', content: null, tool_calls: [toolCall] }, { role: 'tool', tool_call_id: toolCall.id, content: '1' }] }; + await c.chat.completions.create(args); instrument(); await c.chat.completions.create(args); + assert.deepEqual(JSON.parse(span().attributes['gen_ai.input.messages'])[0].tool_calls[0], toolCall); + }); + test(`${sdkVersion}: terminal failed response is not OK`, async () => { + instrument(); const fixture = { ...response, status: 'failed', error: { code: 'server_error', message: 'synthetic failure' } }; + assert.equal((await client(fixture).responses.create({ model: 'synthetic', input: 'synthetic' })).status, 'failed'); + assert.equal(span().status.code, 2, 'failed response must be ERROR'); + }); +} +test(`${sdkVersion}: FI_HIDE_INPUTS covers embedding text attributes`, async () => { + process.env.FI_HIDE_INPUTS = 'true'; + try { + instrument(); await client({ object: 'list', data: [{ index: 0, embedding: [0.125, 0.875] }], model: 'synthetic' }).embeddings.create({ model: 'synthetic', input: 'SYNTHETIC-PRIVATE-EMBEDDING', encoding_format: 'float' }); + assert.ok(!JSON.stringify(span().attributes).includes('SYNTHETIC-PRIVATE-EMBEDDING'), 'hidden embedding input leaked'); + } finally { delete process.env.FI_HIDE_INPUTS; } +}); +test(`${sdkVersion}: FI_HIDE_EMBEDDING_VECTORS covers raw output`, async () => { + process.env.FI_HIDE_EMBEDDING_VECTORS = 'true'; + try { + instrument(); await client({ object: 'list', data: [{ index: 0, embedding: [0.125, 0.875] }], model: 'synthetic' }).embeddings.create({ model: 'synthetic', input: 'synthetic', encoding_format: 'float' }); + assert.ok(!JSON.stringify(span().attributes).includes('0.875'), 'hidden embedding vector leaked through raw.output'); + } finally { delete process.env.FI_HIDE_EMBEDDING_VECTORS; } +}); + +for (const operation of ['chat', 'completions']) { + test(`${sdkVersion}: ${operation} request deadline is recorded as failure`, async () => { + instrument(); const c = streamingClient(operation, { unfinished: true }); + const controller = new AbortController(); + const stream = await (operation === 'chat' + ? c.chat.completions.create({ model: 'synthetic', messages: [], stream: true }, { signal: controller.signal }) + : c.completions.create({ model: 'synthetic', prompt: 'synthetic', stream: true }, { signal: controller.signal })); + const listenersDuring = getEventListeners(controller.signal, 'abort').length; + let chunks = 0; + for await (const chunk of stream) { + chunks++; + controller.abort(new DOMException('Synthetic deadline exceeded', 'TimeoutError')); + } + // The real SDK swallows AbortError. Instrumentation must not introduce a rejection. + assert.equal(chunks, 1); + assert.equal(span().status.code, 2, 'timed-out operation must be ERROR'); + assert.equal(span().events.length, 1); + assert.equal(span().events[0].attributes['exception.type'], 'TimeoutError'); + // SDK 4 retains its own abort forwarder; only our listener must be removed. + assert.ok(getEventListeners(controller.signal, 'abort').length < listenersDuring); + }); + test(`${sdkVersion}: ${operation} request deadline before first next ends exactly once`, async () => { + instrument({ hideErrorDetails: true }); + const c = streamingClient(operation, { unfinished: true }); + const controller = new AbortController(); + const stream = await (operation === 'chat' + ? c.chat.completions.create({ model: 'synthetic', messages: [], stream: true }, { signal: controller.signal }) + : c.completions.create({ model: 'synthetic', prompt: 'synthetic', stream: true }, { signal: controller.signal })); + const listenersDuring = getEventListeners(controller.signal, 'abort').length; + controller.abort(new DOMException('SYNTHETIC-PRIVATE-DEADLINE', 'TimeoutError')); + await tick(); + assert.equal(span().status.code, 2); + for await (const chunk of stream) {} + stream.controller.abort(); + const finished = span(); + assert.ok(!JSON.stringify({ attrs: finished.attributes, status: finished.status, events: finished.events }).includes('SYNTHETIC-PRIVATE-DEADLINE')); + // SDK 4 retains its own abort forwarder; only our listener must be removed. + assert.ok(getEventListeners(controller.signal, 'abort').length < listenersDuring); + }); + test(`${sdkVersion}: ${operation} normal completion detaches request abort listener`, async () => { + instrument(); const c = streamingClient(operation); + const controller = new AbortController(); + const stream = await (operation === 'chat' + ? c.chat.completions.create({ model: 'synthetic', messages: [], stream: true }, { signal: controller.signal }) + : c.completions.create({ model: 'synthetic', prompt: 'synthetic', stream: true }, { signal: controller.signal })); + const listenersDuring = getEventListeners(controller.signal, 'abort').length; + for await (const chunk of stream) {} + assert.ok(getEventListeners(controller.signal, 'abort').length < listenersDuring); + assert.equal(span().status.code, 1); + controller.abort(new DOMException('Late abort must not change success', 'TimeoutError')); + assert.equal(span().status.code, 1); + }); +} +if (OpenAI.Responses) { + for (const status of ['failed', 'incomplete']) { + for (const streamed of [false, true]) { + test(`${sdkVersion}: Responses ${status} streamed=${streamed} keeps SDK payload and hidden errors`, async () => { + const fixture = { ...response, status, error: status === 'failed' ? { code: 'server_error', message: 'SYNTHETIC-PRIVATE-FAILURE' } : null }; + const c = new OpenAI({ apiKey: 'synthetic', maxRetries: 0, fetch: async () => new Response( + streamed ? `data: ${JSON.stringify({ type: `response.${status}`, response: fixture })}\n\ndata: [DONE]\n\n` : JSON.stringify(fixture), + { headers: { 'content-type': streamed ? 'text/event-stream' : 'application/json' } }, + ) }); + const consume = async () => { + const result = await c.responses.create({ model: 'synthetic', input: 'synthetic', stream: streamed }); + if (!streamed) return result; + const events = []; for await (const event of result) events.push(event); + return events; + }; + const baseline = await consume(); + instrument({ hideErrorDetails: true }); + assert.deepEqual(await consume(), baseline); + const finished = span(); + assert.equal(finished.status.code, 2); + assert.ok(!JSON.stringify({ attrs: finished.attributes, status: finished.status, events: finished.events }).includes('SYNTHETIC-PRIVATE-FAILURE')); + }); + } + } +} From 0c8fb123413c69f6879b65bccc089217ae30955b Mon Sep 17 00:00:00 2001 From: Nikhil Pareek Date: Mon, 2 Nov 2026 03:30:42 -0800 Subject: [PATCH 2/4] fix(openai): address recall privacy and streaming review Protect stored-prompt inputs and MCP credentials in all telemetry copies. Integrate the shared structured-message redactor and tests from #416 at 8630636b3d80e6b8cefdf85e41b1d9da8e88e344 while preserving duplicate-payload masking. Aggregate function/custom deltas by SDK index without overwriting request model metadata. Run core Jest directly inside network-isolated CI. Co-Authored-By: Claude Code --- .github/workflows/openai-sdk-contracts.yml | 9 +- .../__tests__/duplicatePayloadPrivacy.test.ts | 8 +- .../src/trace/trace-config/maskingRules.ts | 95 +-- .../trace/trace-config/messageRedaction.ts | 84 +++ .../fi-core/test/genai-privacy.test.cjs | 245 ++++++++ .../src/__tests__/openai.test.ts | 6 +- .../traceai_openai/src/instrumentation.ts | 191 +++--- .../traceai_openai/test/currentSdkShapes.mts | 31 +- .../traceai_openai/test/reviewRegressions.cjs | 562 ++++++++++++++++++ 9 files changed, 1029 insertions(+), 202 deletions(-) create mode 100644 typescript/packages/fi-core/src/trace/trace-config/messageRedaction.ts create mode 100644 typescript/packages/fi-core/test/genai-privacy.test.cjs create mode 100644 typescript/packages/traceai_openai/test/reviewRegressions.cjs diff --git a/.github/workflows/openai-sdk-contracts.yml b/.github/workflows/openai-sdk-contracts.yml index fdac066ea..04d1a1774 100644 --- a/.github/workflows/openai-sdk-contracts.yml +++ b/.github/workflows/openai-sdk-contracts.yml @@ -75,7 +75,8 @@ jobs: sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ env PATH="$PATH" OPENAI_CONSUMER_DIR="$OPENAI_CONSUMER_DIR" \ node --test packages/traceai_openai/test/recallRegressions.cjs \ - packages/traceai_openai/test/privacyContext.cjs + packages/traceai_openai/test/privacyContext.cjs \ + packages/traceai_openai/test/reviewRegressions.cjs - name: Typecheck current SDK request contracts if: matrix.sdk == '7.47.0' env: @@ -116,4 +117,8 @@ jobs: env PATH="$PATH" node packages/traceai_openai/node_modules/jest/bin/jest.js \ --config packages/traceai_openai/jest.config.js --runInBand sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ - env PATH="$PATH" pnpm --filter @traceai/fi-core test --runInBand + env PATH="$PATH" node packages/fi-core/node_modules/jest/bin/jest.js \ + --config packages/fi-core/jest.config.js --runInBand + sudo unshare --net -- bash -c 'ip link set lo up; exec "$@"' bash \ + env PATH="$PATH" FI_CORE_FIXTURE="${{ runner.temp }}/consumer" \ + node --test packages/fi-core/test/genai-privacy.test.cjs diff --git a/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts b/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts index 4ab7bc7b8..6f8c690e3 100644 --- a/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts +++ b/typescript/packages/fi-core/src/__tests__/duplicatePayloadPrivacy.test.ts @@ -4,10 +4,10 @@ import { FITracer } from "../trace/trace-config/FITracer"; import { TraceConfigOptions } from "../trace/trace-config/types"; const cases: [keyof TraceConfigOptions, string[]][] = [ - ["hideInputs", [`${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`, `${SC.EMBEDDING_EMBEDDINGS}.12.${SC.EMBEDDING_TEXT}`]], - ["hideInputText", [SC.INPUT_VALUE, SC.RAW_INPUT, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`]], - ["hideInputImages", [SC.INPUT_VALUE, SC.RAW_INPUT]], - ["hideInputMessages", [SC.INPUT_VALUE, SC.RAW_INPUT]], + ["hideInputs", [SC.PROMPT_TEMPLATE_VARIABLES, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`, `${SC.EMBEDDING_EMBEDDINGS}.12.${SC.EMBEDDING_TEXT}`]], + ["hideInputText", [SC.PROMPT_TEMPLATE_VARIABLES, SC.INPUT_VALUE, SC.RAW_INPUT, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_TEXT}`]], + ["hideInputImages", [SC.PROMPT_TEMPLATE_VARIABLES, SC.INPUT_VALUE, SC.RAW_INPUT]], + ["hideInputMessages", [SC.PROMPT_TEMPLATE_VARIABLES, SC.INPUT_VALUE, SC.RAW_INPUT]], ["hideOutputText", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT]], ["hideOutputMessages", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT]], ["hideEmbeddingVectors", [SC.OUTPUT_VALUE, SC.RAW_OUTPUT, `${SC.EMBEDDING_EMBEDDINGS}.0.${SC.EMBEDDING_VECTOR}`]], diff --git a/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts b/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts index d20b64677..ecab09bc1 100644 --- a/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts +++ b/typescript/packages/fi-core/src/trace/trace-config/maskingRules.ts @@ -4,85 +4,23 @@ import { SemanticConventions } from "@traceai/fi-semantic-conventions"; import { MaskingRule, MaskingRuleArgs, TraceConfig } from "./types"; import { redactPiiInValue } from "./piiRedaction"; import { isErrorDetailAttribute } from "./errorDetails"; +import { redactMessages } from "./messageRedaction"; -/** - * Redacts content within a JSON blob of messages. - * Parses the JSON string, replaces all "content" fields with REDACTED_VALUE, - * and returns the modified JSON string. - */ -function redactContentInJsonBlob(value: AttributeValue): AttributeValue | undefined { - if (typeof value !== "string") return value; - try { - const messages = JSON.parse(value); - if (!Array.isArray(messages)) return REDACTED_VALUE; - const redacted = messages.map((msg: Record) => { - const copy = { ...msg }; - if (copy.content !== undefined) { - copy.content = REDACTED_VALUE; - } - return copy; - }); - return JSON.stringify(redacted); - } catch { - return REDACTED_VALUE; - } -} - -/** - * Redacts images within a JSON blob of messages. - * Removes image_url fields from content arrays within messages. - */ -function redactImagesInJsonBlob(value: AttributeValue): AttributeValue | undefined { - if (typeof value !== "string") return value; - try { - const messages = JSON.parse(value); - if (!Array.isArray(messages)) return value; - const redacted = messages.map((msg: Record) => { - const copy = { ...msg }; - if (Array.isArray(copy.content)) { - copy.content = (copy.content as Record[]).filter( - (part) => part.type !== "image_url" && part.type !== "input_image" && part.type !== "image" - ); - } - return copy; - }); - return JSON.stringify(redacted); - } catch { - return value; - } -} - -/** - * Masks input text in LLM input messages (JSON blob format). - * Redacts "content" fields within the gen_ai.input.messages JSON blob. - */ -const maskInputTextRule: MaskingRule = { +// Apply text and image settings together: the masking rule list stops at its +// first match, so independent rules would let image hiding bypass text hiding. +const maskInputContentRule: MaskingRule = { condition: ({ config, key }) => - config.hideInputText && + (config.hideInputText || config.hideInputImages) && key === SemanticConventions.LLM_INPUT_MESSAGES, - action: ({ value }: MaskingRuleArgs) => value != null ? redactContentInJsonBlob(value) : undefined, + action: ({ config, value }) => value != null + ? redactMessages({ value, hideText: config.hideInputText, hideImages: config.hideInputImages }) : undefined, }; -/** - * Masks output text in LLM output messages (JSON blob format). - * Redacts "content" fields within the gen_ai.output.messages JSON blob. - */ const maskOutputTextRule: MaskingRule = { condition: ({ config, key }) => - config.hideOutputText && - key === SemanticConventions.LLM_OUTPUT_MESSAGES, - action: ({ value }: MaskingRuleArgs) => value != null ? redactContentInJsonBlob(value) : undefined, -}; - -/** - * Masks input images in LLM input messages (JSON blob format). - * Removes image content parts from the gen_ai.input.messages JSON blob. - */ -const maskInputImagesRule: MaskingRule = { - condition: ({ config, key }) => - config.hideInputImages && - key === SemanticConventions.LLM_INPUT_MESSAGES, - action: ({ value }: MaskingRuleArgs) => value != null ? redactImagesInJsonBlob(value) : undefined, + config.hideOutputText && key === SemanticConventions.LLM_OUTPUT_MESSAGES, + action: ({ value }) => value != null + ? redactMessages({ value, hideText: true, hideImages: false }) : undefined, }; /** @@ -124,9 +62,10 @@ const maskingRules: MaskingRule[] = [ action: () => REDACTED_VALUE as AttributeValue, }, { - // The values filled into a prompt template are the template's inputs + // Prompt substitution values are opaque input payloads, including images. condition: ({ config, key }) => - config.hideInputs && key === SemanticConventions.PROMPT_TEMPLATE_VARIABLES, + (config.hideInputs || config.hideInputText || config.hideInputImages || config.hideInputMessages) && + key === SemanticConventions.PROMPT_TEMPLATE_VARIABLES, action: () => REDACTED_VALUE as AttributeValue, }, { @@ -157,8 +96,7 @@ const maskingRules: MaskingRule[] = [ key === SemanticConventions.LLM_OUTPUT_MESSAGES, action: () => undefined, }, - maskInputImagesRule, - maskInputTextRule, + maskInputContentRule, maskOutputTextRule, maskEmbeddingVectorsRule, ]; @@ -177,10 +115,11 @@ export function mask({ }: MaskingRuleArgs): AttributeValue | undefined { for (const rule of maskingRules) { if (rule.condition({ config, key, value })) { - return rule.action({ config, key, value }); + value = rule.action({ config, key, value }); + break; } } - // No key-based rule matched — apply PII redaction if enabled + // Apply PII redaction to retained text even when a content rule matched. if (config.piiRedaction && value != null) { return redactPiiInValue(value); } diff --git a/typescript/packages/fi-core/src/trace/trace-config/messageRedaction.ts b/typescript/packages/fi-core/src/trace/trace-config/messageRedaction.ts new file mode 100644 index 000000000..e5f262871 --- /dev/null +++ b/typescript/packages/fi-core/src/trace/trace-config/messageRedaction.ts @@ -0,0 +1,84 @@ +import { AttributeValue } from "@opentelemetry/api"; +import { REDACTED_VALUE } from "./constants"; + +const MESSAGE_METADATA = ["role", "name", "id", "tool_call_id", "finish_reason"]; +const PART_METADATA = ["type", "id", "name", "modality", "mime_type"]; +const IMAGE_TYPES = ["image", "image_url", "input_image"]; +const TEXT_TYPES = ["text", "input_text", "output_text", "reasoning", "compaction"]; +const MEDIA_TYPES = ["blob", "file", "uri"]; + +type JsonObject = Record; + +function isObject(value: unknown): value is JsonObject { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +// Only scalar metadata is safe to retain. Unknown extension fields and structured +// payloads (including tool arguments/results) may contain text or images. +function redactFields(value: JsonObject, keep: readonly string[]): JsonObject { + return Object.fromEntries(Object.entries(value).map(([key, field]) => [ + key, + keep.includes(key) && (field === null || typeof field !== "object") + ? field : REDACTED_VALUE, + ])); +} + +function isImage(part: JsonObject): boolean { + return (typeof part.type === "string" && IMAGE_TYPES.includes(part.type)) || part.modality === "image" || + (typeof part.mime_type === "string" && /^image\//i.test(part.mime_type)); +} + +function redactParts(parts: unknown, hideText: boolean, hideImages: boolean): unknown { + if (!Array.isArray(parts)) return REDACTED_VALUE; + return parts + .filter(part => !(hideImages && isObject(part) && isImage(part))) + .map(part => { + if (!isObject(part)) return REDACTED_VALUE; + const type = typeof part.type === "string" ? part.type : ""; + if (!hideText && TEXT_TYPES.includes(type)) { + return redactFields(part, [...PART_METADATA, "content", "text"]); + } + // Image-only hiding retains explicitly non-image media. Missing/unknown + // modality fails closed rather than guessing what opaque data contains. + if (!hideText && MEDIA_TYPES.includes(type) && + ((typeof part.modality === "string" && ["audio", "video", "document"].includes(part.modality)) || + (typeof part.mime_type === "string" && /^(audio|video|text)\//i.test(part.mime_type)))) { + return redactFields(part, [...PART_METADATA, "content", "uri", "file_id"]); + } + return redactFields(part, PART_METADATA); + }); +} + +/** + * Mask both legacy content messages and native OTel GenAI role/parts messages. + * Text hiding conservatively redacts all payloads, including opaque tool/media + * data. Image-only hiding retains known text/non-image media. Unknown shapes + * fail closed; no arbitrary payload is recursively inspected or copied through. + */ +export function redactMessages({ value, hideText, hideImages }: { + value: AttributeValue; + hideText: boolean; + hideImages: boolean; +}): AttributeValue { + if (typeof value !== "string") return REDACTED_VALUE; + try { + const messages: unknown = JSON.parse(value); + if (!Array.isArray(messages) || !messages.every(isObject)) return REDACTED_VALUE; + return JSON.stringify(messages.map(message => { + const redacted = redactFields(message, MESSAGE_METADATA); + if ("content" in message) { + // Preserve the legacy contract: text hiding redacts the entire content, + // including a multimodal array, not just string content. + redacted.content = hideText ? REDACTED_VALUE : + typeof message.content === "string" ? message.content : + redactParts(message.content, hideText, hideImages); + } + if ("parts" in message) { + redacted.parts = redactParts(message.parts, hideText, hideImages); + } + return redacted; + })); + } catch { + return REDACTED_VALUE; + } +} diff --git a/typescript/packages/fi-core/test/genai-privacy.test.cjs b/typescript/packages/fi-core/test/genai-privacy.test.cjs new file mode 100644 index 000000000..9472479b5 --- /dev/null +++ b/typescript/packages/fi-core/test/genai-privacy.test.cjs @@ -0,0 +1,245 @@ +// Run after building fi-core, or set FI_CORE_FIXTURE to a packed-package install. +// Native shapes: https://github.com/open-telemetry/semantic-conventions-genai/blob/ +// 2e994c6d59a93bb4fc1752c5378eedb9b8e14d6b/model/gen-ai/gen-ai-input-messages.json +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const { resolve } = require('node:path'); +const { createServer } = require('node:http'); +const { once } = require('node:events'); +const { test } = require('node:test'); +const load = createRequire(resolve(process.env.FI_CORE_FIXTURE || __dirname + '/..', 'package.json')); +const { FITracer } = process.env.FI_CORE_FIXTURE ? load('@traceai/fi-core') : load('./dist/src/index.js'); +const { BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor, SamplingDecision } = load('@opentelemetry/sdk-trace-base'); +const { OTLPTraceExporter } = load('@opentelemetry/exporter-trace-otlp-http'); +const { ROOT_CONTEXT, trace } = load('@opentelemetry/api'); +const INPUT = 'gen_ai.input.messages'; +const OUTPUT = 'gen_ai.output.messages'; +const REDACTED = '__REDACTED__'; +const SECRET = 'synthetic-private-payload'; +const native = parts => JSON.stringify([{ role: 'user', parts }]); +const text = native([{ type: 'text', content: SECRET }]); + +async function record(config, key, value, route = 'creation') { + const exporter = new InMemorySpanExporter(); + const sampled = []; + const provider = new BasicTracerProvider({ + spanProcessors: [new SimpleSpanProcessor(exporter)], + sampler: { + shouldSample(_ctx, _id, _name, _kind, attributes) { + sampled.push(attributes); + return { decision: SamplingDecision.RECORD_AND_SAMPLED }; + }, + toString: () => 'privacy-test', + }, + }); + try { + const tracer = new FITracer({ tracer: provider.getTracer('genai-test'), traceConfig: config }); + const attributes = { [key]: value, 'gen_ai.request.model': 'synthetic-model' }; + const linked = { traceId: '1'.repeat(32), spanId: '2'.repeat(16), traceFlags: 1 }; + if (route === 'active') { + tracer.startActiveSpan('synthetic', { attributes }, span => span.end()); + } else { + const span = tracer.startSpan('synthetic', route === 'creation' ? { attributes } : + route === 'creation-link' ? { links: [{ context: linked, attributes }] } : {}); + if (route === 'setAttribute') span.setAttribute(key, value); + if (route === 'setAttributes') span.setAttributes(attributes); + if (route === 'link') span.addLink({ context: linked, attributes }); + if (route === 'links') span.addLinks([{ context: linked, attributes }]); + span.end(); + } + await provider.forceFlush(); + const spans = exporter.getFinishedSpans(); + assert.equal(spans.length, 1); + return { attributes: spans[0].attributes, links: spans[0].links, sampled }; + } finally { + await provider.shutdown(); + } +} + +for (const [key, flag] of [[INPUT, 'hideInputText'], [OUTPUT, 'hideOutputText']]) { + for (const route of ['creation', 'active', 'setAttribute', 'setAttributes', 'creation-link', 'link', 'links']) { + test(`${flag}: native parts masked via ${route}, including sampler`, async () => { + const result = await record({ [flag]: true }, key, text, route); + assert.ok(!JSON.stringify(result).includes(SECRET)); + const attributes = route.includes('link') ? result.links[0].attributes : result.attributes; + assert.deepEqual(JSON.parse(attributes[key]), [{ role: 'user', parts: [{ type: 'text', content: REDACTED }] }]); + }); + } + for (const part of [ + { type: 'reasoning', content: SECRET }, + { type: 'compaction', content: SECRET, encrypted_content: SECRET }, + { type: 'tool_call', id: 'call-1', name: 'lookup', arguments: { query: SECRET } }, + { type: 'tool_call_response', id: 'call-1', response: { nested: [SECRET] } }, + { type: 'server_tool_call', name: 'search', server_tool_call: { type: 'search', query: SECRET } }, + { type: 'server_tool_call_response', server_tool_call_response: { type: 'search', results: [SECRET] } }, + { type: 'future_part', nested: { secret: SECRET } }, + { type: 'blob', modality: 'image', content: SECRET }, + { type: 'uri', modality: 'image', uri: SECRET }, + { type: 'file', modality: 'image', file_id: SECRET }, + ]) { + test(`${flag}: redacts ${part.type} payloads and preserves metadata`, async () => { + const { attributes } = await record({ [flag]: true }, key, native([part])); + assert.ok(!attributes[key].includes(SECRET)); + const redacted = JSON.parse(attributes[key])[0].parts[0]; + assert.equal(redacted.type, part.type); + if (part.id) assert.equal(redacted.id, part.id); + if (part.name) assert.equal(redacted.name, part.name); + }); + } + test(`${flag}: legacy content and tool payloads, extensions, dual shapes`, async () => { + const message = { role: 'assistant', content: SECRET, parts: [{ type: 'text', content: SECRET }], + tool_calls: [{ id: 'call-1', function: { name: 'lookup', arguments: SECRET } }], + function_call: { arguments: SECRET }, extension: { data: SECRET } }; + const { attributes } = await record({ [flag]: true }, key, JSON.stringify([message])); + assert.ok(!attributes[key].includes(SECRET)); + assert.equal(JSON.parse(attributes[key])[0].content, REDACTED); + }); +} + +for (const part of [ + { type: 'image', content: SECRET }, + { type: 'image_url', image_url: { url: SECRET } }, + { type: 'input_image', image_url: SECRET }, + { type: 'blob', modality: 'image', mime_type: 'image/png', content: SECRET }, + { type: 'uri', modality: 'image', uri: SECRET }, + { type: 'file', modality: 'image', file_id: SECRET }, + { type: 'blob', mime_type: 'image/jpeg', content: SECRET }, + { type: 'uri', modality: 'audio', mime_type: 'image/png', uri: SECRET }, +]) { + for (const field of ['parts', 'content']) { + test(`hideInputImages: removes ${part.type}/${part.modality || part.mime_type || 'legacy'} in ${field}`, async () => { + const value = JSON.stringify([{ role: 'user', [field]: [{ type: 'text', content: 'keep text' }, part] }]); + const { attributes } = await record({ hideInputImages: true }, INPUT, value); + assert.ok(!attributes[INPUT].includes(SECRET)); + assert.deepEqual(JSON.parse(attributes[INPUT])[0][field], [{ type: 'text', content: 'keep text' }]); + }); + } +} + +test('hideInputImages: preserves declared non-image media and legacy text', async () => { + const parts = [{ type: 'blob', modality: 'audio', mime_type: 'audio/wav', content: 'audio' }, + { type: 'uri', modality: 'video', uri: 'https://example.invalid/video' }, + { type: 'text', content: 'keep text' }]; + const value = JSON.stringify([{ role: 'user', parts, content: 'keep legacy' }]); + const { attributes } = await record({ hideInputImages: true }, INPUT, value); + assert.deepEqual(JSON.parse(attributes[INPUT]), JSON.parse(value)); +}); + +test('hideInputImages: opaque tool payloads and unknown parts fail closed', async () => { + const value = native([{ type: 'custom', payload: SECRET }, { type: 'blob', content: SECRET }, + { type: ['text'], content: SECRET }, + { type: 'blob', modality: ['audio'], content: SECRET }, + { type: 'blob', mime_type: 'application/octet-stream', content: SECRET }, + { type: 'tool_call_response', response: { image_url: SECRET } }]); + const { attributes } = await record({ hideInputImages: true }, INPUT, value); + assert.ok(!attributes[INPUT].includes(SECRET)); +}); + +for (const config of [{ hideInputText: true }, { hideInputImages: true }, { hideInputText: true, hideInputImages: true }]) { + for (const value of ['[broken-private-payload', '{"content":"synthetic-private-payload"}', 'null', + '[null]', '["synthetic-private-payload"]', 42, ['synthetic-private-payload'], + native([null, SECRET]), JSON.stringify([{ role: 'user', parts: SECRET }])]) { + test(`fails closed for malformed messages ${JSON.stringify(config)} ${JSON.stringify(value)}`, async () => { + const { attributes } = await record(config, INPUT, value); + assert.ok(!JSON.stringify(attributes).includes(SECRET)); + assert.notEqual(attributes[INPUT], value); + }); + } +} + +test('image and text flags compose instead of the first rule winning', async () => { + for (const field of ['parts', 'content']) { + const value = JSON.stringify([{ role: 'user', [field]: [{ type: 'text', content: SECRET }, + { type: 'image_url', image_url: { url: SECRET } }] }]); + const { attributes } = await record({ hideInputText: true, hideInputImages: true }, INPUT, value); + assert.ok(!attributes[INPUT].includes(SECRET)); + } +}); + +test('image hiding still applies PII redaction to retained text', async () => { + const { attributes } = await record({ hideInputImages: true, piiRedaction: true }, INPUT, + native([{ type: 'text', content: 'Contact john@example.com' }])); + assert.ok(!attributes[INPUT].includes('john@example.com')); + assert.ok(attributes[INPUT].includes('')); +}); + +test('disabled and opposite-direction flags preserve bytes; broad flags remove messages', async () => { + for (const config of [{}, { hideOutputText: true }]) { + assert.equal((await record(config, INPUT, text)).attributes[INPUT], text); + } + for (const flag of ['hideInputs', 'hideInputMessages']) { + assert.equal((await record({ [flag]: true }, INPUT, text)).attributes[INPUT], undefined); + } + for (const flag of ['hideOutputs', 'hideOutputMessages']) { + assert.equal((await record({ [flag]: true }, OUTPUT, text)).attributes[OUTPUT], undefined); + } +}); + +test('empty messages and repeated masking are stable', async () => { + const config = { hideInputText: true, hideInputImages: true }; + assert.equal((await record(config, INPUT, '[]')).attributes[INPUT], '[]'); + const onceMasked = (await record(config, INPUT, text)).attributes[INPUT]; + assert.equal((await record(config, INPUT, onceMasked)).attributes[INPUT], onceMasked); +}); + +test('structured metadata and a large mixed message batch cannot carry payloads through', async () => { + const value = JSON.stringify(Array.from({ length: 1000 }, () => ({ role: 'user', + name: { data: SECRET }, parts: [{ type: 'text', content: SECRET, id: { data: SECRET } }, + { type: 'tool_call_response', response: [SECRET] }] }))); + const { attributes } = await record({ hideInputText: true }, INPUT, value); + assert.ok(!attributes[INPUT].includes(SECRET)); + assert.equal(JSON.parse(attributes[INPUT]).length, 1000); +}); + +test('environment privacy flags use the same native message masking', async () => { + const previous = process.env.FI_HIDE_INPUT_TEXT; + process.env.FI_HIDE_INPUT_TEXT = 'true'; + try { + assert.ok(!(await record({}, INPUT, text)).attributes[INPUT].includes(SECRET)); + } finally { + if (previous === undefined) delete process.env.FI_HIDE_INPUT_TEXT; + else process.env.FI_HIDE_INPUT_TEXT = previous; + } +}); + +test('real OTLP HTTP wire: wrapped payloads private, raw external spans remain outside boundary', async () => { + const bodies = []; + const server = createServer((request, response) => { + const chunks = []; + request.on('data', chunk => chunks.push(chunk)); + request.on('end', () => { + bodies.push(JSON.parse(Buffer.concat(chunks).toString('utf8'))); + response.writeHead(200, { 'Content-Type': 'application/json' }); + response.end('{}'); + }); + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + const exporter = new OTLPTraceExporter({ url: `http://127.0.0.1:${server.address().port}/v1/traces` }); + const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); + try { + const raw = provider.getTracer('wire-test'); + const tracer = new FITracer({ tracer: raw, traceConfig: { hideInputText: true, hideOutputText: true, hideInputImages: true } }); + const parent = tracer.startSpan('parent', { attributes: { [INPUT]: text, 'gen_ai.request.model': 'synthetic-model' } }); + const child = tracer.startSpan('child', {}, trace.setSpan(ROOT_CONTEXT, parent)); + child.setAttributes({ [OUTPUT]: native([{ type: 'tool_call', name: 'lookup', arguments: { query: SECRET } }]) }); + child.end(); + parent.end(); + raw.startSpan('external-control', { attributes: { [INPUT]: text } }).end(); + await provider.forceFlush(); + const spans = bodies.flatMap(body => body.resourceSpans.flatMap(resource => resource.scopeSpans.flatMap(scope => scope.spans))); + assert.equal(spans.length, 3, 'no duplicate spans'); + const wrapped = spans.filter(span => span.name !== 'external-control'); + assert.equal(wrapped.length, 2); + assert.ok(!JSON.stringify(wrapped).includes(SECRET)); + const wireParent = spans.find(span => span.name === 'parent'); + const wireChild = spans.find(span => span.name === 'child'); + assert.equal(wireChild.traceId, wireParent.traceId); + assert.equal(wireChild.parentSpanId, wireParent.spanId); + assert.ok(JSON.stringify(wireParent).includes('synthetic-model')); + assert.ok(JSON.stringify(spans.find(span => span.name === 'external-control')).includes(SECRET)); + } finally { + await provider.shutdown(); + await new Promise(resolve => server.close(resolve)); + } +}); diff --git a/typescript/packages/traceai_openai/src/__tests__/openai.test.ts b/typescript/packages/traceai_openai/src/__tests__/openai.test.ts index 944fb6b42..7c2052af9 100644 --- a/typescript/packages/traceai_openai/src/__tests__/openai.test.ts +++ b/typescript/packages/traceai_openai/src/__tests__/openai.test.ts @@ -753,9 +753,9 @@ describe("OpenAIInstrumentation", () => { "fi.span.kind": "LLM", "gen_ai.input.messages": "[{"role":"user","content":"What's the weather today?"}]", "gen_ai.operation.name": "chat", - "gen_ai.output.messages": "[{"role":"assistant","content":"","tool_calls":[{"id":"call_PGkcUg2u6vYrCpTn0e9ofykY","function":{"name":"getWeather","arguments":"{}"}}]}]", + "gen_ai.output.messages": "[{"role":"assistant","content":"","tool_calls":[{"id":"call_PGkcUg2u6vYrCpTn0e9ofykY","type":"function","function":{"name":"getWeather","arguments":"{}"}}]}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"stream":true}", "gen_ai.response.finish_reasons": "["tool_calls"]", "gen_ai.response.id": "chatcmpl-8iA39kCtuVHIVDr9AnBdJZjgSjNWL", @@ -871,7 +871,7 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"","function_call_name":"getWeather","function_call_arguments":"{}"}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","functions":[{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}},{"name":"getCurrentLocation","description":"Get the current location of the user.","parameters":{"type":"object","properties":{}}}],"stream":true}", "gen_ai.response.finish_reasons": "["function_call"]", "gen_ai.response.id": "chatcmpl-8iA39kCtuVHIVDr9AnBdJZjgSjNWL", diff --git a/typescript/packages/traceai_openai/src/instrumentation.ts b/typescript/packages/traceai_openai/src/instrumentation.ts index 98f3d2f95..590af2b00 100644 --- a/typescript/packages/traceai_openai/src/instrumentation.ts +++ b/typescript/packages/traceai_openai/src/instrumentation.ts @@ -38,7 +38,7 @@ import { CreateEmbeddingResponse, EmbeddingCreateParams, } from "openai/resources"; -import { assertUnreachable, isString } from "./typeUtils"; +import { assertUnreachable } from "./typeUtils"; import { isTracingSuppressed } from "@opentelemetry/core"; import { @@ -486,8 +486,18 @@ export class OpenAIInstrumentation extends InstrumentationBase{ this: unknown, ...args: Parameters ) { - const body = args[0]; - const { input: _messages, instructions: _instructions, ...invocationParameters } = body; + // Stored prompts are declared by SDK releases newer than our build SDK. + const body = args[0] as ResponseCreateParamsBase & { + prompt?: { id: string; version?: string; variables?: Record }; + }; + // Telemetry-only copy: MCP credentials must never reach any exported + // duplicate, but the original SDK request must remain untouched. + const telemetryBody = { ...body, ...(body.tools && { tools: sanitizeToolDefinitions(body.tools) }) }; + const { input: _messages, instructions: _instructions, prompt, ...parameters } = telemetryBody; + const invocationParameters = { + ...parameters, + ...(prompt && { prompt: { id: prompt.id, version: prompt.version } }), + }; const span = instrumentation.fiTracer.startSpan( `OpenAI Responses`, { @@ -496,15 +506,16 @@ export class OpenAIInstrumentation extends InstrumentationBase{ [SemanticConventions.FI_SPAN_KIND]: FISpanKind.LLM, [SemanticConventions.LLM_MODEL_NAME]: body.model, - [SemanticConventions.INPUT_VALUE]: JSON.stringify(body), + [SemanticConventions.INPUT_VALUE]: JSON.stringify(telemetryBody), [SemanticConventions.INPUT_MIME_TYPE]: MimeType.JSON, [SemanticConventions.LLM_INVOCATION_PARAMETERS]: JSON.stringify(invocationParameters), + [SemanticConventions.PROMPT_TEMPLATE_VARIABLES]: safelyJSONStringify(prompt?.variables) ?? undefined, [SemanticConventions.LLM_PROVIDER]: providerForResource(this), [SemanticConventions.GEN_AI_OPERATION_NAME]: "chat", ...getResponsesInputMessagesAttributes(body), ...getLLMToolsJSONSchema(body), - [SemanticConventions.RAW_INPUT]: safelyJSONStringify(body) ?? "", + [SemanticConventions.RAW_INPUT]: safelyJSONStringify(telemetryBody) ?? "", }, }, ); @@ -775,6 +786,18 @@ function getLLMInputMessagesAttributes( }; } +/** Remove MCP credentials from telemetry without mutating transport objects. */ +function sanitizeToolDefinitions(tools: { type: string }[]): { type: string }[] { + return tools.map((tool) => { + if (tool.type !== "mcp") return tool; + // MCP arrived after the SDK build floor. All headers are omitted because + // custom header names can carry credentials too. + const { authorization: _authorization, headers: _headers, ...definition } = + tool as { type: string; authorization?: string; headers?: Record }; + return definition; + }); +} + /** * Converts tool definitions into a JSON blob */ @@ -785,7 +808,7 @@ function getLLMToolsJSONSchema( return {}; } return { - [SemanticConventions.LLM_TOOLS]: safelyJSONStringify(body.tools) ?? "[]", + [SemanticConventions.LLM_TOOLS]: safelyJSONStringify(sanitizeToolDefinitions(body.tools)) ?? "[]", }; } @@ -913,7 +936,6 @@ function getStreamMetadataAttributes(chunks: (ChatCompletionChunk | Completion)[ const finishReasons = new Map(); for (const chunk of chunks) { if (chunk.model) { - attributes[SemanticConventions.LLM_MODEL_NAME] = chunk.model; attributes[SemanticConventions.GEN_AI_RESPONSE_MODEL] = chunk.model; } if (chunk.id) attributes[SemanticConventions.GEN_AI_RESPONSE_ID] = chunk.id; @@ -1023,132 +1045,75 @@ function getEmbeddingEmbeddingsAttributes( }, {} as Attributes); } -/** - * Consumes the stream chunks and adds them to the span - */ +// Structural union supports modern custom tools while compiling against SDK 4. +type StreamToolCall = { + index: number; + id?: string; + type?: string; + function?: { name?: string; arguments?: string }; + custom?: { name?: string; input?: string }; +}; + +/** Accumulate one tool by its stable SDK index, never its position in a chunk. */ +function accumulateToolCall( + tools: Map, + delta: StreamToolCall, +): void { + const tool = tools.get(delta.index) ?? { index: delta.index }; + if (delta.id !== undefined) tool.id = delta.id; + if (delta.type !== undefined) tool.type = delta.type; + if (delta.function) { + tool.function ??= { name: "", arguments: "" }; + tool.function.name = (tool.function.name ?? "") + (delta.function.name ?? ""); + tool.function.arguments = (tool.function.arguments ?? "") + (delta.function.arguments ?? ""); + } + if (delta.custom) { + tool.custom ??= { name: "", input: "" }; + tool.custom.name = (tool.custom.name ?? "") + (delta.custom.name ?? ""); + tool.custom.input = (tool.custom.input ?? "") + (delta.custom.input ?? ""); + } + tools.set(delta.index, tool); +} + +/** Consume the first choice, retaining SDK chunks unchanged for the caller. */ async function consumeChatCompletionStreamChunks( stream: Iterable, span: Span, ) { - // --- ADD LOG --- - // diag.debug(`@traceai/openai: consumeChatCompletionStreamChunks CALLED for span: ${span.spanContext().spanId}`); let streamResponse = ""; - // Tool and function call attributes can also arrive in the stream - // NB: the tools and function calls arrive in partial diffs - // So the final tool and function calls need to be aggregated - // across chunks - const toolAndFunctionCallAttributes: Attributes = {}; - // The first message is for the assistant response so we start at 1 + const tools = new Map(); + let functionName = ""; + let functionArguments = ""; const allChunks: ChatCompletionChunk[] = []; for await (const chunk of stream) { - allChunks.push(chunk); // Store all chunks - if (chunk.choices.length <= 0) { - continue; - } + allChunks.push(chunk); const choice = chunk.choices[0]; - if (choice.delta.content) { - streamResponse += choice.delta.content; - } - // Accumulate the tool and function call attributes - const toolAndFunctionCallAttributesDiff = - getToolAndFunctionCallAttributesFromStreamChunk(chunk); - for (const [key, value] of Object.entries( - toolAndFunctionCallAttributesDiff, - )) { - if (isString(toolAndFunctionCallAttributes[key]) && isString(value)) { - toolAndFunctionCallAttributes[key] += value; - } else if (isString(value)) { - toolAndFunctionCallAttributes[key] = value; - } + if (!choice) continue; + streamResponse += choice.delta.content ?? ""; + for (const tool of choice.delta.tool_calls ?? []) { + accumulateToolCall(tools, tool); } + functionName += choice.delta.function_call?.name ?? ""; + functionArguments += choice.delta.function_call?.arguments ?? ""; } - // Build the output message as a JSON blob const outputMessage: Record = { role: "assistant", content: streamResponse, }; - // Aggregate tool calls from stream attributes into structured format - const toolCalls: Record> = {}; - for (const [key, value] of Object.entries(toolAndFunctionCallAttributes)) { - // Parse tool call attributes from the nested format - const toolCallMatch = key.match(/^message\.tool_calls\.(\d+)\./); - if (toolCallMatch) { - const idx = parseInt(toolCallMatch[1]); - if (!toolCalls[idx]) toolCalls[idx] = {}; - if (key.endsWith("tool_call.id")) toolCalls[idx].id = value; - if (key.endsWith("tool_call.function.name")) { - if (!toolCalls[idx].function) toolCalls[idx].function = {}; - (toolCalls[idx].function as Record).name = value; - } - if (key.endsWith("tool_call.function.arguments")) { - if (!toolCalls[idx].function) toolCalls[idx].function = {}; - (toolCalls[idx].function as Record).arguments = value; - } - } - // Handle legacy function_call - if (key === SemanticConventions.MESSAGE_FUNCTION_CALL_NAME) { - outputMessage.function_call_name = value; - } - if (key === SemanticConventions.MESSAGE_FUNCTION_CALL_ARGUMENTS_JSON) { - outputMessage.function_call_arguments = value; - } - } - const toolCallArray = Object.values(toolCalls); - if (toolCallArray.length > 0) { - outputMessage.tool_calls = toolCallArray; + if (tools.size) { + outputMessage.tool_calls = [...tools.entries()] + .sort(([a], [b]) => a - b) + .map(([, { index: _index, ...tool }]) => tool); } + if (functionName) outputMessage.function_call_name = functionName; + if (functionArguments) outputMessage.function_call_arguments = functionArguments; - const attributes: Attributes = { + span.setAttributes({ ...getStreamMetadataAttributes(allChunks), [SemanticConventions.OUTPUT_VALUE]: streamResponse, [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.TEXT, [SemanticConventions.LLM_OUTPUT_MESSAGES]: safelyJSONStringify([outputMessage]) ?? "[]", [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(allChunks) ?? "", - }; - span.setAttributes(attributes); + }); span.setStatus({ code: SpanStatusCode.OK }); - // --- ADD LOG --- - // diag.debug(`@traceai/openai: consumeChatCompletionStreamChunks span ENDED: ${span.spanContext().spanId}`); } - -/** - * Extracts the semantic attributes from the stream chunk for tool_calls and function_calls - */ -function getToolAndFunctionCallAttributesFromStreamChunk( - chunk: ChatCompletionChunk, -): Attributes { - if (chunk.choices.length <= 0) { - return {}; - } - const choice = chunk.choices[0]; - const attributes: Attributes = {}; - if (choice.delta.tool_calls) { - choice.delta.tool_calls.forEach((toolCall, index) => { - const toolCallIndexPrefix = `${SemanticConventions.MESSAGE_TOOL_CALLS}.${index}.`; - // Add the tool call id if it exists - if (toolCall.id) { - attributes[ - `${toolCallIndexPrefix}${SemanticConventions.TOOL_CALL_ID}` - ] = toolCall.id; - } - // Double check that the tool call has a function - // NB: OpenAI only supports tool calls with functions right now but this may change - if (toolCall.function) { - attributes[ - toolCallIndexPrefix + SemanticConventions.TOOL_CALL_FUNCTION_NAME - ] = toolCall.function.name; - attributes[ - toolCallIndexPrefix + - SemanticConventions.TOOL_CALL_FUNCTION_ARGUMENTS_JSON - ] = toolCall.function.arguments; - } - }); - } - if (choice.delta.function_call) { - attributes[SemanticConventions.MESSAGE_FUNCTION_CALL_NAME] = - choice.delta.function_call.name; - attributes[SemanticConventions.MESSAGE_FUNCTION_CALL_ARGUMENTS_JSON] = - choice.delta.function_call.arguments; - } - return attributes; -} \ No newline at end of file diff --git a/typescript/packages/traceai_openai/test/currentSdkShapes.mts b/typescript/packages/traceai_openai/test/currentSdkShapes.mts index d3ca01d82..1a6fc1481 100644 --- a/typescript/packages/traceai_openai/test/currentSdkShapes.mts +++ b/typescript/packages/traceai_openai/test/currentSdkShapes.mts @@ -2,7 +2,21 @@ import OpenAI from 'openai'; // Compile-only witnesses against the installed released SDK. No vendor calls. declare const client: OpenAI; const storedPrompt: OpenAI.Responses.ResponseCreateParamsNonStreaming = { - model: 'synthetic', prompt: { id: 'pmpt_synthetic' }, + model: 'synthetic', + prompt: { + id: 'pmpt_synthetic', + variables: { + text: 'synthetic', + image: { type: 'input_image', image_url: 'https://example.invalid/synthetic', detail: 'auto' }, + }, + }, +}; +const mcp: OpenAI.Responses.ResponseCreateParamsNonStreaming = { + model: 'synthetic', input: 'synthetic', + tools: [{ + type: 'mcp', server_label: 'synthetic', server_url: 'https://example.invalid/mcp', + authorization: 'synthetic', headers: { Authorization: 'Bearer synthetic' }, + }], }; const custom: OpenAI.Chat.Completions.ChatCompletionMessageCustomToolCall = { id: 'call-synthetic', type: 'custom', custom: { name: 'code_exec', input: 'print(1)' }, @@ -18,4 +32,17 @@ const request: OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming = const stream: OpenAI.Completions.CompletionCreateParamsStreaming = { model: 'synthetic', prompt: 'synthetic', stream: true, stream_options: { include_usage: true }, }; -void [client.responses.create(storedPrompt), client.chat.completions.create(request), client.completions.create(stream)]; +const customDelta: OpenAI.Chat.Completions.ChatCompletionChunk = { + id: 'chat-synthetic', created: 1, object: 'chat.completion.chunk', model: 'synthetic', + choices: [{ index: 0, delta: { + tool_calls: [{ index: 0, id: 'call-synthetic', type: 'custom', custom: { name: 'code_exec', input: 'print(' } }], + }, finish_reason: null }], +}; +const parallelDelta: OpenAI.Chat.Completions.ChatCompletionChunk = { + id: 'chat-synthetic', created: 1, object: 'chat.completion.chunk', model: 'synthetic', + choices: [{ index: 0, delta: { + tool_calls: [{ index: 1, id: 'call-b', type: 'function', function: { name: 'second', arguments: '{"b":' } }], + }, finish_reason: null }], +}; +void [client.responses.create(storedPrompt), client.responses.create(mcp), + client.chat.completions.create(request), client.completions.create(stream), customDelta, parallelDelta]; diff --git a/typescript/packages/traceai_openai/test/reviewRegressions.cjs b/typescript/packages/traceai_openai/test/reviewRegressions.cjs new file mode 100644 index 000000000..1fc90aa54 --- /dev/null +++ b/typescript/packages/traceai_openai/test/reviewRegressions.cjs @@ -0,0 +1,562 @@ +const { test, after, afterEach } = require("node:test"); +const assert = require("node:assert/strict"); +const { createRequire } = require("node:module"); +const path = require("node:path"); +const req = createRequire( + path.resolve(process.env.OPENAI_CONSUMER_DIR, "package.json"), +); +const OpenAI = req("openai"); +const { OpenAIInstrumentation } = req("@traceai/openai"); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req( + "@opentelemetry/sdk-trace-base", +); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ + spanProcessors: [new SimpleSpanProcessor(exporter)], +}); +let inst; +function instrument(config = {}) { + inst = new OpenAIInstrumentation({ + instrumentationConfig: { enabled: false }, + traceConfig: config, + }); + inst.setTracerProvider(provider); + inst.manuallyInstrument(OpenAI); +} +afterEach(async () => { + inst?.disable(); + inst = undefined; + await provider.forceFlush(); + exporter.reset(); +}); +after(() => provider.shutdown()); +function span() { + const spans = exporter.getFinishedSpans(); + assert.equal(spans.length, 1); + return spans[0]; +} +const response = { + id: "resp-synthetic", + object: "response", + status: "completed", + model: "resolved-model", + output: [], + usage: { input_tokens: 2, output_tokens: 3, total_tokens: 5 }, +}; +const chat = { + id: "chat-synthetic", + object: "chat.completion", + model: "resolved-model", + choices: [ + { + index: 0, + message: { role: "assistant", content: "synthetic" }, + finish_reason: "stop", + }, + ], +}; +function client(body, sse = false) { + return new OpenAI({ + apiKey: "synthetic-not-a-key", + maxRetries: 0, + fetch: async () => + new Response( + sse + ? body.map((c) => "data: " + JSON.stringify(c) + "\n\n").join("") + + "data: [DONE]\n\n" + : JSON.stringify(body), + { + headers: { + "content-type": sse ? "text/event-stream" : "application/json", + }, + }, + ), + }); +} +function assertNoMarker(marker) { + const attrs = span().attributes; + const leaked = Object.keys(attrs).filter((k) => + JSON.stringify(attrs[k]).includes(marker), + ); + assert.deepEqual(leaked, [], `private marker found in ${leaked.join(", ")}`); +} +for (const operation of ["chat", "completions"]) + test(`${operation}: streaming response must not overwrite requested model`, async () => { + instrument(); + const c = client( + [ + { + id: "synthetic", + object: + operation === "chat" ? "chat.completion.chunk" : "text_completion", + model: "resolved-model", + choices: [ + { + index: 0, + ...(operation === "chat" + ? { delta: { content: "synthetic" } } + : { text: "synthetic" }), + finish_reason: "stop", + }, + ], + }, + ], + true, + ); + const s = await (operation === "chat" + ? c.chat.completions.create({ + model: "requested-alias", + messages: [], + stream: true, + }) + : c.completions.create({ + model: "requested-alias", + prompt: "synthetic", + stream: true, + })); + for await (const chunk of s) { + } + assert.equal(span().attributes["gen_ai.response.model"], "resolved-model"); + assert.equal(span().attributes["gen_ai.request.model"], "requested-alias"); + }); +if (OpenAI.Responses) { + for (const flag of ["hideInputs", "hideInputText", "hideInputImages"]) + test(`Responses stored prompt variables respect ${flag}`, async () => { + const marker = "SYNTHETIC-PRIVATE-PROMPT"; + const variable = + flag === "hideInputImages" + ? { + type: "input_image", + image_url: "https://example.invalid/" + marker, + detail: "auto", + } + : marker; + const c = client(response); + const body = { + model: "synthetic", + prompt: { id: "pmpt_synthetic", variables: { value: variable } }, + }; + await c.responses.create(body); + instrument({ [flag]: true }); + await c.responses.create(body); + assertNoMarker(marker); + }); + test("Responses MCP credentials are never exported even when inputs are hidden", async () => { + const marker = "SYNTHETIC-MCP-CREDENTIAL"; + const body = { + model: "synthetic", + input: "synthetic", + tools: [ + { + type: "mcp", + server_label: "synthetic", + server_url: "https://example.invalid/mcp", + authorization: marker, + headers: { Authorization: "Bearer " + marker }, + }, + ], + }; + const c = client(response); + await c.responses.create(body); + instrument({ hideInputs: true, hideOutputs: true }); + await c.responses.create(body); + assertNoMarker(marker); + }); + test("streamed custom tool call preserves its name and complete input", async () => { + const c = client( + [ + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: "call-synthetic", + type: "custom", + custom: { name: "code_exec", input: "print(" }, + }, + ], + }, + finish_reason: null, + }, + ], + }, + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { tool_calls: [{ index: 0, custom: { input: "1)" } }] }, + finish_reason: "tool_calls", + }, + ], + }, + ], + true, + ); + const body = { + model: "synthetic", + messages: [], + stream: true, + tools: [{ type: "custom", custom: { name: "code_exec" } }], + }; + const baseline = []; + for await (const x of await c.chat.completions.create(body)) + baseline.push(x); + instrument(); + const actual = []; + for await (const x of await c.chat.completions.create(body)) actual.push(x); + assert.deepEqual(actual, baseline); + const tools = JSON.parse(span().attributes["gen_ai.output.messages"])[0] + .tool_calls; + assert.deepEqual(tools, [ + { + id: "call-synthetic", + type: "custom", + custom: { name: "code_exec", input: "print(1)" }, + }, + ]); + }); + for (const direction of ["input", "output"]) + test(`custom tool text is hidden in structured ${direction} messages`, async () => { + const marker = "SYNTHETIC-PRIVATE-CUSTOM"; + const tool = { + id: "call-synthetic", + type: "custom", + custom: { name: "code_exec", input: marker }, + }; + const c = client( + direction === "output" + ? { + ...chat, + choices: [ + { + index: 0, + message: { + role: "assistant", + content: null, + tool_calls: [tool], + }, + finish_reason: "tool_calls", + }, + ], + } + : chat, + ); + instrument({ + [direction === "input" ? "hideInputText" : "hideOutputText"]: true, + }); + await c.chat.completions.create({ + model: "synthetic", + messages: + direction === "input" + ? [ + { role: "assistant", content: null, tool_calls: [tool] }, + { role: "tool", tool_call_id: tool.id, content: "synthetic" }, + ] + : [], + }); + assertNoMarker(marker); + }); +} +test("parallel streamed function deltas remain separate by tool index", async () => { + const c = client( + [ + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: "call-a", + type: "function", + function: { name: "first", arguments: '{"a":' }, + }, + ], + }, + finish_reason: null, + }, + ], + }, + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 1, + id: "call-b", + type: "function", + function: { name: "second", arguments: '{"b":' }, + }, + ], + }, + finish_reason: null, + }, + ], + }, + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { index: 0, function: { arguments: "1}" } }, + { index: 1, function: { arguments: "2}" } }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }, + ], + true, + ); + instrument(); + for await (const x of await c.chat.completions.create({ + model: "synthetic", + messages: [], + stream: true, + tools: [ + { + type: "function", + function: { + name: "first", + parameters: { type: "object", properties: { a: { type: "number" } } }, + }, + }, + { + type: "function", + function: { + name: "second", + parameters: { type: "object", properties: { b: { type: "number" } } }, + }, + }, + ], + })) { + } + const tools = JSON.parse(span().attributes["gen_ai.output.messages"])[0] + .tool_calls; + assert.equal(tools.length, 2); + assert.equal(tools[0].id, "call-a"); + assert.deepEqual(tools[0].function, { name: "first", arguments: '{"a":1}' }); + assert.equal(tools[1].id, "call-b"); + assert.deepEqual(tools[1].function, { name: "second", arguments: '{"b":2}' }); +}); + +if (OpenAI.Responses) { + for (const config of [{}, { hideInputs: true, hideOutputs: true }]) { + test(`MCP telemetry is credential-free without changing transport: ${JSON.stringify(config)}`, async () => { + const marker = "SYNTHETIC-MCP-SECRET"; + const tool = { + type: "mcp", + server_label: "synthetic", + server_url: "https://example.invalid/mcp", + authorization: marker, + headers: { Authorization: marker, "X-Custom-Auth": marker }, + }; + const body = { model: "synthetic", input: "public", tools: [tool] }; + const before = structuredClone(body); + let sent; + const c = new OpenAI({ + apiKey: "synthetic-not-a-key", + maxRetries: 0, + fetch: async (_url, init) => { + sent = JSON.parse(init.body); + // The server echoes request configuration. Only output fields belong in + // telemetry output; tool credentials must not leak through an echo. + return new Response(JSON.stringify({ ...response, tools: [tool] }), { + headers: { "content-type": "application/json" }, + }); + }, + }); + instrument(config); + const actual = await c.responses.create(body); + assert.deepEqual(sent, before); + assert.deepEqual(body, before); + assert.deepEqual(actual.tools, [tool]); + assertNoMarker(marker); + const attrs = span().attributes; + const expected = [ + { + type: "mcp", + server_label: "synthetic", + server_url: "https://example.invalid/mcp", + }, + ]; + assert.deepEqual(JSON.parse(attrs["gen_ai.tool.definitions"]), expected); + assert.deepEqual( + JSON.parse(attrs["gen_ai.request.parameters"]).tools, + expected, + ); + }); + } + for (const flag of [ + null, + "hideInputs", + "hideInputText", + "hideInputImages", + "hideInputMessages", + ]) { + test(`stored prompt values have a maskable input attribute: ${flag ?? "capture enabled"}`, async () => { + const marker = "SYNTHETIC-STORED-PROMPT"; + const body = { + model: "synthetic", + prompt: { + id: "pmpt_synthetic", + version: "2", + variables: { + text: marker, + image: { + type: "input_image", + image_url: "https://example.invalid/" + marker, + detail: "auto", + }, + }, + }, + }; + const before = structuredClone(body); + let sent; + const c = new OpenAI({ + apiKey: "synthetic-not-a-key", + maxRetries: 0, + fetch: async (_url, init) => { + sent = JSON.parse(init.body); + return new Response( + JSON.stringify({ ...response, prompt: body.prompt }), + { headers: { "content-type": "application/json" } }, + ); + }, + }); + instrument(flag ? { [flag]: true } : {}); + await c.responses.create(body); + assert.deepEqual(sent, before); + assert.deepEqual(body, before); + assert.deepEqual( + JSON.parse(span().attributes["gen_ai.request.parameters"]).prompt, + { id: "pmpt_synthetic", version: "2" }, + ); + if (flag) assertNoMarker(marker); + else assert.ok(JSON.stringify(span().attributes).includes(marker)); + }); + } + test("streamed custom inputs are also covered by granular output hiding", async () => { + const marker = "SYNTHETIC-STREAM-CUSTOM"; + const c = client( + [ + { + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: "call-custom", + type: "custom", + custom: { name: "code_exec", input: marker }, + }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }, + ], + true, + ); + instrument({ hideOutputText: true }); + const chunks = []; + for await (const chunk of await c.chat.completions.create({ + model: "synthetic", + messages: [], + stream: true, + })) + chunks.push(chunk); + assert.equal(chunks[0].choices[0].delta.tool_calls[0].custom.input, marker); + assertNoMarker(marker); + }); +} + +test("out-of-order and sparse stream indices preserve tool identity", async () => { + const deltas = [ + [ + { + index: 2, + id: "call-c", + type: "function", + function: { name: "thi", arguments: '{"c":' }, + }, + ], + [ + { + index: 0, + id: "call-a", + type: "function", + function: { name: "first", arguments: '{"a":' }, + }, + ], + [ + { + index: 2, + id: "call-c", + type: "function", + function: { name: "rd", arguments: "3}" }, + }, + { index: 0, function: { arguments: "1}" } }, + ], + ]; + const chunks = deltas.map((tool_calls) => ({ + id: "chat-synthetic", + object: "chat.completion.chunk", + model: "synthetic", + choices: [{ index: 0, delta: { tool_calls }, finish_reason: null }], + })); + const c = client(chunks, true); + instrument(); + const actual = []; + for await (const chunk of await c.chat.completions.create({ + model: "synthetic", + messages: [], + stream: true, + })) + actual.push(chunk); + assert.deepEqual(actual, chunks); + assert.deepEqual( + JSON.parse(span().attributes["gen_ai.output.messages"])[0].tool_calls, + [ + { + id: "call-a", + type: "function", + function: { name: "first", arguments: '{"a":1}' }, + }, + { + id: "call-c", + type: "function", + function: { name: "third", arguments: '{"c":3}' }, + }, + ], + ); +}); From 8b1d0a3893bc9e21a7450690b505ad45dbf8ab27 Mon Sep 17 00:00:00 2001 From: Nikhil Pareek Date: Mon, 2 Nov 2026 04:23:52 -0800 Subject: [PATCH 3/4] fix(openai): address second recall review findings Mask prediction/suffix content, preserve screenshot structure and request models, select streamed choice zero by SDK index, and declare the directly imported OTel core dependency. Cover real SDK bounds and strict non-hoisted CJS/ESM imports. Co-Authored-By: Claude Code --- .github/workflows/openai-sdk-contracts.yml | 17 +- .../packages/traceai_openai/package.json | 1 + .../src/__tests__/openai.test.ts | 28 ++-- .../src/__tests__/responseAttributes.test.ts | 6 +- .../traceai_openai/src/instrumentation.ts | 16 +- .../traceai_openai/src/responseAttributes.ts | 7 +- .../traceai_openai/test/choiceIndexes.cjs | 56 +++++++ .../traceai_openai/test/currentSdkShapes.mts | 22 +++ .../traceai_openai/test/reviewRoundTwo.cjs | 153 ++++++++++++++++++ .../traceai_openai/test/strictImports.cjs | 20 +++ .../traceai_openai/test/vercelGateway.cjs | 3 +- typescript/pnpm-lock.yaml | 3 + 12 files changed, 306 insertions(+), 26 deletions(-) create mode 100644 typescript/packages/traceai_openai/test/choiceIndexes.cjs create mode 100644 typescript/packages/traceai_openai/test/reviewRoundTwo.cjs create mode 100644 typescript/packages/traceai_openai/test/strictImports.cjs diff --git a/.github/workflows/openai-sdk-contracts.yml b/.github/workflows/openai-sdk-contracts.yml index 04d1a1774..f67b8b487 100644 --- a/.github/workflows/openai-sdk-contracts.yml +++ b/.github/workflows/openai-sdk-contracts.yml @@ -54,6 +54,19 @@ jobs: @opentelemetry/exporter-trace-otlp-http@0.202.0 \ @opentelemetry/exporter-trace-otlp-proto@0.202.0 @opentelemetry/otlp-transformer@0.202.0 \ typescript@5.8.3 @types/node@22.15.29 + - name: Install a non-hoisted package consumer + run: | + mkdir -p "$RUNNER_TEMP/strict-consumer" + cat > "$RUNNER_TEMP/strict-consumer/package.json" < { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"This is a test."}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo"}", "gen_ai.response.finish_reasons": "["stop"]", "gen_ai.response.id": "chatcmpl-8adq9JloOzNZ9TyuzrKyLpGXexh6p", @@ -294,9 +294,9 @@ describe("OpenAIInstrumentation", () => { async (): Promise => { const iterator = () => (async function* () { - yield { choices: [{ delta: { content: "This is " } }] }; - yield { choices: [{ delta: { content: "a test." } }] }; - yield { choices: [{ delta: { finish_reason: "stop" } }] }; + yield { choices: [{ index: 0, delta: { content: "This is " } }] }; + yield { choices: [{ index: 0, delta: { content: "a test." } }] }; + yield { choices: [{ index: 0, delta: {}, finish_reason: "stop" }] }; })(); const controller = new AbortController(); return new Stream(iterator, controller); @@ -327,12 +327,13 @@ describe("OpenAIInstrumentation", () => { "gen_ai.provider.name": "openai", "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","stream":true}", + "gen_ai.response.finish_reasons": "["stop"]", "input.mime_type": "application/json", "input.value": "{"messages":[{"role":"user","content":"Say this is a test"}],"model":"gpt-3.5-turbo","stream":true}", "output.mime_type": "text/plain", "output.value": "This is a test.", "raw.input": "{"messages":[{"role":"user","content":"Say this is a test"}],"model":"gpt-3.5-turbo","stream":true}", - "raw.output": "[{"choices":[{"delta":{"content":"This is "}}]},{"choices":[{"delta":{"content":"a test."}}]},{"choices":[{"delta":{"finish_reason":"stop"}}]}]", + "raw.output": "[{"choices":[{"index":0,"delta":{"content":"This is "}}]},{"choices":[{"index":0,"delta":{"content":"a test."}}]},{"choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}]", } `); }); @@ -571,7 +572,7 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","tool_calls":[{"id":"call_5ERYvu4iTGSvDlcDQjDP3g3J","type":"function","function":{"name":"getCurrentLocation","arguments":"{}"}}]}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"tool_choice":"auto","stream":false}", "gen_ai.response.finish_reasons": "["tool_calls"]", "gen_ai.response.id": "chatcmpl-8hhqZDFTRD0vzExhqWnMLE7viVl7E", @@ -596,7 +597,7 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","tool_calls":[{"id":"call_0LCdYLkdRUt3rV3dawoIFHBf","type":"function","function":{"name":"getWeather","arguments":"{\\n \\"location\\": \\"Boston\\"\\n}"}}]}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"tool_choice":"auto","stream":false}", "gen_ai.response.finish_reasons": "["tool_calls"]", "gen_ai.response.id": "chatcmpl-8hhsP9eAplUFYB3mHUJxBkq7IwnjZ", @@ -621,7 +622,7 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"The weather in Boston this week is expected to be rainy with a temperature of 52 degrees."}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo","tools":[{"type":"function","function":{"name":"getCurrentLocation","parameters":{"type":"object","properties":{}},"description":"Get the current location of the user."}},{"type":"function","function":{"name":"getWeather","description":"Get the weather for a location.","parameters":{"type":"object","properties":{"location":{"type":"string"}}}}}],"tool_choice":"auto","stream":false}", "gen_ai.response.finish_reasons": "["stop"]", "gen_ai.response.id": "chatcmpl-8hhtfzSD33tsG7XJiBg4F9MqnXKDp", @@ -986,7 +987,7 @@ describe("OpenAIInstrumentation", () => { "gen_ai.operation.name": "chat", "gen_ai.output.messages": "[{"role":"assistant","content":"This is a test."}]", "gen_ai.provider.name": "openai", - "gen_ai.request.model": "gpt-3.5-turbo-0613", + "gen_ai.request.model": "gpt-3.5-turbo", "gen_ai.request.parameters": "{"model":"gpt-3.5-turbo"}", "gen_ai.response.finish_reasons": "["stop"]", "gen_ai.response.id": "chatcmpl-8adq9JloOzNZ9TyuzrKyLpGXexh6p", @@ -1389,9 +1390,9 @@ describe("AzureOpenAIInstrumentation", () => { async (): Promise => { const iterator = () => (async function* () { - yield { choices: [{ delta: { content: "This is " } }] }; - yield { choices: [{ delta: { content: "a test." } }] }; - yield { choices: [{ delta: { finish_reason: "stop" } }] }; + yield { choices: [{ index: 0, delta: { content: "This is " } }] }; + yield { choices: [{ index: 0, delta: { content: "a test." } }] }; + yield { choices: [{ index: 0, delta: {}, finish_reason: "stop" }] }; })(); const controller = new AbortController(); return new Stream(iterator, controller); @@ -1422,12 +1423,13 @@ describe("AzureOpenAIInstrumentation", () => { "gen_ai.provider.name": "azure", "gen_ai.request.model": "gpt-35-turbo", "gen_ai.request.parameters": "{"model":"gpt-35-turbo","stream":true}", + "gen_ai.response.finish_reasons": "["stop"]", "input.mime_type": "application/json", "input.value": "{"messages":[{"role":"user","content":"Say this is a test"}],"model":"gpt-35-turbo","stream":true}", "output.mime_type": "text/plain", "output.value": "This is a test.", "raw.input": "{"messages":[{"role":"user","content":"Say this is a test"}],"model":"gpt-35-turbo","stream":true}", - "raw.output": "[{"choices":[{"delta":{"content":"This is "}}]},{"choices":[{"delta":{"content":"a test."}}]},{"choices":[{"delta":{"finish_reason":"stop"}}]}]", + "raw.output": "[{"choices":[{"index":0,"delta":{"content":"This is "}}]},{"choices":[{"index":0,"delta":{"content":"a test."}}]},{"choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}]", } `); }); diff --git a/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts b/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts index ed2dbc585..bae595f2a 100644 --- a/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts +++ b/typescript/packages/traceai_openai/src/__tests__/responseAttributes.test.ts @@ -178,7 +178,7 @@ describe('Response Attributes Functions', () => { { type: 'computer_call_output', call_id: 'computer_123', - output: { screenshot: 'base64_data...' }, + output: { type: 'computer_screenshot', image_url: 'data:image/png;base64,synthetic' }, }, ], }; @@ -189,7 +189,9 @@ describe('Response Attributes Functions', () => { expect(messages).toHaveLength(1); expect(messages[0].role).toBe('tool'); expect(messages[0].tool_call_id).toBe('computer_123'); - expect(messages[0].content).toBe('{"screenshot":"base64_data..."}'); + expect(messages[0].content).toEqual([ + { type: 'input_image', image_url: 'data:image/png;base64,synthetic' }, + ]); }); it('should handle file search call items without results', () => { diff --git a/typescript/packages/traceai_openai/src/instrumentation.ts b/typescript/packages/traceai_openai/src/instrumentation.ts index 590af2b00..f88fd29be 100644 --- a/typescript/packages/traceai_openai/src/instrumentation.ts +++ b/typescript/packages/traceai_openai/src/instrumentation.ts @@ -219,7 +219,8 @@ export class OpenAIInstrumentation extends InstrumentationBase{ ...args: Parameters ) { const body = args[0]; - const { messages: _messages, ...invocationParameters } = body; + // Prediction text is input content, captured only in maskable payloads. + const { messages: _messages, prediction: _prediction, ...invocationParameters } = body; // diag.debug("@traceai/openai: ChatCompletion patch CALLED. Starting span..."); @@ -283,8 +284,6 @@ export class OpenAIInstrumentation extends InstrumentationBase{ span.setAttributes({ [SemanticConventions.OUTPUT_VALUE]: JSON.stringify(result), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.JSON, - // Override the model from the value sent by the server - [SemanticConventions.LLM_MODEL_NAME]: result.model, [SemanticConventions.GEN_AI_RESPONSE_MODEL]: result.model, [SemanticConventions.GEN_AI_RESPONSE_ID]: result.id, [SemanticConventions.GEN_AI_RESPONSE_FINISH_REASONS]: JSON.stringify( @@ -323,7 +322,8 @@ export class OpenAIInstrumentation extends InstrumentationBase{ ...args: Parameters ) { const body = args[0]; - const { prompt: _prompt, ...invocationParameters } = body; + // Suffix is input text, not an unmasked tuning parameter. + const { prompt: _prompt, suffix: _suffix, ...invocationParameters } = body; const span = instrumentation.fiTracer.startSpan( `OpenAI Completions`, { @@ -369,7 +369,6 @@ export class OpenAIInstrumentation extends InstrumentationBase{ span.setAttributes({ [SemanticConventions.OUTPUT_VALUE]: JSON.stringify(result), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.JSON, - [SemanticConventions.LLM_MODEL_NAME]: result.model, [SemanticConventions.GEN_AI_RESPONSE_MODEL]: result.model, [SemanticConventions.GEN_AI_RESPONSE_ID]: result.id, [SemanticConventions.GEN_AI_RESPONSE_FINISH_REASONS]: JSON.stringify( @@ -385,7 +384,9 @@ export class OpenAIInstrumentation extends InstrumentationBase{ result = observeStream(result, span, (chunks) => { span.setAttributes({ ...getStreamMetadataAttributes(chunks), - [SemanticConventions.OUTPUT_VALUE]: chunks.map((chunk) => chunk.choices[0]?.text ?? "").join(""), + [SemanticConventions.OUTPUT_VALUE]: chunks + .map((chunk) => chunk.choices.find((choice) => choice.index === 0)?.text ?? "") + .join(""), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.TEXT, [SemanticConventions.RAW_OUTPUT]: safelyJSONStringify(chunks) ?? "", }); @@ -555,7 +556,6 @@ export class OpenAIInstrumentation extends InstrumentationBase{ span.setAttributes({ [SemanticConventions.OUTPUT_VALUE]: JSON.stringify(output), [SemanticConventions.OUTPUT_MIME_TYPE]: MimeType.JSON, - [SemanticConventions.LLM_MODEL_NAME]: result.model, [SemanticConventions.GEN_AI_RESPONSE_MODEL]: result.model, [SemanticConventions.GEN_AI_RESPONSE_ID]: result.id, ...getResponsesOutputMessagesAttributes(result), @@ -1087,7 +1087,7 @@ async function consumeChatCompletionStreamChunks( const allChunks: ChatCompletionChunk[] = []; for await (const chunk of stream) { allChunks.push(chunk); - const choice = chunk.choices[0]; + const choice = chunk.choices.find((choice) => choice.index === 0); if (!choice) continue; streamResponse += choice.delta.content ?? ""; for (const tool of choice.delta.tool_calls ?? []) { diff --git a/typescript/packages/traceai_openai/src/responseAttributes.ts b/typescript/packages/traceai_openai/src/responseAttributes.ts index 3d1bf2793..d9bb72bcb 100644 --- a/typescript/packages/traceai_openai/src/responseAttributes.ts +++ b/typescript/packages/traceai_openai/src/responseAttributes.ts @@ -46,7 +46,12 @@ function serializeResponseItem( tool_calls: [{ id: item.id, function: { name: item.type, arguments: JSON.stringify(item.action) } }], }; case "computer_call_output": - return { role: "tool", tool_call_id: item.call_id, content: JSON.stringify(item.output) }; + // Retain image structure so image-only privacy rules can recognize it. + return { + role: "tool", + tool_call_id: item.call_id, + content: [{ ...item.output, type: "input_image" }], + }; case "web_search_call": return { role: "assistant", diff --git a/typescript/packages/traceai_openai/test/choiceIndexes.cjs b/typescript/packages/traceai_openai/test/choiceIndexes.cjs new file mode 100644 index 000000000..c3274bd7e --- /dev/null +++ b/typescript/packages/traceai_openai/test/choiceIndexes.cjs @@ -0,0 +1,56 @@ +const { test, after, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const path = require('node:path'); +const req = createRequire(path.join(process.env.OPENAI_CONSUMER_DIR, 'package.json')); +const OpenAI = req('openai'); +const { OpenAIInstrumentation } = req('@traceai/openai'); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req('@opentelemetry/sdk-trace-base'); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); +let inst; +afterEach(async () => { inst?.disable(); await provider.forceFlush(); exporter.reset(); }); +after(() => provider.shutdown()); +for (const route of ['chat', 'completions']) { + for (const together of [false, true]) { + test(`${route}: n=2 ${together ? 'reordered' : 'interleaved'} choices retain stable identity`, async () => { + const choices = [[1, 'B0'], [0, 'A0'], [1, 'B1'], [0, 'A1']].map(([index, text], i) => ({ + index, + ...(route === 'chat' ? { delta: { content: text } } : { text, logprobs: null }), + finish_reason: i > 1 ? 'stop' : null, + })); + const groups = together ? [choices.slice(0, 2), choices.slice(2)] : choices.map(c => [c]); + const chunks = groups.map(choices => ({ id: 'synthetic', created: 1, model: 'server-model', + object: route === 'chat' ? 'chat.completion.chunk' : 'text_completion', choices })); + // Usage-only chunks must still be processed even though they contain no choice zero. + chunks.push({ ...chunks[0], choices: [], usage: { prompt_tokens: 3, completion_tokens: 4, total_tokens: 7 } }); + const client = new OpenAI({ apiKey: 'synthetic-not-a-key', maxRetries: 0, + fetch: async () => new Response( + chunks.map(c => 'data: ' + JSON.stringify(c) + '\n\n').join('') + 'data: [DONE]\n\n', + { headers: { 'content-type': 'text/event-stream' } }), + }); + const invoke = () => route === 'chat' + ? client.chat.completions.create({ model: 'requested-alias', messages: [{ role: 'user', content: 'hello' }], n: 2, stream: true }) + : client.completions.create({ model: 'requested-alias', prompt: 'hello', n: 2, stream: true }); + const baseline = []; + for await (const c of await invoke()) baseline.push(c); + inst = new OpenAIInstrumentation({ instrumentationConfig: { enabled: false } }); + inst.setTracerProvider(provider); + inst.manuallyInstrument(OpenAI); + const actual = []; + for await (const c of await invoke()) actual.push(c); + assert.deepEqual(actual, baseline); + assert.equal(exporter.getFinishedSpans().length, 1); + const attrs = exporter.getFinishedSpans()[0].attributes; + assert.equal(attrs['output.value'], 'A0A1'); + assert.equal(attrs['gen_ai.request.model'], 'requested-alias'); + assert.equal(attrs['gen_ai.response.model'], 'server-model'); + assert.equal(attrs['gen_ai.usage.input_tokens'], 3); + assert.equal(attrs['gen_ai.usage.output_tokens'], 4); + assert.deepEqual(JSON.parse(attrs['raw.output']), chunks); + if (route === 'chat') { + assert.deepEqual(JSON.parse(attrs['gen_ai.output.messages']), [{ role: 'assistant', content: 'A0A1' }]); + } + }); + } +} diff --git a/typescript/packages/traceai_openai/test/currentSdkShapes.mts b/typescript/packages/traceai_openai/test/currentSdkShapes.mts index 1a6fc1481..4e7094aab 100644 --- a/typescript/packages/traceai_openai/test/currentSdkShapes.mts +++ b/typescript/packages/traceai_openai/test/currentSdkShapes.mts @@ -46,3 +46,25 @@ const parallelDelta: OpenAI.Chat.Completions.ChatCompletionChunk = { }; void [client.responses.create(storedPrompt), client.responses.create(mcp), client.chat.completions.create(request), client.completions.create(stream), customDelta, parallelDelta]; + +const predicted = { + model: 'gpt-4.1', messages: [{ role: 'user', content: 'Rewrite this document' }], + prediction: { type: 'content', content: 'SYNTHETIC-PREDICTED-PRIVATE' }, +} satisfies OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming; +const predictedParts = { + ...predicted, prediction: { type: 'content', content: [{ type: 'text', text: 'SYNTHETIC-PREDICTED-PRIVATE' }] }, +} satisfies OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming; +const suffix = { + model: 'gpt-3.5-turbo-instruct', prompt: 'hello', suffix: 'SYNTHETIC-SUFFIX-PRIVATE', +} satisfies OpenAI.Completions.CompletionCreateParamsNonStreaming; +const screenshot = { + model: 'computer-use-preview', input: [{ type: 'computer_call_output', call_id: 'call_synthetic', + output: { type: 'computer_screenshot', image_url: 'https://example.invalid/SYNTHETIC-SCREENSHOT-PRIVATE' }, + }], +} satisfies OpenAI.Responses.ResponseCreateParamsNonStreaming; +const screenshotFile = { + ...screenshot, input: [{ type: 'computer_call_output', call_id: 'call_synthetic', + output: { type: 'computer_screenshot', file_id: 'file-SYNTHETIC-SCREENSHOT-PRIVATE' }, + }], +} satisfies OpenAI.Responses.ResponseCreateParamsNonStreaming; +void [predicted, predictedParts, suffix, screenshot, screenshotFile]; diff --git a/typescript/packages/traceai_openai/test/reviewRoundTwo.cjs b/typescript/packages/traceai_openai/test/reviewRoundTwo.cjs new file mode 100644 index 000000000..5f791203d --- /dev/null +++ b/typescript/packages/traceai_openai/test/reviewRoundTwo.cjs @@ -0,0 +1,153 @@ +const { test, after, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const path = require('node:path'); +const req = createRequire(path.join(process.env.OPENAI_CONSUMER_DIR, 'package.json')); +const OpenAI = req('openai'); +const { OpenAIInstrumentation } = req('@traceai/openai'); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req('@opentelemetry/sdk-trace-base'); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); +let inst; +function instrument(traceConfig = {}) { + inst = new OpenAIInstrumentation({ instrumentationConfig: { enabled: false }, traceConfig }); + inst.setTracerProvider(provider); + inst.manuallyInstrument(OpenAI); +} +afterEach(async () => { + inst?.disable(); + inst = undefined; + await provider.forceFlush(); + exporter.reset(); +}); +after(() => provider.shutdown()); +const chat = { + id: 'chat-synthetic', object: 'chat.completion', created: 1, model: 'server-model', + choices: [{ index: 0, message: { role: 'assistant', content: 'hello' }, finish_reason: 'stop' }], +}; +const completion = { + id: 'completion-synthetic', object: 'text_completion', created: 1, model: 'server-model', + choices: [{ index: 0, text: 'hello', logprobs: null, finish_reason: 'stop' }], +}; +const response = { + id: 'response-synthetic', object: 'response', created_at: 1, status: 'completed', + model: 'server-model', output: [], usage: { input_tokens: 2, output_tokens: 1, total_tokens: 3 }, +}; +function fixture(payload, stream = false) { + let wire; + const client = new OpenAI({ apiKey: 'synthetic-not-a-key', maxRetries: 0, + fetch: async (_url, options) => { + wire = JSON.parse(options.body); + return new Response(stream + ? payload.map(c => 'data: ' + JSON.stringify(c) + '\n\n').join('') + 'data: [DONE]\n\n' + : JSON.stringify(payload), { + headers: { 'content-type': stream ? 'text/event-stream' : 'application/json' }, + }); + }, + }); + return { client, getWire: () => wire }; +} +function span() { + const spans = exporter.getFinishedSpans(); + assert.equal(spans.length, 1); + return spans[0]; +} +function noMarker(marker) { + const attrs = span().attributes; + assert.deepEqual(Object.keys(attrs).filter(k => JSON.stringify(attrs[k]).includes(marker)), [], + 'synthetic private content must be absent from every attribute'); +} +function checkInput(marker, flag) { + if (flag) noMarker(marker); + else assert.ok(span().attributes['raw.input'].includes(marker), 'capture-enabled control'); + const parameters = JSON.parse(span().attributes['gen_ai.request.parameters']); + assert.ok(!JSON.stringify(parameters).includes(marker), 'input content is not invocation metadata'); + assert.ok(parameters.model, 'ordinary request metadata remains available'); +} +for (const route of ['chat', 'completions', ...(OpenAI.Responses ? ['responses'] : [])]) { + test(`${route}: nonstream response preserves requested alias`, async () => { + const payload = route === 'chat' ? chat : route === 'completions' ? completion : response; + const { client } = fixture(payload); + const invoke = () => route === 'chat' + ? client.chat.completions.create({ model: 'requested-alias', messages: [] }) + : route === 'completions' + ? client.completions.create({ model: 'requested-alias', prompt: 'hello' }) + : client.responses.create({ model: 'requested-alias', input: 'hello' }); + const expected = await invoke(); + instrument(); + assert.deepEqual(await invoke(), expected); + assert.equal(span().attributes['gen_ai.response.model'], 'server-model'); + assert.equal(span().attributes['gen_ai.request.model'], 'requested-alias'); + }); +} +for (const flag of ['hideInputs', 'hideInputText', null]) { + test(`completions: suffix content respects ${flag ?? 'capture enabled'}`, async () => { + const marker = 'SYNTHETIC-SUFFIX-PRIVATE'; + const { client, getWire } = fixture(completion); + const body = { model: 'gpt-3.5-turbo-instruct', prompt: 'hello', suffix: marker }; + const before = structuredClone(body); + const expected = await client.completions.create(body); + instrument(flag ? { [flag]: true } : {}); + assert.deepEqual(await client.completions.create(body), expected); + assert.deepEqual(body, before); + assert.deepEqual(getWire(), before); + checkInput(marker, flag); + }); + // SDK 4.0.0 predates both Responses and predicted output requests. + if (OpenAI.Responses) for (const arrayContent of [false, true]) { + test(`chat: ${arrayContent ? 'array' : 'string'} prediction respects ${flag ?? 'capture enabled'}`, async () => { + const marker = 'SYNTHETIC-PREDICTED-PRIVATE'; + const { client, getWire } = fixture(chat); + const body = { model: 'gpt-4.1', messages: [{ role: 'user', content: 'Rewrite this document' }], + prediction: { type: 'content', content: arrayContent ? [{ type: 'text', text: marker }] : marker } }; + const before = structuredClone(body); + const expected = await client.chat.completions.create(body); + instrument(flag ? { [flag]: true } : {}); + assert.deepEqual(await client.chat.completions.create(body), expected); + assert.deepEqual(body, before); + assert.deepEqual(getWire(), before); + checkInput(marker, flag); + }); + } +} +if (OpenAI.Responses) { + test('Responses: streaming preserves requested model alias', async () => { + const { client } = fixture([{ type: 'response.completed', sequence_number: 0, response }], true); + const body = { model: 'requested-alias', input: 'hello', stream: true }; + const expected = []; + for await (const x of await client.responses.create(body)) expected.push(x); + instrument(); + const actual = []; + for await (const x of await client.responses.create(body)) actual.push(x); + assert.deepEqual(actual, expected); + assert.equal(span().attributes['gen_ai.response.model'], 'server-model'); + assert.equal(span().attributes['gen_ai.request.model'], 'requested-alias'); + }); + for (const flag of ['hideInputImages', 'hideInputs', null]) { + for (const field of ['image_url', 'file_id']) { + test(`Responses: computer screenshot ${field} respects ${flag ?? 'capture enabled'}`, async () => { + const marker = 'SYNTHETIC-SCREENSHOT-PRIVATE'; + const { client, getWire } = fixture(response); + const output = { type: 'computer_screenshot', + [field]: field === 'image_url' ? 'https://example.invalid/' + marker : 'file-' + marker }; + const body = { model: 'computer-use-preview', input: [ + { role: 'user', content: 'keep this visible text' }, + { type: 'computer_call_output', call_id: 'call_synthetic', output }, + ] }; + const before = structuredClone(body); + const expected = await client.responses.create(body); + instrument(flag ? { [flag]: true } : {}); + assert.deepEqual(await client.responses.create(body), expected); + assert.deepEqual(body, before); + assert.deepEqual(getWire(), before); + checkInput(marker, flag); + if (flag !== 'hideInputs') { + const messages = JSON.parse(span().attributes['gen_ai.input.messages']); + assert.equal(messages[0].content, 'keep this visible text'); + assert.equal(messages[1].tool_call_id, 'call_synthetic'); + assert.deepEqual(messages[1].content, flag ? [] : [{ ...output, type: 'input_image' }]); + } + }); + } + } +} diff --git a/typescript/packages/traceai_openai/test/strictImports.cjs b/typescript/packages/traceai_openai/test/strictImports.cjs new file mode 100644 index 000000000..6e642370a --- /dev/null +++ b/typescript/packages/traceai_openai/test/strictImports.cjs @@ -0,0 +1,20 @@ +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawnSync } = require('node:child_process'); + +// Run in a clean pnpm consumer with both hoisting settings disabled. Importing +// from the workspace or an npm consumer would hide missing direct dependencies. +const cwd = process.env.OPENAI_STRICT_CONSUMER_DIR; +assert.ok(cwd, 'OPENAI_STRICT_CONSUMER_DIR must name the non-hoisted consumer'); +for (const mode of ['commonjs', 'module']) { + test(`non-hoisted ${mode} import loads the packed instrumentation`, () => { + const load = mode === 'module' + ? 'import { OpenAIInstrumentation } from "@traceai/openai";' + : 'const { OpenAIInstrumentation } = require("@traceai/openai");'; + const result = spawnSync(process.execPath, [`--input-type=${mode}`, '-e', + `${load} if (typeof OpenAIInstrumentation !== "function") throw new Error("Missing constructor");`], + { cwd, encoding: 'utf8' }); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); + }); +} diff --git a/typescript/packages/traceai_openai/test/vercelGateway.cjs b/typescript/packages/traceai_openai/test/vercelGateway.cjs index b7d3c0362..ed6c87c1d 100644 --- a/typescript/packages/traceai_openai/test/vercelGateway.cjs +++ b/typescript/packages/traceai_openai/test/vercelGateway.cjs @@ -83,7 +83,8 @@ async function check(expected, kind = 'chat', error = false, streaming = false) assert.equal(attrs['gen_ai.provider.name'], expected); if (!streaming) assert.equal(span.status.code, error ? 2 : 1); if (kind !== 'embeddings') { - assert.equal(attrs[SC.LLM_MODEL_NAME], error || streaming ? requestedModel : responseModel); + assert.equal(attrs[SC.LLM_MODEL_NAME], requestedModel); + if (!error && !streaming) assert.equal(attrs['gen_ai.response.model'], responseModel); if (!error && !streaming) { assert.equal(attrs[SC.LLM_TOKEN_COUNT_PROMPT], 3); assert.equal(attrs[SC.LLM_TOKEN_COUNT_COMPLETION], 2); diff --git a/typescript/pnpm-lock.yaml b/typescript/pnpm-lock.yaml index b79809fa6..c5fb93848 100644 --- a/typescript/pnpm-lock.yaml +++ b/typescript/pnpm-lock.yaml @@ -1607,6 +1607,9 @@ importers: '@opentelemetry/api': specifier: ^1.9.0 version: 1.9.0 + '@opentelemetry/core': + specifier: ^2.0.1 + version: 2.0.1(@opentelemetry/api@1.9.0) '@opentelemetry/instrumentation': specifier: ^0.202.0 version: 0.202.0(@opentelemetry/api@1.9.0) From 046efcb0c29519c2f63c29430f44e3022473e764 Mon Sep 17 00:00:00 2001 From: Nikhil Pareek Date: Mon, 2 Nov 2026 13:17:21 -0800 Subject: [PATCH 4/4] fix(openai): redact MCP URL credentials and serialize Responses custom tools Co-Authored-By: Claude Code --- .github/workflows/openai-sdk-contracts.yml | 1 + .../traceai_openai/src/instrumentation.ts | 42 ++- .../traceai_openai/src/responseAttributes.ts | 17 ++ .../traceai_openai/test/currentSdkShapes.mts | 17 ++ .../traceai_openai/test/reviewRegressions.cjs | 3 +- .../traceai_openai/test/reviewRoundThree.cjs | 246 ++++++++++++++++++ 6 files changed, 323 insertions(+), 3 deletions(-) create mode 100644 typescript/packages/traceai_openai/test/reviewRoundThree.cjs diff --git a/.github/workflows/openai-sdk-contracts.yml b/.github/workflows/openai-sdk-contracts.yml index f67b8b487..810c85bfb 100644 --- a/.github/workflows/openai-sdk-contracts.yml +++ b/.github/workflows/openai-sdk-contracts.yml @@ -91,6 +91,7 @@ jobs: packages/traceai_openai/test/privacyContext.cjs \ packages/traceai_openai/test/reviewRegressions.cjs \ packages/traceai_openai/test/reviewRoundTwo.cjs \ + packages/traceai_openai/test/reviewRoundThree.cjs \ packages/traceai_openai/test/choiceIndexes.cjs - name: Typecheck current SDK request contracts if: matrix.sdk == '7.47.0' diff --git a/typescript/packages/traceai_openai/src/instrumentation.ts b/typescript/packages/traceai_openai/src/instrumentation.ts index f88fd29be..850080ce0 100644 --- a/typescript/packages/traceai_openai/src/instrumentation.ts +++ b/typescript/packages/traceai_openai/src/instrumentation.ts @@ -493,7 +493,11 @@ export class OpenAIInstrumentation extends InstrumentationBase{ }; // Telemetry-only copy: MCP credentials must never reach any exported // duplicate, but the original SDK request must remain untouched. - const telemetryBody = { ...body, ...(body.tools && { tools: sanitizeToolDefinitions(body.tools) }) }; + const telemetryBody = { + ...body, + ...(body.tools && { tools: sanitizeToolDefinitions(body.tools) }), + ...(body.input && { input: sanitizeResponsesInput(body.input) }), + }; const { input: _messages, instructions: _instructions, prompt, ...parameters } = telemetryBody; const invocationParameters = { ...parameters, @@ -786,6 +790,19 @@ function getLLMInputMessagesAttributes( }; } +/** URL userinfo, path, query and fragment may carry credentials. Export only the origin. */ +function sanitizeMcpServerURL(serverURL: string): string { + try { + const url = new URL(serverURL); + if (url.protocol !== "https:" && url.protocol !== "http:") return "[REDACTED]"; + // Paths can themselves embed API tokens. Only the server origin is safe + // as telemetry metadata; the SDK still receives the original full URL. + return url.origin; + } catch { + return "[REDACTED]"; + } +} + /** Remove MCP credentials from telemetry without mutating transport objects. */ function sanitizeToolDefinitions(tools: { type: string }[]): { type: string }[] { return tools.map((tool) => { @@ -794,7 +811,28 @@ function sanitizeToolDefinitions(tools: { type: string }[]): { type: string }[] // custom header names can carry credentials too. const { authorization: _authorization, headers: _headers, ...definition } = tool as { type: string; authorization?: string; headers?: Record }; - return definition; + return { + ...definition, + ...("server_url" in definition && typeof definition.server_url === "string" && { + server_url: sanitizeMcpServerURL(definition.server_url), + }), + }; + }); +} + +/** SDK 4 predates tool search; sanitize its loaded definitions when present. */ +function sanitizeResponsesInput( + input: ResponseCreateParamsBase["input"], +): ResponseCreateParamsBase["input"] { + if (!Array.isArray(input)) return input; + return input.map((item) => { + if (item && typeof item === "object") { + const loaded = item as { type: string; tools?: { type: string }[] }; + if (loaded.type === "tool_search_output" && Array.isArray(loaded.tools)) { + return { ...item, tools: sanitizeToolDefinitions(loaded.tools) } as unknown as typeof item; + } + } + return item; }); } diff --git a/typescript/packages/traceai_openai/src/responseAttributes.ts b/typescript/packages/traceai_openai/src/responseAttributes.ts index d9bb72bcb..5f528a38e 100644 --- a/typescript/packages/traceai_openai/src/responseAttributes.ts +++ b/typescript/packages/traceai_openai/src/responseAttributes.ts @@ -15,6 +15,23 @@ import { safelyJSONStringify } from "@traceai/fi-core"; function serializeResponseItem( item: Exclude, ): Record { + // Branch before the floor SDK's discriminated union so its existing cases + // retain strict type narrowing. + if ((item.type as string) === "custom_tool_call") { + const call = item as unknown as { call_id: string; name: string; input: string }; + return { + role: "assistant", + tool_calls: [{ + id: call.call_id, + type: "custom", + custom: { name: call.name, input: call.input }, + }], + }; + } + if ((item.type as string) === "custom_tool_call_output") { + const result = item as unknown as { call_id: string; output: unknown }; + return { role: "tool", tool_call_id: result.call_id, content: result.output }; + } switch (item.type) { case "function_call": return { diff --git a/typescript/packages/traceai_openai/test/currentSdkShapes.mts b/typescript/packages/traceai_openai/test/currentSdkShapes.mts index 4e7094aab..ba38e49e9 100644 --- a/typescript/packages/traceai_openai/test/currentSdkShapes.mts +++ b/typescript/packages/traceai_openai/test/currentSdkShapes.mts @@ -68,3 +68,20 @@ const screenshotFile = { }], } satisfies OpenAI.Responses.ResponseCreateParamsNonStreaming; void [predicted, predictedParts, suffix, screenshot, screenshotFile]; + +const discoveredMcp = { + model: 'synthetic', + input: [{ type: 'tool_search_output', call_id: 'call-search', execution: 'client', tools: [{ + type: 'mcp', server_label: 'synthetic', server_url: 'https://synthetic:private@example.invalid/mcp?api_key=private', + authorization: 'synthetic', headers: { 'X-Api-Key': 'synthetic' }, + }] }], +} satisfies OpenAI.Responses.ResponseCreateParamsNonStreaming; +const responseCustomCall = { + type: 'custom_tool_call', id: 'ctc-synthetic', call_id: 'call-synthetic', + name: 'code_exec', input: 'print(1)', status: 'completed', +} satisfies OpenAI.Responses.ResponseCustomToolCallItem; +const responseCustomHistory = { + model: 'synthetic', input: [responseCustomCall, + { type: 'custom_tool_call_output', call_id: responseCustomCall.call_id, output: '1' }], +} satisfies OpenAI.Responses.ResponseCreateParamsNonStreaming; +void [discoveredMcp, responseCustomCall, client.responses.create(responseCustomHistory)]; diff --git a/typescript/packages/traceai_openai/test/reviewRegressions.cjs b/typescript/packages/traceai_openai/test/reviewRegressions.cjs index 1fc90aa54..83aa9d8e7 100644 --- a/typescript/packages/traceai_openai/test/reviewRegressions.cjs +++ b/typescript/packages/traceai_openai/test/reviewRegressions.cjs @@ -396,11 +396,12 @@ if (OpenAI.Responses) { assert.deepEqual(actual.tools, [tool]); assertNoMarker(marker); const attrs = span().attributes; + // Export only the origin: even a path can contain bearer credentials. const expected = [ { type: "mcp", server_label: "synthetic", - server_url: "https://example.invalid/mcp", + server_url: "https://example.invalid", }, ]; assert.deepEqual(JSON.parse(attrs["gen_ai.tool.definitions"]), expected); diff --git a/typescript/packages/traceai_openai/test/reviewRoundThree.cjs b/typescript/packages/traceai_openai/test/reviewRoundThree.cjs new file mode 100644 index 000000000..917cdc87d --- /dev/null +++ b/typescript/packages/traceai_openai/test/reviewRoundThree.cjs @@ -0,0 +1,246 @@ +const { test, after, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { createRequire } = require('node:module'); +const path = require('node:path'); +const req = createRequire(path.join(process.env.OPENAI_CONSUMER_DIR, 'package.json')); +const OpenAI = req('openai'); +const { OpenAIInstrumentation } = req('@traceai/openai'); +const { BasicTracerProvider, SimpleSpanProcessor, InMemorySpanExporter } = req( + '@opentelemetry/sdk-trace-base', +); +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); +let inst; +function instrument(traceConfig = {}) { + inst = new OpenAIInstrumentation({ instrumentationConfig: { enabled: false }, traceConfig }); + inst.setTracerProvider(provider); + inst.manuallyInstrument(OpenAI); +} +afterEach(async () => { + inst?.disable(); + inst = undefined; + await provider.forceFlush(); + exporter.reset(); +}); +after(() => provider.shutdown()); +const response = { + id: 'resp-synthetic', + object: 'response', + created_at: 1, + status: 'completed', + model: 'synthetic', + output: [], + usage: { input_tokens: 2, output_tokens: 3, total_tokens: 5 }, +}; +function fixture(payload, stream = false) { + let wire; + const client = new OpenAI({ + apiKey: 'synthetic-not-a-key', + maxRetries: 0, + fetch: async (_url, options) => { + wire = JSON.parse(options.body); + return new Response( + stream + ? payload.map((x) => 'data: ' + JSON.stringify(x) + '\n\n').join('') + 'data: [DONE]\n\n' + : JSON.stringify(payload), + { headers: { 'content-type': stream ? 'text/event-stream' : 'application/json' } }, + ); + }, + }); + return { client, wire: () => wire }; +} +function attrs() { + const spans = exporter.getFinishedSpans(); + assert.equal(spans.length, 1); + return spans[0].attributes; +} +function noMarker(marker) { + const a = attrs(); + assert.deepEqual( + Object.keys(a).filter((k) => JSON.stringify(a[k]).includes(marker)), + [], + 'credential-bearing attributes', + ); +} +if (OpenAI.Responses) { + for (const hidden of [false, true]) + test(`Responses nested MCP authorization: capture ${hidden ? 'disabled' : 'enabled'}`, async () => { + const marker = 'SYNTHETIC-NESTED-AUTH-CREDENTIAL'; + const body = { + model: 'synthetic', + input: [ + { + type: 'tool_search_output', + call_id: 'call_search_synthetic', + execution: 'client', + tools: [ + { + type: 'mcp', + server_label: 'synthetic', + server_url: 'https://example.invalid/mcp', + authorization: marker, + headers: { 'X-Api-Key': marker }, + }, + ], + }, + ], + }; + const before = structuredClone(body); + const { client, wire } = fixture(response); + const baseline = await client.responses.create(body); + instrument({ hideInputs: hidden, hideOutputs: hidden }); + assert.deepEqual(await client.responses.create(body), baseline); + assert.deepEqual(body, before); + assert.deepEqual(wire(), before); + noMarker(marker); + }); + for (const location of ['userinfo', 'query']) + for (const hidden of [false, true]) + test(`Responses MCP URL ${location} credentials: capture ${hidden ? 'disabled' : 'enabled'}`, async () => { + const marker = 'SYNTHETIC-URL-CREDENTIAL'; + const server_url = + location === 'userinfo' + ? 'https://synthetic:' + marker + '@example.invalid/mcp' + : 'https://example.invalid/mcp?api_key=' + marker; + const body = { + model: 'synthetic', + input: 'hello', + tools: [{ type: 'mcp', server_label: 'synthetic', server_url }], + }; + const before = structuredClone(body); + const { client, wire } = fixture(response); + const baseline = await client.responses.create(body); + instrument({ hideInputs: hidden, hideOutputs: hidden }); + assert.deepEqual(await client.responses.create(body), baseline); + assert.deepEqual(body, before); + assert.deepEqual(wire(), before); + noMarker(marker); + }); + const custom = { + type: 'custom_tool_call', + id: 'ctc_synthetic', + call_id: 'call_custom_synthetic', + name: 'code_exec', + input: 'print(1)', + status: 'completed', + }; + for (const stream of [false, true]) + test(`Responses ${stream ? 'stream' : 'nonstream'} custom tool output is not an empty message`, async () => { + const payload = { ...response, output: [custom] }; + const { client } = fixture( + stream ? [{ type: 'response.completed', sequence_number: 0, response: payload }] : payload, + stream, + ); + const body = { + model: 'synthetic', + input: 'execute a calculation', + tools: [{ type: 'custom', name: 'code_exec' }], + ...(stream ? { stream: true } : {}), + }; + async function invoke() { + const v = await client.responses.create(body); + if (!stream) return v; + const chunks = []; + for await (const x of v) chunks.push(x); + return chunks; + } + const baseline = await invoke(); + instrument(); + assert.deepEqual(await invoke(), baseline); + const messages = JSON.parse(attrs()['gen_ai.output.messages']); + assert.notDeepEqual(messages, [{}], 'custom tool output is erased'); + assert.ok( + JSON.stringify(messages).includes('code_exec'), + 'structured output must identify custom tool', + ); + assert.ok( + JSON.stringify(messages).includes('print(1)'), + 'structured output must retain custom input', + ); + assert.ok( + JSON.stringify(messages).includes('call_custom_synthetic'), + 'structured output must retain tool call identity', + ); + }); + test('Responses custom tool history retains call/output relationship', async () => { + const body = { + model: 'synthetic', + input: [custom, { type: 'custom_tool_call_output', call_id: custom.call_id, output: '1' }], + }; + const before = structuredClone(body); + const { client, wire } = fixture(response); + const baseline = await client.responses.create(body); + instrument(); + assert.deepEqual(await client.responses.create(body), baseline); + assert.deepEqual(wire(), before); + assert.deepEqual(body, before); + const messages = JSON.parse(attrs()['gen_ai.input.messages']); + assert.notDeepEqual(messages, [{}, {}], 'custom tool history is erased'); + assert.ok( + JSON.stringify(messages).includes(custom.call_id), + 'structured history must preserve call identity', + ); + assert.ok( + JSON.stringify(messages).includes('print(1)'), + 'structured history must preserve custom input', + ); + assert.ok( + JSON.stringify(messages).includes('"1"'), + 'structured history must preserve tool output', + ); + }); +} +if (OpenAI.Responses) { + for (const [setting, direction] of [ + ['hideInputText', 'input'], + ['hideOutputText', 'output'], + ]) { + test(`Responses custom tool ${direction} payload respects ${setting}`, async () => { + const marker = 'SYNTHETIC-CUSTOM-TOOL-PRIVATE'; + const call = { + type: 'custom_tool_call', + id: 'ctc_private', + call_id: 'call_private', + name: 'code_exec', + input: marker, + status: 'completed', + }; + const body = { + model: 'synthetic', + input: + direction === 'input' + ? [call, { type: 'custom_tool_call_output', call_id: call.call_id, output: marker }] + : 'hello', + }; + const { client } = fixture({ ...response, output: direction === 'output' ? [call] : [] }); + instrument({ [setting]: true }); + await client.responses.create(body); + const messages = attrs()[`gen_ai.${direction}.messages`]; + assert.ok(messages, 'semantic messages should remain available'); + assert.ok( + !messages.includes(marker), + 'private payload must be hidden in structured messages', + ); + assert.ok(messages.includes('assistant'), 'message role must remain available'); + }); + } + test('Responses MCP URL malformed and fragment credentials fail closed', async () => { + const marker = 'SYNTHETIC-URL-SECRET'; + const { client } = fixture(response); + instrument(); + await client.responses.create({ + model: 'synthetic', + input: 'hello', + tools: [ + { type: 'mcp', server_label: 'frag', server_url: 'https://example.invalid/mcp#' + marker }, + { type: 'mcp', server_label: 'bad', server_url: 'not-a-url/' + marker }, + { + type: 'mcp', + server_label: 'path', + server_url: 'https://example.invalid/' + marker + '/mcp', + }, + ], + }); + noMarker(marker); + }); +}