diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 01e76816976..6e003a628d4 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -12,6 +12,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :rocket: Features +* feat(sdk-logs)!: add support for attributes in LoggerOptions [#6573](https://github.com/open-telemetry/opentelemetry-js/pull/6573) @pichlermarc + ### :bug: Bug Fixes * fix(sdk-node): pass all config properties to log record exporters in declarative config [#6708](https://github.com/open-telemetry/opentelemetry-js/pull/6708) @MikeGoldsmith @@ -88,6 +90,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 * feat(sdk-logs)!: add required `forceFlush()` to `LogRecordExporter` interface [#6356](https://github.com/open-telemetry/opentelemetry-js/pull/6356) @pichlermarc * (user-facing): `LogRecordExporter` interface now requires a `forceFlush()` method to be implemented. Custom exporters will need to implement this method to continue working with the Logs SDK. * feat(api-logs, sdk-logs)!: add Logger#enabled() [#6371](https://github.com/open-telemetry/opentelemetry-js/pull/6371) @david-luna +* feat(api-logs)!: rename scopeAttributes to attributes in LoggerOptions [#6573](https://github.com/open-telemetry/opentelemetry-js/pull/6573) @pichlermarc * feat(sampler-composite)!: rename `createComposableTraceIDRatioBasedSampler` to `createComposableProbabilitySampler` [#6541](https://github.com/open-telemetry/opentelemetry-js/pull/6541) @ravitheja4531-cell ### :rocket: Features diff --git a/experimental/packages/api-logs/src/api/logs.ts b/experimental/packages/api-logs/src/api/logs.ts index 67b72d7f9a7..fbd1614fa41 100644 --- a/experimental/packages/api-logs/src/api/logs.ts +++ b/experimental/packages/api-logs/src/api/logs.ts @@ -58,9 +58,17 @@ export class LogsAPI { } /** - * Returns a logger from the global logger provider. + * Returns a Logger, creating one if one with the given name, version, + * schemaUrl, and attributes is not already created. * - * @returns Logger + * Getting a Logger may be expensive, especially when `attributes` are + * provided. Reuse Logger instances where possible instead of calling + * `getLogger()` on hot paths. + * + * @param name The name of the logger or instrumentation library. + * @param version The version of the logger or instrumentation library. + * @param options The options of the logger or instrumentation library. + * @returns {@link Logger} */ public getLogger( name: string, diff --git a/experimental/packages/api-logs/src/types/LoggerOptions.ts b/experimental/packages/api-logs/src/types/LoggerOptions.ts index ab3cd5c708c..4c7dee44f0c 100644 --- a/experimental/packages/api-logs/src/types/LoggerOptions.ts +++ b/experimental/packages/api-logs/src/types/LoggerOptions.ts @@ -12,7 +12,8 @@ export interface LoggerOptions { schemaUrl?: string; /** - * The instrumentation scope attributes to associate with emitted telemetry + * The instrumentation scope attributes to associate with emitted telemetry. + * These attributes also participate in logger identity. */ - scopeAttributes?: LogAttributes; + attributes?: LogAttributes; } diff --git a/experimental/packages/api-logs/src/types/LoggerProvider.ts b/experimental/packages/api-logs/src/types/LoggerProvider.ts index 5ba47c28cc6..5ab1446fd56 100644 --- a/experimental/packages/api-logs/src/types/LoggerProvider.ts +++ b/experimental/packages/api-logs/src/types/LoggerProvider.ts @@ -11,13 +11,17 @@ import type { LoggerOptions } from './LoggerOptions'; */ export interface LoggerProvider { /** - * Returns a Logger, creating one if one with the given name, version, and - * schemaUrl pair is not already created. + * Returns a Logger, creating one if one with the given name, version, + * schemaUrl, and attributes is not already created. + * + * Getting a Logger may be expensive, especially when `attributes` are + * provided. Reuse Logger instances where possible instead of calling + * `getLogger()` on hot paths. * * @param name The name of the logger or instrumentation library. * @param version The version of the logger or instrumentation library. * @param options The options of the logger or instrumentation library. - * @returns Logger A Logger with the given name and version + * @returns {@link Logger} */ getLogger(name: string, version?: string, options?: LoggerOptions): Logger; } diff --git a/experimental/packages/api-logs/test/noop-implementations/noop-logger-provider.test.ts b/experimental/packages/api-logs/test/noop-implementations/noop-logger-provider.test.ts index d0f066ce2dd..47a62fa196a 100644 --- a/experimental/packages/api-logs/test/noop-implementations/noop-logger-provider.test.ts +++ b/experimental/packages/api-logs/test/noop-implementations/noop-logger-provider.test.ts @@ -19,10 +19,10 @@ describe('NoopLoggerProvider', () => { ); }); - describe('LoggerOptions#scopeAttributes (LogAttributes)', () => { - it('should accept scopeAttributes with primitive values', () => { + describe('LoggerOptions#attributes (LogAttributes)', () => { + it('should accept attributes with primitive values', () => { const logger = loggerProvider.getLogger('logger-name', undefined, { - scopeAttributes: { + attributes: { 'service.name': 'api', version: 1, enabled: true, @@ -31,9 +31,9 @@ describe('NoopLoggerProvider', () => { assert.ok(logger); }); - it('should accept scopeAttributes with LogAttribute value types', () => { + it('should accept attributes with LogAttribute value types', () => { const logger = loggerProvider.getLogger('logger-name', undefined, { - scopeAttributes: { + attributes: { scalar: 'value', number: 42, bool: true, diff --git a/experimental/packages/api-logs/test/proxy-implementations/proxy-logger.test.ts b/experimental/packages/api-logs/test/proxy-implementations/proxy-logger.test.ts index d4180c9c215..d6b76625da6 100644 --- a/experimental/packages/api-logs/test/proxy-implementations/proxy-logger.test.ts +++ b/experimental/packages/api-logs/test/proxy-implementations/proxy-logger.test.ts @@ -67,8 +67,8 @@ describe('ProxyLogger', () => { ]); }); - it('should pass LoggerOptions with scopeAttributes (LogAttributes) to delegate', () => { - const scopeAttributes = { + it('should pass LoggerOptions with attributes (LogAttributes) to delegate', () => { + const attributes = { 'service.name': 'api', version: 1, nested: { key: 'value' }, @@ -76,7 +76,7 @@ describe('ProxyLogger', () => { }; const options = { schemaUrl: 'https://opentelemetry.io/schemas/1.7.0', - scopeAttributes, + attributes, }; const logger = provider.getLogger('test', 'v0', options); @@ -88,8 +88,8 @@ describe('ProxyLogger', () => { options, ]); assert.strictEqual( - getLoggerStub.firstCall.args[2]?.scopeAttributes, - scopeAttributes + getLoggerStub.firstCall.args[2]?.attributes, + attributes ); }); }); diff --git a/experimental/packages/otlp-transformer/src/common/internal.ts b/experimental/packages/otlp-transformer/src/common/internal.ts index a2bc5064020..45c006d4a7b 100644 --- a/experimental/packages/otlp-transformer/src/common/internal.ts +++ b/experimental/packages/otlp-transformer/src/common/internal.ts @@ -8,10 +8,10 @@ import type { IKeyValue, Resource, } from './internal-types'; -import type { Attributes } from '@opentelemetry/api'; import type { InstrumentationScope } from '@opentelemetry/core'; import type { Resource as ISdkResource } from '@opentelemetry/resources'; import type { Encoder } from './utils'; +import type { LogAttributes } from '@opentelemetry/api-logs'; export function createResource( resource: ISdkResource, @@ -29,16 +29,27 @@ export function createResource( } export function createInstrumentationScope( - scope: InstrumentationScope + scope: InstrumentationScope & { + attributes?: LogAttributes; + droppedAttributesCount?: number; + }, + encoder: Encoder ): IInstrumentationScope { - return { + const result: IInstrumentationScope = { name: scope.name, version: scope.version, }; + + if (scope.attributes && Object.keys(scope.attributes).length > 0) { + result.attributes = toAttributes(scope.attributes, encoder); + result.droppedAttributesCount = scope.droppedAttributesCount ?? 0; + } + + return result; } export function toAttributes( - attributes: Attributes, + attributes: LogAttributes, encoder: Encoder ): IKeyValue[] { return Object.keys(attributes).map(key => diff --git a/experimental/packages/otlp-transformer/src/common/protobuf/common-serializer.ts b/experimental/packages/otlp-transformer/src/common/protobuf/common-serializer.ts index bb1272618e4..6210d4e35f0 100644 --- a/experimental/packages/otlp-transformer/src/common/protobuf/common-serializer.ts +++ b/experimental/packages/otlp-transformer/src/common/protobuf/common-serializer.ts @@ -199,7 +199,10 @@ export function writeAnyValue(writer: IProtobufWriter, value: AnyValue): void { */ export function writeInstrumentationScope( writer: IProtobufWriter, - scope: InstrumentationScope, + scope: InstrumentationScope & { + attributes?: LogAttributes; + droppedAttributesCount?: number; + }, fieldNumber: number ): void { writer.writeTag(fieldNumber, 2); @@ -216,6 +219,17 @@ export function writeInstrumentationScope( writer.writeString(scope.version); } + if (scope.attributes) { + // attributes (field 3, repeated KeyValue) + writeAttributes(writer, scope.attributes, 3); + } + + if (scope.droppedAttributesCount) { + // dropped_attributes_count (field 4, uint32) + writer.writeTag(4, 0); + writer.writeVarint(scope.droppedAttributesCount); + } + writer.finishLengthDelimited(start, writer.pos - startPos); } diff --git a/experimental/packages/otlp-transformer/src/logs/internal.ts b/experimental/packages/otlp-transformer/src/logs/internal.ts index d4d229247c3..c2be2408717 100644 --- a/experimental/packages/otlp-transformer/src/logs/internal.ts +++ b/experimental/packages/otlp-transformer/src/logs/internal.ts @@ -16,11 +16,9 @@ import { createInstrumentationScope, createResource, toAnyValue, - toKeyValue, + toAttributes, } from '../common/internal'; import type { SeverityNumber } from '@opentelemetry/api-logs'; -import type { IKeyValue } from '../common/internal-types'; -import type { LogAttributes } from '@opentelemetry/api-logs'; export function createExportLogsServiceRequest( logRecords: ReadableLogRecord[], @@ -33,17 +31,17 @@ export function createExportLogsServiceRequest( function createResourceMap( logRecords: ReadableLogRecord[] -): Map> { +): Map< + Resource, + Map +> { const resourceMap: Map< Resource, - Map + Map > = new Map(); for (const record of logRecords) { - const { - resource, - instrumentationScope: { name, version = '', schemaUrl = '' }, - } = record; + const { resource, instrumentationScope } = record; let ismMap = resourceMap.get(resource); if (!ismMap) { @@ -51,11 +49,10 @@ function createResourceMap( resourceMap.set(resource, ismMap); } - const ismKey = `${name}@${version}:${schemaUrl}`; - let records = ismMap.get(ismKey); + let records = ismMap.get(instrumentationScope); if (!records) { records = []; - ismMap.set(ismKey, records); + ismMap.set(instrumentationScope, records); } records.push(record); } @@ -73,7 +70,10 @@ function logRecordsToResourceLogs( resource: processedResource, scopeLogs: Array.from(ismMap, ([, scopeLogs]) => { return { - scope: createInstrumentationScope(scopeLogs[0].instrumentationScope), + scope: createInstrumentationScope( + scopeLogs[0].instrumentationScope, + encoder + ), logRecords: scopeLogs.map(log => toLogRecord(log, encoder)), schemaUrl: scopeLogs[0].instrumentationScope.schemaUrl, }; @@ -91,7 +91,7 @@ function toLogRecord(log: ReadableLogRecord, encoder: Encoder): ILogRecord { severityText: log.severityText, body: toAnyValue(log.body, encoder), eventName: log.eventName, - attributes: toLogAttributes(log.attributes, encoder), + attributes: toAttributes(log.attributes, encoder), droppedAttributesCount: log.droppedAttributesCount, flags: log.spanContext?.traceFlags, traceId: encoder.encodeOptionalSpanContext(log.spanContext?.traceId), @@ -104,12 +104,3 @@ function toSeverityNumber( ): ESeverityNumber | undefined { return severityNumber as number | undefined as ESeverityNumber | undefined; } - -export function toLogAttributes( - attributes: LogAttributes, - encoder: Encoder -): IKeyValue[] { - return Object.keys(attributes).map(key => - toKeyValue(key, attributes[key], encoder) - ); -} diff --git a/experimental/packages/otlp-transformer/src/metrics/internal.ts b/experimental/packages/otlp-transformer/src/metrics/internal.ts index 929de975ec6..7fc8b098896 100644 --- a/experimental/packages/otlp-transformer/src/metrics/internal.ts +++ b/experimental/packages/otlp-transformer/src/metrics/internal.ts @@ -50,7 +50,7 @@ export function toScopeMetrics( ): IScopeMetrics[] { return Array.from( scopeMetrics.map(metrics => ({ - scope: createInstrumentationScope(metrics.scope), + scope: createInstrumentationScope(metrics.scope, encoder), metrics: metrics.metrics.map(metricData => toMetric(metricData, encoder)), schemaUrl: metrics.scope.schemaUrl, })) diff --git a/experimental/packages/otlp-transformer/src/trace/internal.ts b/experimental/packages/otlp-transformer/src/trace/internal.ts index d5052ded2a1..7297a928363 100644 --- a/experimental/packages/otlp-transformer/src/trace/internal.ts +++ b/experimental/packages/otlp-transformer/src/trace/internal.ts @@ -152,7 +152,10 @@ function spanRecordsToResourceSpans( ); scopeResourceSpans.push({ - scope: createInstrumentationScope(scopeSpans[0].instrumentationScope), + scope: createInstrumentationScope( + scopeSpans[0].instrumentationScope, + encoder + ), spans: spans, schemaUrl: scopeSpans[0].instrumentationScope.schemaUrl, }); diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index 88f0a1f95b5..4bb74bb7c30 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -25,6 +25,11 @@ import { ProtobufWriter, } from '../src/common/protobuf/protobuf-writer'; +type InstrumentationScopeWithAttributes = InstrumentationScope & { + attributes?: Record; + droppedAttributesCount?: number; +}; + function createExpectedLogJson(encoder: Encoder): IExportLogsServiceRequest { const timeUnixNano = encoder.encodeHrTime([1680253513, 123241635]); const observedTimeUnixNano = encoder.encodeHrTime([1683526948, 965142784]); @@ -54,6 +59,21 @@ function createExpectedLogJson(encoder: Encoder): IExportLogsServiceRequest { scope: { name: 'scope_name_1', version: '0.1.0', + attributes: [ + { + key: 'scope-attribute', + value: { stringValue: 'scope attribute value' }, + }, + { + key: 'scope-array', + value: { + arrayValue: { + values: [{ stringValue: 'prod' }, { boolValue: true }], + }, + }, + }, + ], + droppedAttributesCount: 1, }, logRecords: [ { @@ -116,7 +136,25 @@ function createExpectedLogProtobuf(): IExportLogsServiceRequest { }, scopeLogs: [ { - scope: { name: 'scope_name_1', version: '0.1.0' }, + scope: { + name: 'scope_name_1', + version: '0.1.0', + attributes: [ + { + key: 'scope-attribute', + value: { stringValue: 'scope attribute value' }, + }, + { + key: 'scope-array', + value: { + arrayValue: { + values: [{ stringValue: 'prod' }, { boolValue: true }], + }, + }, + }, + ], + droppedAttributesCount: 1, + }, logRecords: [ { timeUnixNano: 1680253513123241700, @@ -179,8 +217,8 @@ const DEFAULT_LOG_FRAGMENT: Omit< describe('Logs', () => { let resource_1: Resource; let resource_2: Resource; - let scope_1: InstrumentationScope; - let scope_2: InstrumentationScope; + let scope_1: InstrumentationScopeWithAttributes; + let scope_2: InstrumentationScopeWithAttributes; /* The following log_X_Y_Z should follow the pattern @@ -200,7 +238,7 @@ describe('Logs', () => { function createReadableLogRecord( resource: Resource, - scope: InstrumentationScope, + scope: InstrumentationScopeWithAttributes, logFragment: Omit ): ReadableLogRecord { return { @@ -221,6 +259,11 @@ describe('Logs', () => { name: 'scope_name_1', version: '0.1.0', schemaUrl: 'http://url.to.schema', + attributes: { + 'scope-attribute': 'scope attribute value', + 'scope-array': ['prod', true], + }, + droppedAttributesCount: 1, }; scope_2 = { name: 'scope_name_2', @@ -297,6 +340,51 @@ describe('Logs', () => { assert.strictEqual(exportRequest.resourceLogs?.[0].scopeLogs.length, 2); }); + it('separates logs with different attributes into different scope groups', () => { + const logWithDifferentScopeAttributes = createReadableLogRecord( + resource_1, + { + ...scope_1, + attributes: { + 'scope-attribute': 'another scope attribute value', + }, + }, + DEFAULT_LOG_FRAGMENT + ); + + const exportRequest = createExportLogsServiceRequest( + [log_1_1_1, logWithDifferentScopeAttributes], + PROTOBUF_ENCODER + ); + + assert.ok(exportRequest); + assert.strictEqual(exportRequest.resourceLogs?.length, 1); + assert.strictEqual(exportRequest.resourceLogs?.[0].scopeLogs.length, 2); + }); + + it('keeps different scope objects separate even if their data matches', () => { + const logWithEquivalentScope = createReadableLogRecord( + resource_1, + { + ...scope_1, + attributes: { + 'scope-attribute': 'scope attribute value', + 'scope-array': ['prod', true], + }, + }, + DEFAULT_LOG_FRAGMENT + ); + + const exportRequest = createExportLogsServiceRequest( + [log_1_1_1, logWithEquivalentScope], + PROTOBUF_ENCODER + ); + + assert.ok(exportRequest); + assert.strictEqual(exportRequest.resourceLogs?.length, 1); + assert.strictEqual(exportRequest.resourceLogs?.[0].scopeLogs.length, 2); + }); + it('aggregates multiple logs with different resources', () => { const exportRequest = createExportLogsServiceRequest( [log_1_1_1, log_2_1_1], @@ -346,6 +434,37 @@ describe('Logs', () => { assert.strictEqual(bytesAttr.value.bytesValue, 'AQIDBAU='); assert.strictEqual(bytesAttr.value.stringValue, undefined); }); + + it('exports scope attributes on scope logs', () => { + const exportRequest = createExportLogsServiceRequest( + [log_1_1_1], + JSON_ENCODER + ); + + assert.ok(exportRequest); + assert.deepStrictEqual( + exportRequest.resourceLogs?.[0].scopeLogs[0].scope?.attributes, + [ + { + key: 'scope-attribute', + value: { stringValue: 'scope attribute value' }, + }, + { + key: 'scope-array', + value: { + arrayValue: { + values: [{ stringValue: 'prod' }, { boolValue: true }], + }, + }, + }, + ] + ); + assert.strictEqual( + exportRequest.resourceLogs?.[0].scopeLogs[0].scope + ?.droppedAttributesCount, + 1 + ); + }); }); describe('ProtobufLogsSerializer', function () { diff --git a/experimental/packages/sdk-logs/README.md b/experimental/packages/sdk-logs/README.md index 2b4f4397292..604f1f1fc2b 100644 --- a/experimental/packages/sdk-logs/README.md +++ b/experimental/packages/sdk-logs/README.md @@ -39,6 +39,9 @@ const loggerProvider = new LoggerProvider({ // To create a log record, you first need to get a Logger instance const logger = loggerProvider.getLogger('default'); +// Reuse loggers where possible. getLogger() may be expensive, especially when +// scopeAttributes are provided, so avoid calling it on hot paths. + // You can also use global singleton logsAPI.logs.setGlobalLoggerProvider(loggerProvider); const logger = logsAPI.logs.getLogger('default'); diff --git a/experimental/packages/sdk-logs/src/LogRecordImpl.ts b/experimental/packages/sdk-logs/src/LogRecordImpl.ts index c71f2646023..1efae84425a 100644 --- a/experimental/packages/sdk-logs/src/LogRecordImpl.ts +++ b/experimental/packages/sdk-logs/src/LogRecordImpl.ts @@ -21,15 +21,18 @@ import { } from '@opentelemetry/semantic-conventions'; import type { ReadableLogRecord } from './export/ReadableLogRecord'; import type { LogRecordLimits } from './types'; -import { isLogAttributeValue } from './utils/validation'; import type { LoggerProviderSharedState } from './internal/LoggerProviderSharedState'; +import { addAttribute, AddAttributeDecision } from './utils/validation'; export class LogRecordImpl implements ReadableLogRecord { readonly hrTime: api.HrTime; readonly hrTimeObserved: api.HrTime; readonly spanContext?: api.SpanContext; readonly resource: Resource; - readonly instrumentationScope: InstrumentationScope; + readonly instrumentationScope: InstrumentationScope & { + attributes?: LogAttributes; + droppedAttributesCount?: number; + }; readonly attributes: LogAttributes = {}; private _severityText?: string; private _severityNumber?: SeverityNumber; @@ -129,33 +132,25 @@ export class LogRecordImpl implements ReadableLogRecord { if (this._isLogRecordReadonly()) { return this; } - if (key.length === 0) { - api.diag.warn(`Invalid attribute key: ${key}`); - return this; - } - if (!isLogAttributeValue(value)) { - api.diag.warn(`Invalid attribute value set for key: ${key}`); - return this; - } - const isNewKey = !Object.prototype.hasOwnProperty.call( + + const decision = addAttribute( this.attributes, - key + this._logRecordLimits, + this._attributesCount, + key, + value ); - if ( - isNewKey && - this._attributesCount >= this._logRecordLimits.attributeCountLimit - ) { + + if (decision === AddAttributeDecision.DROP_LIMIT_REACHED) { this._droppedAttributesCount++; - // Only warn once per LogRecord to avoid log spam if (this._droppedAttributesCount === 1) { + // Only warn once per LogRecord to avoid log spam api.diag.warn('Dropping extra attributes.'); } - return this; - } - this.attributes[key] = this._truncateToSize(value); - if (isNewKey) { + } else if (decision === AddAttributeDecision.ADD_NEW) { this._attributesCount++; } + return this; } @@ -195,48 +190,6 @@ export class LogRecordImpl implements ReadableLogRecord { this._isReadonly = true; } - private _truncateToSize(value: AnyValue): AnyValue { - const limit = this._logRecordLimits.attributeValueLengthLimit; - // Check limit - if (limit <= 0) { - // Negative values are invalid, so do not truncate - api.diag.warn(`Attribute value limit must be positive, got ${limit}`); - return value; - } - - // null/undefined - no truncation needed - if (value == null) { - return value; - } - - // String - if (typeof value === 'string') { - return this._truncateToLimitUtil(value, limit); - } - - // Byte arrays - no truncation needed - if (value instanceof Uint8Array) { - return value; - } - - // Arrays (can contain any AnyValue types) - if (Array.isArray(value)) { - return value.map(val => this._truncateToSize(val)); - } - - // Objects/Maps - recursively truncate nested values - if (typeof value === 'object') { - const truncatedObj: Record = {}; - for (const [k, v] of Object.entries(value as Record)) { - truncatedObj[k] = this._truncateToSize(v); - } - return truncatedObj; - } - - // Other types (number, boolean), no need to apply value length limit - return value; - } - private _setException(exception: unknown): void { let hasMinimumAttributes = false; @@ -285,13 +238,6 @@ export class LogRecordImpl implements ReadableLogRecord { } } - private _truncateToLimitUtil(value: string, limit: number): string { - if (value.length <= limit) { - return value; - } - return value.substring(0, limit); - } - private _isLogRecordReadonly(): boolean { if (this._isReadonly) { api.diag.warn('Can not execute the operation on emitted log record'); diff --git a/experimental/packages/sdk-logs/src/Logger.ts b/experimental/packages/sdk-logs/src/Logger.ts index c550fafa762..985d5a2edd9 100644 --- a/experimental/packages/sdk-logs/src/Logger.ts +++ b/experimental/packages/sdk-logs/src/Logger.ts @@ -5,7 +5,6 @@ import type { Logger as ILogger, LogRecord } from '@opentelemetry/api-logs'; import { SeverityNumber } from '@opentelemetry/api-logs'; -import type { InstrumentationScope } from '@opentelemetry/core'; import type { Context } from '@opentelemetry/api'; import { context, @@ -17,23 +16,24 @@ import { import { LogRecordImpl } from './LogRecordImpl'; import type { LoggerProviderSharedState } from './internal/LoggerProviderSharedState'; import type { LoggerConfig } from './types'; +import type { LogInstrumentationScope } from './internal/utils'; export class Logger implements ILogger { - public readonly instrumentationScope: InstrumentationScope; - private _sharedState: LoggerProviderSharedState; + private readonly _instrumentationScope: LogInstrumentationScope; + private readonly _sharedState: LoggerProviderSharedState; private readonly _loggerConfig: Required; constructor( - instrumentationScope: InstrumentationScope, + instrumentationScope: LogInstrumentationScope, sharedState: LoggerProviderSharedState ) { - this.instrumentationScope = instrumentationScope; + this._instrumentationScope = instrumentationScope; this._sharedState = sharedState; // Cache the logger configuration at construction time // Since we don't support re-configuration, this avoids map lookups // and string allocations on each emit() call this._loggerConfig = this._sharedState.getLoggerConfig( - this.instrumentationScope + this._instrumentationScope ); } @@ -50,7 +50,7 @@ export class Logger implements ILogger { */ const logRecordInstance = new LogRecordImpl( this._sharedState, - this.instrumentationScope, + this._instrumentationScope, { context: currentContext, ...logRecord, @@ -106,7 +106,7 @@ export class Logger implements ILogger { // Lastly check if there is any enabled processor const enabledOpts = { context: currentContext, - instrumentationScope: this.instrumentationScope, + instrumentationScope: this._instrumentationScope, severityNumber: options?.severityNumber, eventName: options?.eventName, }; diff --git a/experimental/packages/sdk-logs/src/LoggerProvider.ts b/experimental/packages/sdk-logs/src/LoggerProvider.ts index 48b1d98847f..52237bf75eb 100644 --- a/experimental/packages/sdk-logs/src/LoggerProvider.ts +++ b/experimental/packages/sdk-logs/src/LoggerProvider.ts @@ -18,6 +18,11 @@ import { DEFAULT_LOGGER_CONFIGURATOR, LoggerProviderSharedState, } from './internal/LoggerProviderSharedState'; +import { + getInstrumentationScopeKey, + type LogInstrumentationScope, +} from './internal/utils'; +import { normalizeScopeAttributes } from './utils/validation'; export const DEFAULT_LOGGER_NAME = 'unknown'; @@ -67,14 +72,20 @@ export class LoggerProvider implements ILoggerProvider { diag.warn('Logger requested without instrumentation scope name.'); } const loggerName = name || DEFAULT_LOGGER_NAME; - const key = `${loggerName}@${version || ''}:${options?.schemaUrl || ''}`; + const instrumentationScope: LogInstrumentationScope = { + name: loggerName, + version, + schemaUrl: options?.schemaUrl, + ...normalizeScopeAttributes( + this._sharedState.logRecordLimits, + options?.attributes + ), + }; + const key = getInstrumentationScopeKey(instrumentationScope); if (!this._sharedState.loggers.has(key)) { this._sharedState.loggers.set( key, - new Logger( - { name: loggerName, version, schemaUrl: options?.schemaUrl }, - this._sharedState - ) + new Logger(instrumentationScope, this._sharedState) ); } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion diff --git a/experimental/packages/sdk-logs/src/export/ReadableLogRecord.ts b/experimental/packages/sdk-logs/src/export/ReadableLogRecord.ts index 7e1f6046a20..c9517d32b01 100644 --- a/experimental/packages/sdk-logs/src/export/ReadableLogRecord.ts +++ b/experimental/packages/sdk-logs/src/export/ReadableLogRecord.ts @@ -26,7 +26,10 @@ export interface ReadableLogRecord { * MUST be stable across identical scopes, as it is intended be used for efficient scope-based * filtering and grouping. */ - readonly instrumentationScope: InstrumentationScope; + readonly instrumentationScope: InstrumentationScope & { + attributes?: LogAttributes; + droppedAttributesCount?: number; + }; readonly attributes: LogAttributes; readonly droppedAttributesCount: number; } diff --git a/experimental/packages/sdk-logs/src/export/SdkLogRecord.ts b/experimental/packages/sdk-logs/src/export/SdkLogRecord.ts index c1356671b2e..f91ee9d4d06 100644 --- a/experimental/packages/sdk-logs/src/export/SdkLogRecord.ts +++ b/experimental/packages/sdk-logs/src/export/SdkLogRecord.ts @@ -27,6 +27,13 @@ export interface SdkLogRecord { readonly hrTimeObserved: HrTime; readonly spanContext?: SpanContext; readonly resource: Resource; + /** + * The instrumentation scope associated with the log record. + * + * Downstream consumers may group exported records by the identity of this + * object. If a distinct grouping is required, construct a new scope object + * instead of reusing an existing one. + */ readonly instrumentationScope: InstrumentationScope; readonly attributes: LogAttributes; severityText?: string; diff --git a/experimental/packages/sdk-logs/src/internal/utils.ts b/experimental/packages/sdk-logs/src/internal/utils.ts index 9d390c62ff4..edc45e473c8 100644 --- a/experimental/packages/sdk-logs/src/internal/utils.ts +++ b/experimental/packages/sdk-logs/src/internal/utils.ts @@ -3,15 +3,80 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type { AnyValue, LogAttributes } from '@opentelemetry/api-logs'; import type { InstrumentationScope } from '@opentelemetry/core'; +export type LogInstrumentationScope = InstrumentationScope & { + readonly attributes?: LogAttributes; + readonly droppedAttributesCount?: number; +}; + +/** + * Normalizes an AnyValue to a JSON-serializable [typeTag, payload] tuple. + * + * Using a type tag as the first element guarantees that two values can only + * produce the same tuple when they have the same type AND the same data, + * avoiding cross-type collisions such as: + * - null vs NaN vs Infinity (all become JSON `null` via JSON.stringify) + * - -0 vs 0 (both become JSON `0` via JSON.stringify) + * - string "null" vs the value null + * + * Object keys are sorted so that attribute maps with the same entries but + * different insertion orders produce the same key. + */ +function normalizeAnyValue(value: AnyValue): [string, unknown] { + if (value === undefined) { + return ['u', null]; + } + if (value === null) { + return ['n', null]; + } + + const valueType = typeof value; + if (valueType === 'string') { + return ['s', value]; + } + if (valueType === 'boolean') { + return ['b', value]; + } + if (valueType === 'number') { + if (Number.isNaN(value)) return ['nan', null]; + if (value === Infinity) return ['inf', null]; + if (value === -Infinity) return ['-inf', null]; + if (Object.is(value, -0)) return ['n0', null]; + return ['d', value]; + } + if (value instanceof Uint8Array) { + return ['bytes', Array.from(value)]; + } + if (Array.isArray(value)) { + return ['arr', value.map(normalizeAnyValue)]; + } + // AnyValueMap — sort keys for insertion-order independence + return [ + 'map', + Object.entries(value) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([k, v]) => [k, normalizeAnyValue(v)]), + ]; +} + /** * Converting the instrumentation scope object to a unique identifier string. * @param scope - The instrumentation scope to convert * @returns A unique string identifier for the scope */ export function getInstrumentationScopeKey( - scope: InstrumentationScope + scope: LogInstrumentationScope ): string { - return `${scope.name}@${scope.version || ''}:${scope.schemaUrl || ''}`; + return JSON.stringify([ + scope.name, + scope.version || '', + scope.schemaUrl || '', + normalizeAnyValue(scope.attributes), + // we include the dropped attributes count to avoid collisions between scopes with the same identifying + // characteristics, but different dropped counts. While there still can be collisions this is the best we can do if + // we want to resolve the same logger without relying on object identity. + scope.droppedAttributesCount ?? 0, + ]); } diff --git a/experimental/packages/sdk-logs/src/utils/validation.ts b/experimental/packages/sdk-logs/src/utils/validation.ts index 9e8e402e898..37c39079903 100644 --- a/experimental/packages/sdk-logs/src/utils/validation.ts +++ b/experimental/packages/sdk-logs/src/utils/validation.ts @@ -3,12 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { AnyValue } from '@opentelemetry/api-logs'; +import { diag } from '@opentelemetry/api'; +import type { AnyValue, LogAttributes } from '@opentelemetry/api-logs'; +import type { LogRecordLimits } from '../types'; /** * Validates if a value is a valid AnyValue for Log Attributes according to OpenTelemetry spec. * Log Attributes support a superset of standard Attributes and must support: - * - Scalar values: string, boolean, signed 64 bit integer, or double precision floating point + * - Scalar values: string, boolean, signed 64-bit integer, or double precision floating point * - Byte arrays (Uint8Array) * - Arrays of any values (heterogeneous arrays allowed) * - Maps from string to any value (nested objects) @@ -54,7 +56,12 @@ function isLogAttributeValueInternal( // Arrays (can contain any AnyValue, including heterogeneous) if (Array.isArray(val)) { - return val.every(item => isLogAttributeValueInternal(item, visited)); + for (const item of val) { + if (!isLogAttributeValueInternal(item, visited)) { + return false; + } + } + return true; } // Only accept plain objects (not built-in objects like Date, RegExp, Error, etc.) @@ -66,10 +73,142 @@ function isLogAttributeValueInternal( // Objects/Maps (including empty objects) // All object properties must be valid AnyValues - return Object.values(obj).every(item => - isLogAttributeValueInternal(item, visited) - ); + for (const key in obj) { + if ( + Object.prototype.hasOwnProperty.call(obj, key) && + !isLogAttributeValueInternal(obj[key], visited) + ) { + return false; + } + } + return true; } return false; } + +export const enum AddAttributeDecision { + DROP_INVALID = 0, + DROP_LIMIT_REACHED = 1, + ADD_NEW = 2, + ADD_OVERWRITE_EXISTING = 3, +} + +export function addAttribute( + attributes: LogAttributes, + limits: Readonly>, + currentAttributesCount: number, + key: string, + value?: AnyValue +): AddAttributeDecision { + if (key.length === 0) { + diag.warn(`Invalid attribute key: ${key}`); + return AddAttributeDecision.DROP_INVALID; + } + if (!isLogAttributeValue(value)) { + diag.warn(`Invalid attribute value set for key: ${key}`); + return AddAttributeDecision.DROP_INVALID; + } + const isNewKey = !Object.prototype.hasOwnProperty.call(attributes, key); + if (isNewKey && currentAttributesCount >= limits.attributeCountLimit) { + return AddAttributeDecision.DROP_LIMIT_REACHED; + } + + attributes[key] = truncateToSize(value, limits.attributeValueLengthLimit); + if (isNewKey) { + return AddAttributeDecision.ADD_NEW; + } + return AddAttributeDecision.ADD_OVERWRITE_EXISTING; +} + +function truncateToSize(value: AnyValue, limit: number): AnyValue { + // Check limit + if (limit <= 0) { + // Negative values are invalid, so do not truncate + diag.warn(`Attribute value limit must be positive, got ${limit}`); + return value; + } + + // null/undefined - no truncation needed + if (value == null) { + return value; + } + + // String + if (typeof value === 'string') { + if (value.length <= limit) { + return value; + } + return value.substring(0, limit); + } + + // Byte arrays - no truncation needed + if (value instanceof Uint8Array) { + return value; + } + + // Arrays (can contain any AnyValue types) + if (Array.isArray(value)) { + return value.map(val => truncateToSize(val, limit)); + } + + // Objects/Maps - recursively truncate nested values + if (typeof value === 'object') { + const truncatedObj: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + truncatedObj[k] = truncateToSize(v, limit); + } + return truncatedObj; + } + + // Other types (number, boolean), no need to apply value length limit + return value; +} + +/** + * Normalize attributes for use on the instrumentation scope. Drops invalid attributes and keeps track of + * how many were dropped. + * + * @param limits + * @param attributes + */ +export function normalizeScopeAttributes( + limits: Readonly>, + attributes?: LogAttributes +): { + readonly attributes?: LogAttributes; + readonly droppedAttributesCount?: number; +} { + if (attributes == null) { + return {}; + } + + const normalizedAttributes: LogAttributes = {}; + let currentAttributesCount = 0; + let droppedAttributesCount = 0; + + for (const [key, value] of Object.entries(attributes)) { + const decision = addAttribute( + normalizedAttributes, + limits, + currentAttributesCount, + key, + value + ); + + if (decision === AddAttributeDecision.ADD_NEW) { + currentAttributesCount += 1; + } else if (decision === AddAttributeDecision.DROP_INVALID) { + droppedAttributesCount += 1; + } else if (decision === AddAttributeDecision.DROP_LIMIT_REACHED) { + droppedAttributesCount += 1; + } else { + // do nothing + } + } + + return { + attributes: currentAttributesCount > 0 ? normalizedAttributes : undefined, + droppedAttributesCount, + }; +} diff --git a/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts b/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts index ed53e575381..42702d273b8 100644 --- a/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts +++ b/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import { logs } from '@opentelemetry/api-logs'; +import type { LogAttributes } from '@opentelemetry/api-logs'; import { diag } from '@opentelemetry/api'; import { defaultResource, @@ -161,11 +162,22 @@ describe('LoggerProvider', () => { const testName = 'test name'; const testVersion = 'test version'; const testSchemaURL = 'test schema url'; + const testScopeAttributes = { + service: 'payments', + nested: { + enabled: true, + region: 'us-east-1', + }, + tags: ['prod', 'logs'], + }; it('should create a logger instance with default name if the name is invalid ', () => { const provider = new LoggerProvider(); const logger = provider.getLogger('') as Logger; - assert.strictEqual(logger.instrumentationScope.name, DEFAULT_LOGGER_NAME); + assert.strictEqual( + logger['_instrumentationScope'].name, + DEFAULT_LOGGER_NAME + ); }); it("should create a logger instance if the name doesn't exist", () => { @@ -189,6 +201,30 @@ describe('LoggerProvider', () => { assert.strictEqual(sharedState.loggers.size, 3); }); + it('should create a new object if scope attributes are not unique', function () { + // arrange + const provider = new LoggerProvider(); + const sharedState = provider['_sharedState']; + + // act + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: testScopeAttributes, + }); + + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { + ...testScopeAttributes, + service: 'checkout', + }, + }); + + // assert + assert.strictEqual(sharedState.loggers.size, 2); + assert.notStrictEqual(logger1, logger2); + }); + it('should not create A new object if the name & version & schemaUrl are unique', () => { const provider = new LoggerProvider(); const sharedState = provider['_sharedState']; @@ -207,6 +243,193 @@ describe('LoggerProvider', () => { assert.ok(logger2 instanceof Logger); assert.strictEqual(logger1, logger2); }); + + it('should not create a new object for equivalent scope attributes data', function () { + // arrange + const provider = new LoggerProvider(); + const sharedState = provider['_sharedState']; + + // act + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: testScopeAttributes, + }); + + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { + tags: ['prod', 'logs'], + nested: { + region: 'us-east-1', + enabled: true, + }, + service: 'payments', + }, + }); + + // assert + assert.strictEqual(logger1, logger2); + assert.strictEqual(sharedState.loggers.size, 1); + }); + + it('should drop invalid scope attributes but still use distinct logger due to dropped attrs', () => { + // arrange + const warnStub = sinon.stub(diag, 'warn'); + const provider = new LoggerProvider(); + const circular: Record = {}; + circular.self = circular; + const invalidScopeAttributes = { + valid: 'payments', + invalid: circular, + '': 'empty-key', + } as unknown as LogAttributes; + + // act + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: invalidScopeAttributes, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { + valid: 'payments', + }, + }); + + // assert + assert.deepStrictEqual( + (logger1 as Logger)['_instrumentationScope'].attributes, + { + valid: 'payments', + } + ); + assert.strictEqual( + (logger1 as Logger)['_instrumentationScope'].droppedAttributesCount, + 2 + ); + assert.notStrictEqual(logger1, logger2); + sinon.assert.calledWith( + warnStub, + 'Invalid attribute value set for key: invalid' + ); + sinon.assert.calledWith(warnStub, 'Invalid attribute key: '); + }); + + it('should distinguish NaN from null in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { n: null }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { n: NaN }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should distinguish Infinity from null in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: null }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: Infinity }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should distinguish -Infinity from Infinity in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: Infinity }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: -Infinity }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should distinguish -0 from 0 in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: 0 }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: -0 }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should distinguish string "null" from null in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: null }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: 'null' }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should return the same logger for identical special-value attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { a: NaN, b: Infinity, c: -Infinity }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { a: NaN, b: Infinity, c: -Infinity }, + }); + assert.strictEqual(logger1, logger2); + }); + + it('should distinguish Uint8Arrays from in scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: new Uint8Array([1, 2, 3]) }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: new Uint8Array([3, 2, 1]) }, + }); + assert.notStrictEqual(logger1, logger2); + }); + + it('should return the same logger for identical Uint8Array attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: new Uint8Array([1, 2, 3]) }, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: { v: new Uint8Array([1, 2, 3]) }, + }); + assert.strictEqual(logger1, logger2); + }); + + it('should return the same logger for empty scope attributes and no scope attributes', () => { + const provider = new LoggerProvider(); + const logger1 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + }); + const logger2 = provider.getLogger(testName, testVersion, { + schemaUrl: testSchemaURL, + attributes: {}, + }); + assert.strictEqual(logger1, logger2); + }); }); describe('.forceFlush()', () => {