Skip to content

Commit c0395f9

Browse files
committed
address comments
1 parent 1a83cdf commit c0395f9

File tree

3 files changed

+96
-8
lines changed

3 files changed

+96
-8
lines changed

aws-distro-opentelemetry-node-autoinstrumentation/src/llo-handler.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import { diag, Attributes, HrTime, ROOT_CONTEXT, createContextKey } from '@opentelemetry/api';
4+
import { Attributes, HrTime, ROOT_CONTEXT, createContextKey } from '@opentelemetry/api';
55
import { LoggerProvider } from '@opentelemetry/sdk-logs';
66
import { EventLoggerProvider } from '@opentelemetry/sdk-events';
77
import { Event } from '@opentelemetry/api-events';
@@ -11,6 +11,7 @@ import { AnyValue } from '@opentelemetry/api-logs';
1111
const ROLE_SYSTEM = 'system';
1212
const ROLE_USER = 'user';
1313
const ROLE_ASSISTANT = 'assistant';
14+
const SESSION_ID = 'session.id';
1415

1516
// Types of LLO attribute patterns
1617
export enum PatternType {
@@ -381,14 +382,14 @@ export class LLOHandler {
381382
const eventLogger = this.eventLoggerProvider.getEventLogger(span.instrumentationLibrary.name);
382383

383384
// Hack - Workaround to add a custom-made Context to an Event so that the emitted event log
384-
// has the correct associated traceId and spanId. This is needed because a ReadableSpan only
385+
// has the correct associated traceId, spanId, flag. This is needed because a ReadableSpan only
385386
// provides its SpanContext, but does not provide access to its associated Context. We can only
386387
// provide a Context to an Event, but not a SpanContext. Here we attach a custom Context that
387388
// is associated to the ReadableSpan to mimic the ReadableSpan's actual Context.
388389
//
389390
// When a log record instance is created from this event, it will use the "custom Context" to
390391
// extract the ReadableSpan from the "custom Context", then extract the SpanContext from the
391-
// RedableSpan. This way, the emitted log event has the correct SpanContext (traceId, spanId).
392+
// RedableSpan. This way, the emitted log event has the correct SpanContext (traceId, spanId, flag).
392393
// - https://github.com/open-telemetry/opentelemetry-js/blob/experimental/v0.57.1/experimental/packages/sdk-logs/src/LogRecord.ts#L101
393394
// - https://github.com/open-telemetry/opentelemetry-js/blob/experimental/v0.57.1/api/src/trace/context-utils.ts#L78-L85
394395
//
@@ -404,8 +405,13 @@ export class LLOHandler {
404405
context: customContext,
405406
};
406407

408+
if (span.attributes[SESSION_ID]) {
409+
event.attributes = {
410+
[SESSION_ID]: span.attributes[SESSION_ID],
411+
};
412+
}
413+
407414
eventLogger.emit(event);
408-
diag.debug('Emitted Gen AI Event with input/output message format');
409415
}
410416

411417
/**

aws-distro-opentelemetry-node-autoinstrumentation/src/otlp-aws-span-exporter.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { ExportResult } from '@opentelemetry/core';
88
import { LLOHandler } from './llo-handler';
99
import { LoggerProvider as APILoggerProvider, logs } from '@opentelemetry/api-logs';
1010
import { LoggerProvider } from '@opentelemetry/sdk-logs';
11-
import { LogsAPI } from '@opentelemetry/api-logs/build/src/api/logs';
1211
import { getNodeVersion, isAgentObservabilityEnabled } from './utils';
1312
import { diag } from '@opentelemetry/api';
1413

@@ -36,7 +35,6 @@ export class OTLPAwsSpanExporter extends OTLPProtoTraceExporter {
3635

3736
private lloHandler: LLOHandler | undefined;
3837
private loggerProvider: APILoggerProvider | undefined;
39-
private logsApi: LogsAPI = logs;
4038

4139
constructor(endpoint: string, config?: OTLPExporterNodeConfigBase, loggerProvider?: APILoggerProvider) {
4240
super(OTLPAwsSpanExporter.changeUrlConfig(endpoint, config));
@@ -48,14 +46,15 @@ export class OTLPAwsSpanExporter extends OTLPProtoTraceExporter {
4846
this.loggerProvider = loggerProvider;
4947
}
5048

51-
// Lazily initialize LLO handler when needed to avoid initialization order issues"""
49+
// Lazily initialize LLO handler when needed to avoid initialization order issues
5250
private ensureLloHandler(): boolean {
5351
if (!this.lloHandler && isAgentObservabilityEnabled()) {
5452
// If loggerProvider wasn't provided, try to get the current one
5553
if (!this.loggerProvider) {
5654
try {
57-
this.loggerProvider = this.logsApi.getLoggerProvider();
55+
this.loggerProvider = logs.getLoggerProvider();
5856
} catch (e: unknown) {
57+
diag.debug('Failed to get logger provider', e);
5958
return false;
6059
}
6160
}

aws-distro-opentelemetry-node-autoinstrumentation/test/llo-handler.events.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,4 +552,87 @@ describe('TestLLOHandlerEvents', () => {
552552
expect(outputMessages[0].content).toBe('Quantum computing is...');
553553
expect(outputMessages[0].role).toBe('assistant');
554554
});
555+
556+
/**
557+
* Verify session.id attribute from span is copied to event attributes when present.
558+
*/
559+
it('emitLloAttributes should copy session.id to event attributes when present', () => {
560+
const attributes = {
561+
'session.id': 'test-session-123',
562+
'gen_ai.prompt': 'Hello, AI',
563+
'gen_ai.completion': 'Hello! How can I help you?',
564+
};
565+
566+
const span = testBase.createMockSpan(attributes);
567+
testBase.updateMockSpanKey(span, 'endTime', [1234567899, 0]);
568+
testBase.updateMockSpanKey(span, 'instrumentationLibrary', { name: 'test.scope', version: '1.0.0' });
569+
570+
testBase.lloHandler['emitLloAttributes'](span, attributes);
571+
572+
sinon.assert.calledOnce(testBase.eventLoggerMock.emit as any);
573+
const emittedEvent = (testBase.eventLoggerMock.emit as any).getCall(0).args[0] as Event;
574+
575+
// Verify session.id was copied to event attributes
576+
expect(emittedEvent.attributes).toBeDefined();
577+
expect(emittedEvent.attributes?.['session.id']).toBe('test-session-123');
578+
579+
// Verify event body still contains LLO data
580+
const eventBody = emittedEvent.data as any;
581+
expect(eventBody.input).toBeDefined();
582+
expect(eventBody.output).toBeDefined();
583+
});
584+
585+
/**
586+
* Verify event attributes do not contain session.id when not present in span attributes.
587+
*/
588+
it('emitLloAttributes should not include session.id in event attributes when not present', () => {
589+
const attributes = {
590+
'gen_ai.prompt': 'Hello, AI',
591+
'gen_ai.completion': 'Hello! How can I help you?',
592+
};
593+
594+
const span = testBase.createMockSpan(attributes);
595+
testBase.updateMockSpanKey(span, 'endTime', [1234567899, 0]);
596+
testBase.updateMockSpanKey(span, 'instrumentationLibrary', { name: 'test.scope', version: '1.0.0' });
597+
598+
testBase.lloHandler['emitLloAttributes'](span, attributes);
599+
600+
sinon.assert.calledOnce(testBase.eventLoggerMock.emit as any);
601+
const emittedEvent = (testBase.eventLoggerMock.emit as any).getCall(0).args[0] as Event;
602+
603+
// Verify session.id is not in event attributes (because the event doesn't have attributes)
604+
expect(emittedEvent.attributes).toBeUndefined();
605+
expect(emittedEvent).not.toHaveProperty('attributes');
606+
});
607+
608+
/**
609+
* Verify only session.id is copied from span attributes when mixed with other attributes.
610+
*/
611+
it('emitLloAttributes should only copy session.id when mixed with other attributes', () => {
612+
const attributes = {
613+
'session.id': 'session-456',
614+
'user.id': 'user-789',
615+
'gen_ai.prompt': "What's the weather?",
616+
'gen_ai.completion': "I can't check the weather.",
617+
'other.attribute': 'some-value',
618+
};
619+
620+
const span = testBase.createMockSpan(attributes);
621+
testBase.updateMockSpanKey(span, 'endTime', [1234567899, 0]);
622+
testBase.updateMockSpanKey(span, 'instrumentationLibrary', { name: 'test.scope', version: '1.0.0' });
623+
624+
testBase.lloHandler['emitLloAttributes'](span, attributes);
625+
626+
sinon.assert.calledOnce(testBase.eventLoggerMock.emit as any);
627+
const emittedEvent = (testBase.eventLoggerMock.emit as any).getCall(0).args[0] as Event;
628+
629+
// Verify only session.id was copied to event attributes (plus event.name from Event class)
630+
expect(emittedEvent.attributes).toBeDefined();
631+
expect(emittedEvent.attributes?.['session.id']).toBe('session-456');
632+
// Verify other span attributes were not copied
633+
expect(emittedEvent.attributes).not.toHaveProperty('user.id');
634+
expect(emittedEvent.attributes).not.toHaveProperty('other.attribute');
635+
expect(emittedEvent.attributes).not.toHaveProperty('gen_ai.prompt');
636+
expect(emittedEvent.attributes).not.toHaveProperty('gen_ai.completion');
637+
});
555638
});

0 commit comments

Comments
 (0)