Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

### Bugs Fixed
- A365: prevent `recordAttributes()` from overwriting scope-owned attributes such as builder-populated tags and `gen_ai.operation.name`, while still allowing late writes for known keys that were never set and repeated last-write-wins custom attributes. [#243](https://github.com/microsoft/opentelemetry-distro-javascript/pull/243)

### Other Changes
- Consolidate Dependabot updates for Vitest 4.1.11, Hono 4.13.7, qs 6.16.0, fast-uri 3.1.7, actions/deploy-pages 5.0.1, and actions/checkout 7.0.1.

Expand Down
39 changes: 27 additions & 12 deletions src/a365/scopes/OpenTelemetryScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export abstract class OpenTelemetryScope {
private customEndTime?: TimeInput;
private errorType?: string;
private hasEnded = false;
private readonly ownedAttributeKeys = new Set<string>();
private readonly logger = Logger.getInstance();

/**
Expand Down Expand Up @@ -94,6 +95,7 @@ export abstract class OpenTelemetryScope {
},
currentContext,
);
this.ownedAttributeKeys.add(OpenTelemetryConstants.GEN_AI_OPERATION_NAME_KEY);

this.wallClockStartMs = Date.now();
if (startTime !== undefined) {
Expand Down Expand Up @@ -164,25 +166,20 @@ export abstract class OpenTelemetryScope {
this.span.recordException(error);
}

/** Records multiple attribute key/value pairs. */
/**
* Records multiple attribute key/value pairs without overwriting values that
* the scope already populated through its typed setters or span builder.
*/
public recordAttributes(
attributes:
Iterable<[string, AttributeValue]> | Record<string, AttributeValue> | null | undefined,
): void {
if (!attributes) return;

if (Symbol.iterator in Object(attributes) && typeof attributes !== "string") {
for (const [key, value] of attributes as Iterable<[string, AttributeValue]>) {
if (key && typeof key === "string" && key.trim()) {
this.span.setAttribute(key, value);
}
}
this.recordUnownedAttributes(attributes as Iterable<[string, AttributeValue]>);
} else if (typeof attributes === "object") {
for (const key of Object.keys(attributes as Record<string, AttributeValue>)) {
if (key && key.trim()) {
this.span.setAttribute(key, (attributes as Record<string, AttributeValue>)[key]);
}
}
this.recordUnownedAttributes(Object.entries(attributes as Record<string, AttributeValue>));
}
}

Expand All @@ -198,7 +195,10 @@ export abstract class OpenTelemetryScope {
this.setTagMaybe(OpenTelemetryConstants.GEN_AI_OUTPUT_MESSAGES_KEY, serializeMessages(wrapper));
}

/** Sets a tag on the span if the value is not null or undefined. */
/**
* Sets a tag on the span if the value is not null or undefined, and marks the
* key as owned by the scope so later generic attribute writes do not replace it.
*/
protected setTagMaybe<T extends string | number | boolean | string[] | number[]>(
name: string,
value: T | null | undefined,
Expand All @@ -207,9 +207,24 @@ export abstract class OpenTelemetryScope {
this.span.setAttributes({
[name]: value as string | number | boolean | string[] | number[],
});
this.ownedAttributeKeys.add(name);
}
}

private recordUnownedAttributes(attributes: Iterable<[string, AttributeValue]>): void {
for (const [key, value] of attributes) {
if (!OpenTelemetryScope.isNonBlankAttributeKey(key) || this.ownedAttributeKeys.has(key)) {
continue;
}

this.span.setAttribute(key, value);
}
}

private static isNonBlankAttributeKey(key: string): boolean {
return typeof key === "string" && key.trim().length > 0;
}

/**
* Adds an event to the current span.
* @param name The event name
Expand Down
112 changes: 112 additions & 0 deletions test/internal/unit/a365/scopes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,118 @@ describe("Request content and message serialization (span attributes)", () => {
});
});

describe("recordAttributes ownership and precedence", () => {
const testAgentDetails: AgentDetails = {
agentId: "test-agent",
agentName: "Test Agent",
tenantId: "test-tenant-456",
};

beforeEach(() => {
sharedExporter.reset();
});

const getLastSpan = (): ReadableSpan => {
const spans = sharedExporter.getFinishedSpans();
expect(spans.length).toBeGreaterThanOrEqual(1);
return spans[spans.length - 1];
};

it("should preserve builder-populated attributes when recordAttributes sees the same keys", () => {
const scope = InvokeAgentScope.start(
{
conversationId: "conv-owned",
channel: { name: "Teams", description: "https://teams.example" },
},
{},
testAgentDetails,
);

scope.recordAttributes({
[OpenTelemetryConstants.GEN_AI_AGENT_NAME_KEY]: "Override Agent",
[OpenTelemetryConstants.GEN_AI_CONVERSATION_ID_KEY]: "override-conv",
"custom.attribute": "custom value",
});
scope.dispose();

const attributes = getLastSpan().attributes;
expect(attributes[OpenTelemetryConstants.GEN_AI_AGENT_NAME_KEY]).toBe("Test Agent");
expect(attributes[OpenTelemetryConstants.GEN_AI_CONVERSATION_ID_KEY]).toBe("conv-owned");
expect(attributes["custom.attribute"]).toBe("custom value");
});

it("should preserve the span builder operation name when recordAttributes provides another value", () => {
const scope = ExecuteToolScope.start(
{ conversationId: "conv-op-name" },
{ toolName: "search" },
testAgentDetails,
);

scope.recordAttributes({
[OpenTelemetryConstants.GEN_AI_OPERATION_NAME_KEY]:
OpenTelemetryConstants.CHAT_OPERATION_NAME,
});
scope.dispose();

expect(getLastSpan().attributes[OpenTelemetryConstants.GEN_AI_OPERATION_NAME_KEY]).toBe(
OpenTelemetryConstants.EXECUTE_TOOL_OPERATION_NAME,
);
});

it("should accept known keys that were absent when the scope was created", () => {
const scope = InferenceScope.start(
{ conversationId: "conv-late-known", channel: { name: "Teams" } },
{ operationName: InferenceOperationType.CHAT, model: "gpt-4" },
testAgentDetails,
);

scope.recordAttributes({
[OpenTelemetryConstants.CHANNEL_LINK_KEY]: "https://teams.example/deep-link",
});
scope.dispose();

expect(getLastSpan().attributes[OpenTelemetryConstants.CHANNEL_LINK_KEY]).toBe(
"https://teams.example/deep-link",
);
});

it("should keep custom recordAttributes keys last-write-wins across repeated calls", () => {
const scope = ExecuteToolScope.start(
{ conversationId: "conv-custom-repeat" },
{ toolName: "search" },
testAgentDetails,
);

scope.recordAttributes({ "custom.repeat": "first" });
scope.recordAttributes({ "custom.repeat": "second" });
scope.dispose();

expect(getLastSpan().attributes["custom.repeat"]).toBe("second");
});

it("should support iterable attributes while skipping owned and blank keys", () => {
const scope = InvokeAgentScope.start(
{ conversationId: "conv-iterable", channel: { name: "Teams" } },
{},
testAgentDetails,
);

scope.recordAttributes([
[OpenTelemetryConstants.GEN_AI_CONVERSATION_ID_KEY, "override-conv"],
["", "ignored"],
[" ", "also ignored"],
["custom.iterable", 42],
]);
scope.dispose();

const attributes = getLastSpan().attributes;
expect(attributes[OpenTelemetryConstants.GEN_AI_CONVERSATION_ID_KEY]).toBe("conv-iterable");
expect(attributes["custom.iterable"]).toBe(42);
expect(attributes[""]).toBeUndefined();
expect(attributes[" "]).toBeUndefined();
});
});

// Validate attribute key constant values use the new schema namespace.
describe("Attribute key schema values", () => {
it("caller keys use user.* / client.* namespace", () => {
Expand Down