From cb0461a9391da16b5d09433aacf0037ea5eeae60 Mon Sep 17 00:00:00 2001 From: Alex Velikanov Date: Wed, 25 Jun 2025 15:53:39 -0400 Subject: [PATCH 1/3] fix(api-logs,sdk-logs): allow AnyValue attributes for logs and handle circular json --- experimental/packages/api-logs/src/index.ts | 1 + .../packages/api-logs/src/utils/validation.ts | 77 ++++++ .../test/common/utils/validation.test.ts | 255 ++++++++++++++++++ .../packages/sdk-logs/src/LogRecordImpl.ts | 52 ++-- .../sdk-logs/test/common/LogRecord.test.ts | 190 +++++++++++++ .../packages/sdk-logs/test/common/utils.ts | 6 +- 6 files changed, 552 insertions(+), 29 deletions(-) create mode 100644 experimental/packages/api-logs/src/utils/validation.ts create mode 100644 experimental/packages/api-logs/test/common/utils/validation.test.ts diff --git a/experimental/packages/api-logs/src/index.ts b/experimental/packages/api-logs/src/index.ts index f07793d370b..dd17e290163 100644 --- a/experimental/packages/api-logs/src/index.ts +++ b/experimental/packages/api-logs/src/index.ts @@ -20,6 +20,7 @@ export { SeverityNumber } from './types/LogRecord'; export type { LogAttributes, LogBody, LogRecord } from './types/LogRecord'; export type { LoggerOptions } from './types/LoggerOptions'; export type { AnyValue, AnyValueMap } from './types/AnyValue'; +export { isLogAttributeValue } from './utils/validation'; export { NOOP_LOGGER, NoopLogger } from './NoopLogger'; export { NOOP_LOGGER_PROVIDER, NoopLoggerProvider } from './NoopLoggerProvider'; export { ProxyLogger } from './ProxyLogger'; diff --git a/experimental/packages/api-logs/src/utils/validation.ts b/experimental/packages/api-logs/src/utils/validation.ts new file mode 100644 index 00000000000..02fa29c1d6e --- /dev/null +++ b/experimental/packages/api-logs/src/utils/validation.ts @@ -0,0 +1,77 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AnyValue } from '../types/AnyValue'; + +/** + * 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 + * - Byte arrays (Uint8Array) + * - Arrays of any values (heterogeneous arrays allowed) + * - Maps from string to any value (nested objects) + * - Empty values (null/undefined) + * + * @param val - The value to validate + * @returns true if the value is a valid AnyValue, false otherwise + */ +export function isLogAttributeValue(val: unknown): val is AnyValue { + return isLogAttributeValueInternal(val, new WeakSet()); +} + +function isLogAttributeValueInternal(val: unknown, visited: WeakSet): val is AnyValue { + // null and undefined are explicitly allowed + if (val == null) { + return true; + } + + // Scalar values + if (typeof val === 'string' || typeof val === 'number' || typeof val === 'boolean') { + return true; + } + + // Byte arrays + if (val instanceof Uint8Array) { + return true; + } + + // For objects and arrays, check for circular references + if (typeof val === 'object') { + if (visited.has(val as object)) { + // Circular reference detected - reject it + return false; + } + visited.add(val as object); + + // Arrays (can contain any AnyValue, including heterogeneous) + if (Array.isArray(val)) { + return val.every(item => isLogAttributeValueInternal(item, visited)); + } + + // Only accept plain objects (not built-in objects like Date, RegExp, Error, etc.) + // Check if it's a plain object by verifying its constructor is Object or it has no constructor + const obj = val as Record; + if (obj.constructor !== Object && obj.constructor !== undefined) { + return false; + } + + // Objects/Maps (including empty objects) + // All object properties must be valid AnyValues + return Object.values(obj).every(item => isLogAttributeValueInternal(item, visited)); + } + + return false; +} diff --git a/experimental/packages/api-logs/test/common/utils/validation.test.ts b/experimental/packages/api-logs/test/common/utils/validation.test.ts new file mode 100644 index 00000000000..4bf3a911e68 --- /dev/null +++ b/experimental/packages/api-logs/test/common/utils/validation.test.ts @@ -0,0 +1,255 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import { isLogAttributeValue } from '../../../src/utils/validation'; + +describe('isLogAttributeValue', () => { + describe('should accept scalar values', () => { + it('should accept strings', () => { + assert.strictEqual(isLogAttributeValue('test'), true); + assert.strictEqual(isLogAttributeValue(''), true); + assert.strictEqual(isLogAttributeValue('multi\nline'), true); + assert.strictEqual(isLogAttributeValue('unicode: 🎉'), true); + }); + + it('should accept numbers', () => { + assert.strictEqual(isLogAttributeValue(42), true); + assert.strictEqual(isLogAttributeValue(0), true); + assert.strictEqual(isLogAttributeValue(-123), true); + assert.strictEqual(isLogAttributeValue(3.14159), true); + assert.strictEqual(isLogAttributeValue(Number.MAX_SAFE_INTEGER), true); + assert.strictEqual(isLogAttributeValue(Number.MIN_SAFE_INTEGER), true); + assert.strictEqual(isLogAttributeValue(Infinity), true); + assert.strictEqual(isLogAttributeValue(-Infinity), true); + assert.strictEqual(isLogAttributeValue(NaN), true); + }); + + it('should accept booleans', () => { + assert.strictEqual(isLogAttributeValue(true), true); + assert.strictEqual(isLogAttributeValue(false), true); + }); + }); + + describe('should accept null and undefined values', () => { + it('should accept null', () => { + assert.strictEqual(isLogAttributeValue(null), true); + }); + + it('should accept undefined', () => { + assert.strictEqual(isLogAttributeValue(undefined), true); + }); + }); + + describe('should accept byte arrays', () => { + it('should accept Uint8Array', () => { + assert.strictEqual(isLogAttributeValue(new Uint8Array([1, 2, 3])), true); + assert.strictEqual(isLogAttributeValue(new Uint8Array(0)), true); + assert.strictEqual(isLogAttributeValue(new Uint8Array([255, 0, 128])), true); + }); + }); + + describe('should accept arrays', () => { + it('should accept homogeneous arrays', () => { + assert.strictEqual(isLogAttributeValue(['a', 'b', 'c']), true); + assert.strictEqual(isLogAttributeValue([1, 2, 3]), true); + assert.strictEqual(isLogAttributeValue([true, false]), true); + }); + + it('should accept heterogeneous arrays', () => { + assert.strictEqual(isLogAttributeValue(['string', 42, true]), true); + assert.strictEqual(isLogAttributeValue([null, undefined, 'test']), true); + assert.strictEqual(isLogAttributeValue(['test', new Uint8Array([1, 2])]), true); + }); + + it('should accept nested arrays', () => { + assert.strictEqual(isLogAttributeValue([['a', 'b'], [1, 2]]), true); + assert.strictEqual(isLogAttributeValue([[1, 2, 3], ['nested', 'array']]), true); + }); + + it('should accept arrays with null/undefined', () => { + assert.strictEqual(isLogAttributeValue([null, 'test', undefined]), true); + assert.strictEqual(isLogAttributeValue([]), true); + }); + + it('should accept arrays with objects', () => { + assert.strictEqual(isLogAttributeValue([{ key: 'value' }, 'string']), true); + }); + }); + + describe('should accept objects/maps', () => { + it('should accept simple objects', () => { + assert.strictEqual(isLogAttributeValue({ key: 'value' }), true); + assert.strictEqual(isLogAttributeValue({ num: 42, bool: true }), true); + }); + + it('should accept empty objects', () => { + assert.strictEqual(isLogAttributeValue({}), true); + }); + + it('should accept nested objects', () => { + const nested = { + level1: { + level2: { + deep: 'value', + number: 123 + } + } + }; + assert.strictEqual(isLogAttributeValue(nested), true); + }); + + it('should accept objects with arrays', () => { + const obj = { + strings: ['a', 'b'], + numbers: [1, 2, 3], + mixed: ['str', 42, true] + }; + assert.strictEqual(isLogAttributeValue(obj), true); + }); + + it('should accept objects with null/undefined values', () => { + assert.strictEqual(isLogAttributeValue({ nullVal: null, undefVal: undefined }), true); + }); + + it('should accept objects with byte arrays', () => { + assert.strictEqual(isLogAttributeValue({ bytes: new Uint8Array([1, 2, 3]) }), true); + }); + }); + + describe('should accept complex combinations', () => { + it('should accept deeply nested structures', () => { + const complex = { + scalars: { + str: 'test', + num: 42, + bool: true + }, + arrays: { + homogeneous: ['a', 'b', 'c'], + heterogeneous: [1, 'two', true, null], + nested: [[1, 2], ['a', 'b']] + }, + bytes: new Uint8Array([255, 254, 253]), + nullish: { + nullValue: null, + undefinedValue: undefined + }, + empty: {} + }; + assert.strictEqual(isLogAttributeValue(complex), true); + }); + + it('should accept arrays of complex objects', () => { + const arrayOfObjects = [ + { name: 'obj1', value: 123 }, + { name: 'obj2', nested: { deep: 'value' } }, + { bytes: new Uint8Array([1, 2, 3]) } + ]; + assert.strictEqual(isLogAttributeValue(arrayOfObjects), true); + }); + }); + + describe('should reject invalid values', () => { + it('should reject functions', () => { + assert.strictEqual(isLogAttributeValue(() => {}), false); + assert.strictEqual(isLogAttributeValue(function() {}), false); + }); + + it('should reject symbols', () => { + assert.strictEqual(isLogAttributeValue(Symbol('test')), false); + assert.strictEqual(isLogAttributeValue(Symbol.for('test')), false); + }); + + it('should reject Date objects', () => { + assert.strictEqual(isLogAttributeValue(new Date()), false); + }); + + it('should reject RegExp objects', () => { + assert.strictEqual(isLogAttributeValue(/test/), false); + }); + + it('should reject Error objects', () => { + assert.strictEqual(isLogAttributeValue(new Error('test')), false); + }); + + it('should reject class instances', () => { + class TestClass { + value = 'test'; + } + assert.strictEqual(isLogAttributeValue(new TestClass()), false); + }); + + it('should reject arrays containing invalid values', () => { + assert.strictEqual(isLogAttributeValue(['valid', () => {}]), false); + assert.strictEqual(isLogAttributeValue([Symbol('test'), 'valid']), false); + assert.strictEqual(isLogAttributeValue([new Date()]), false); + }); + + it('should reject objects containing invalid values', () => { + assert.strictEqual(isLogAttributeValue({ valid: 'test', invalid: () => {} }), false); + assert.strictEqual(isLogAttributeValue({ symbol: Symbol('test') }), false); + assert.strictEqual(isLogAttributeValue({ date: new Date() }), false); + }); + + it('should reject deeply nested invalid values', () => { + const nested = { + level1: { + level2: { + valid: 'value', + invalid: Symbol('test') + } + } + }; + assert.strictEqual(isLogAttributeValue(nested), false); + }); + + it('should reject arrays with nested invalid values', () => { + const nestedArray = [ + ['valid', 'array'], + ['has', Symbol('invalid')] + ]; + assert.strictEqual(isLogAttributeValue(nestedArray), false); + }); + }); + + describe('edge cases', () => { + it('should handle circular references gracefully', () => { + const circular: any = { a: 'test' }; + circular.self = circular; + + // This should not throw an error, though it might return false + // The exact behavior isn't specified in the OpenTelemetry spec + const result = isLogAttributeValue(circular); + assert.strictEqual(typeof result, 'boolean'); + }); + + it('should handle very deep nesting', () => { + let deep: any = 'bottom'; + for (let i = 0; i < 100; i++) { + deep = { level: i, nested: deep }; + } + + const result = isLogAttributeValue(deep); + assert.strictEqual(typeof result, 'boolean'); + }); + + it('should handle large arrays', () => { + const largeArray = new Array(1000).fill('test'); + assert.strictEqual(isLogAttributeValue(largeArray), true); + }); + }); +}); diff --git a/experimental/packages/sdk-logs/src/LogRecordImpl.ts b/experimental/packages/sdk-logs/src/LogRecordImpl.ts index c0ec70daaee..4bb97360a3d 100644 --- a/experimental/packages/sdk-logs/src/LogRecordImpl.ts +++ b/experimental/packages/sdk-logs/src/LogRecordImpl.ts @@ -14,19 +14,18 @@ * limitations under the License. */ -import { AttributeValue, diag } from '@opentelemetry/api'; +import { diag } from '@opentelemetry/api'; import type * as logsAPI from '@opentelemetry/api-logs'; import * as api from '@opentelemetry/api'; import { timeInputToHrTime, - isAttributeValue, InstrumentationScope, } from '@opentelemetry/core'; import type { Resource } from '@opentelemetry/resources'; import type { ReadableLogRecord } from './export/ReadableLogRecord'; import type { LogRecordLimits } from './types'; -import { AnyValue, LogAttributes, LogBody } from '@opentelemetry/api-logs'; +import { AnyValue, LogAttributes, LogBody, isLogAttributeValue } from '@opentelemetry/api-logs'; import { LoggerProviderSharedState } from './internal/LoggerProviderSharedState'; export class LogRecordImpl implements ReadableLogRecord { @@ -129,21 +128,11 @@ export class LogRecordImpl implements ReadableLogRecord { if (this._isLogRecordReadonly()) { return this; } - if (value === null) { - return this; - } if (key.length === 0) { api.diag.warn(`Invalid attribute key: ${key}`); return this; } - if ( - !isAttributeValue(value) && - !( - typeof value === 'object' && - !Array.isArray(value) && - Object.keys(value).length > 0 - ) - ) { + if (!isLogAttributeValue(value)) { api.diag.warn(`Invalid attribute value set for key: ${key}`); return this; } @@ -159,11 +148,7 @@ export class LogRecordImpl implements ReadableLogRecord { } return this; } - if (isAttributeValue(value)) { - this.attributes[key] = this._truncateToSize(value); - } else { - this.attributes[key] = value; - } + this.attributes[key] = this._truncateToSize(value); return this; } @@ -203,7 +188,7 @@ export class LogRecordImpl implements ReadableLogRecord { this._isReadonly = true; } - private _truncateToSize(value: AttributeValue): AttributeValue { + private _truncateToSize(value: AnyValue): AnyValue { const limit = this._logRecordLimits.attributeValueLengthLimit; // Check limit if (limit <= 0) { @@ -212,19 +197,36 @@ export class LogRecordImpl implements ReadableLogRecord { return value; } + // null/undefined - no truncation needed + if (value == null) { + return value; + } + // String if (typeof value === 'string') { return this._truncateToLimitUtil(value, limit); } - // Array of strings + // Byte arrays - no truncation needed + if (value instanceof Uint8Array) { + return value; + } + + // Arrays (can contain any AnyValue types) if (Array.isArray(value)) { - return (value as []).map(val => - typeof val === 'string' ? this._truncateToLimitUtil(val, limit) : val - ); + 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, no need to apply value length limit + // Other types (number, boolean), no need to apply value length limit return value; } diff --git a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts index c85b9bc6fd0..823d1bc043d 100644 --- a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts +++ b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts @@ -25,6 +25,7 @@ import { TraceFlags, } from '@opentelemetry/api'; import * as logsAPI from '@opentelemetry/api-logs'; +import { AnyValue } from '@opentelemetry/api-logs'; import type { HrTime } from '@opentelemetry/api'; import { hrTimeToMilliseconds, timeInputToHrTime } from '@opentelemetry/core'; import { defaultResource } from '@opentelemetry/resources'; @@ -427,6 +428,195 @@ describe('LogRecord', () => { }); }); + describe('OpenTelemetry Log Attributes spec compliance', () => { + describe('should support all AnyValue types per OpenTelemetry spec', () => { + it('should support scalar values (string, number, boolean)', () => { + const { logRecord } = setup(); + logRecord.setAttribute('string', 'test'); + logRecord.setAttribute('number', 42); + logRecord.setAttribute('boolean', true); + logRecord.setAttribute('negativeNumber', -123.45); + + assert.strictEqual(logRecord.attributes.string, 'test'); + assert.strictEqual(logRecord.attributes.number, 42); + assert.strictEqual(logRecord.attributes.boolean, true); + assert.strictEqual(logRecord.attributes.negativeNumber, -123.45); + }); + + it('should support byte arrays (Uint8Array)', () => { + const { logRecord } = setup(); + const byteArray = new Uint8Array([1, 2, 3, 4, 5]); + logRecord.setAttribute('bytes', byteArray); + + assert.deepStrictEqual(logRecord.attributes.bytes, byteArray); + assert.ok(logRecord.attributes.bytes instanceof Uint8Array); + }); + + it('should support heterogeneous arrays (arrays with mixed types)', () => { + const { logRecord } = setup(); + const mixedArray = ['string', 42, true, null, new Uint8Array([1, 2])]; + logRecord.setAttribute('mixedArray', mixedArray); + + assert.deepStrictEqual(logRecord.attributes.mixedArray, mixedArray); + }); + + it('should support nested arrays', () => { + const { logRecord } = setup(); + const nestedArray = [['a', 'b'], [1, 2], [true, false]]; + logRecord.setAttribute('nestedArray', nestedArray); + + assert.deepStrictEqual(logRecord.attributes.nestedArray, nestedArray); + }); + + it('should support nested objects/maps', () => { + const { logRecord } = setup(); + const nestedObject = { + level1: { + level2: { + string: 'deep value', + number: 123, + array: ['nested', 'array'] + } + }, + topLevel: 'value' + }; + logRecord.setAttribute('nested', nestedObject); + + assert.deepStrictEqual(logRecord.attributes.nested, nestedObject); + }); + + it('should support empty objects', () => { + const { logRecord } = setup(); + const emptyObj = {}; + logRecord.setAttribute('empty', emptyObj); + + assert.deepStrictEqual(logRecord.attributes.empty, emptyObj); + }); + + it('should support null and undefined values', () => { + const { logRecord } = setup(); + logRecord.setAttribute('nullValue', null); + logRecord.setAttribute('undefinedValue', undefined); + + assert.strictEqual(logRecord.attributes.nullValue, null); + assert.strictEqual(logRecord.attributes.undefinedValue, undefined); + }); + + it('should support complex combinations of AnyValue types', () => { + const { logRecord } = setup(); + const complexValue = { + scalars: { + str: 'test', + num: 42, + bool: true + }, + arrays: { + homogeneous: ['a', 'b', 'c'], + heterogeneous: [1, 'two', true, null], + nested: [[1, 2], ['a', 'b']] + }, + bytes: new Uint8Array([255, 254, 253]), + nullish: { + nullValue: null, + undefinedValue: undefined + }, + empty: {} + }; + logRecord.setAttribute('complex', complexValue); + + assert.deepStrictEqual(logRecord.attributes.complex, complexValue); + }); + }); + + describe('should properly truncate string values in complex structures', () => { + it('should truncate strings in nested objects', () => { + const { logRecord } = setup({ attributeValueLengthLimit: 5 }); + const nestedObject = { + level1: { + shortString: 'ok', + longString: 'this is too long' + }, + topLevelLong: 'also too long' + }; + logRecord.setAttribute('nested', nestedObject); + + const expected = { + level1: { + shortString: 'ok', + longString: 'this ' + }, + topLevelLong: 'also ' + }; + assert.deepStrictEqual(logRecord.attributes.nested, expected); + }); + + it('should truncate strings in heterogeneous arrays', () => { + const { logRecord } = setup({ attributeValueLengthLimit: 5 }); + const mixedArray = ['short', 'this is too long', 42, true, 'another long string']; + logRecord.setAttribute('mixed', mixedArray); + + const expected = ['short', 'this ', 42, true, 'anoth']; + assert.deepStrictEqual(logRecord.attributes.mixed, expected); + }); + + it('should not truncate non-string values', () => { + const { logRecord } = setup({ attributeValueLengthLimit: 5 }); + const byteArray = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); + logRecord.setAttribute('bytes', byteArray); + + // Byte arrays should not be truncated + assert.deepStrictEqual(logRecord.attributes.bytes, byteArray); + }); + }); + + describe('should reject invalid values', () => { + it('should reject functions', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + logRecord.setAttribute('func', (() => 'test') as unknown as AnyValue); + + assert.strictEqual(logRecord.attributes.func, undefined); + sinon.assert.calledWith(warnStub, 'Invalid attribute value set for key: func'); + warnStub.restore(); + }); + + it('should reject symbols', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + logRecord.setAttribute('symbol', Symbol('test') as any); + + assert.strictEqual(logRecord.attributes.symbol, undefined); + sinon.assert.calledWith(warnStub, 'Invalid attribute value set for key: symbol'); + warnStub.restore(); + }); + + it('should reject objects with invalid nested values', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + const invalidNested = { + valid: 'string', + invalid: Symbol('test') + }; + logRecord.setAttribute('nested', invalidNested as any); + + assert.strictEqual(logRecord.attributes.nested, undefined); + sinon.assert.calledWith(warnStub, 'Invalid attribute value set for key: nested'); + warnStub.restore(); + }); + + it('should reject arrays with invalid nested values', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + const invalidArray = ['valid', Symbol('invalid')]; + logRecord.setAttribute('array', invalidArray as any); + + assert.strictEqual(logRecord.attributes.array, undefined); + sinon.assert.calledWith(warnStub, 'Invalid attribute value set for key: array'); + warnStub.restore(); + }); + }); + }); + describe('log record processor', () => { it('should call onEmit synchronously when log record is emitted', () => { let emitted = false; diff --git a/experimental/packages/sdk-logs/test/common/utils.ts b/experimental/packages/sdk-logs/test/common/utils.ts index 80dc84e0c74..8363bd70b67 100644 --- a/experimental/packages/sdk-logs/test/common/utils.ts +++ b/experimental/packages/sdk-logs/test/common/utils.ts @@ -22,13 +22,11 @@ export const validAttributes = { 'array': [1, 2], 'array': [true, false], object: { bar: 'foo' }, + 'empty-object': {}, + 'non-homogeneous-array': [0, ''], }; export const invalidAttributes = { - // invalid attribute empty object - object: {}, - // invalid attribute inhomogeneous array - 'non-homogeneous-array': [0, ''], // This empty length attribute should not be set '': 'empty-key', }; From 75f3d26fa35b2ffc052a7acdef4fcffce93cc6e3 Mon Sep 17 00:00:00 2001 From: Alex Velikanov Date: Wed, 25 Jun 2025 16:03:27 -0400 Subject: [PATCH 2/3] update changelog --- experimental/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 6216c63e1c3..b79c0b6f332 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -20,6 +20,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 * user-facing: `LogRecord` class is now not exported anymore. A newly exported interface `SdkLogRecord` is used in its place. * refactor: Removed `api-events` and `sdk-events` [#5737](https://github.com/open-telemetry/opentelemetry-js/pull/5737) @svetlanabrennan * chore: Regenerated certs [#5752](https://github.com/open-telemetry/opentelemetry-js/pull/5752) @svetlanabrennan +* fix(api-logs,sdk-logs): allow AnyValue attributes for logs and handle circular references [#5765](https://github.com/open-telemetry/opentelemetry-js/pull/5765) @alec2435 ## 0.202.0 From 5f795934e2decbdbc746344cc977d1a961889159 Mon Sep 17 00:00:00 2001 From: Alex Velikanov Date: Thu, 3 Jul 2025 15:30:07 -0400 Subject: [PATCH 3/3] Additional test coverage --- .../test/common/utils/validation.test.ts | 48 +++++++++++++++ .../sdk-logs/test/common/LogRecord.test.ts | 60 +++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/experimental/packages/api-logs/test/common/utils/validation.test.ts b/experimental/packages/api-logs/test/common/utils/validation.test.ts index 4bf3a911e68..d4982a83632 100644 --- a/experimental/packages/api-logs/test/common/utils/validation.test.ts +++ b/experimental/packages/api-logs/test/common/utils/validation.test.ts @@ -128,6 +128,30 @@ describe('isLogAttributeValue', () => { it('should accept objects with byte arrays', () => { assert.strictEqual(isLogAttributeValue({ bytes: new Uint8Array([1, 2, 3]) }), true); }); + + it('should accept plain objects without prototypes', () => { + const objWithoutProto = Object.create(null); + objWithoutProto.key = 'value'; + objWithoutProto.number = 42; + assert.strictEqual(isLogAttributeValue(objWithoutProto), true); + }); + + it('should accept nested objects without prototypes', () => { + const parent = Object.create(null); + const child = Object.create(null); + child.deep = 'value'; + parent.nested = child; + parent.regular = { withProto: true }; + assert.strictEqual(isLogAttributeValue(parent), true); + }); + + it('should accept objects without prototypes containing arrays', () => { + const obj = Object.create(null); + obj.strings = ['a', 'b', 'c']; + obj.mixed = [1, 'two', true]; + obj.bytes = new Uint8Array([1, 2, 3]); + assert.strictEqual(isLogAttributeValue(obj), true); + }); }); describe('should accept complex combinations', () => { @@ -193,16 +217,40 @@ describe('isLogAttributeValue', () => { assert.strictEqual(isLogAttributeValue(new TestClass()), false); }); + it('should reject Map objects', () => { + assert.strictEqual(isLogAttributeValue(new Map()), false); + assert.strictEqual(isLogAttributeValue(new Map([['key', 'value']])), false); + + const nestedMap = new Map(); + nestedMap.set('nested', new Map([['inner', 'value']])); + assert.strictEqual(isLogAttributeValue(nestedMap), false); + }); + + it('should reject Set objects', () => { + assert.strictEqual(isLogAttributeValue(new Set()), false); + assert.strictEqual(isLogAttributeValue(new Set([1, 2, 3])), false); + assert.strictEqual(isLogAttributeValue(new Set(['a', 'b', 'c'])), false); + }); + + it('should reject WeakMap and WeakSet objects', () => { + assert.strictEqual(isLogAttributeValue(new WeakMap()), false); + assert.strictEqual(isLogAttributeValue(new WeakSet()), false); + }); + it('should reject arrays containing invalid values', () => { assert.strictEqual(isLogAttributeValue(['valid', () => {}]), false); assert.strictEqual(isLogAttributeValue([Symbol('test'), 'valid']), false); assert.strictEqual(isLogAttributeValue([new Date()]), false); + assert.strictEqual(isLogAttributeValue([new Map()]), false); + assert.strictEqual(isLogAttributeValue(['valid', new Set([1, 2, 3])]), false); }); it('should reject objects containing invalid values', () => { assert.strictEqual(isLogAttributeValue({ valid: 'test', invalid: () => {} }), false); assert.strictEqual(isLogAttributeValue({ symbol: Symbol('test') }), false); assert.strictEqual(isLogAttributeValue({ date: new Date() }), false); + assert.strictEqual(isLogAttributeValue({ map: new Map([['key', 'value']]) }), false); + assert.strictEqual(isLogAttributeValue({ set: new Set([1, 2, 3]) }), false); }); it('should reject deeply nested invalid values', () => { diff --git a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts index 823d1bc043d..62dc90d8c8c 100644 --- a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts +++ b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts @@ -617,6 +617,66 @@ describe('LogRecord', () => { }); }); + describe('should reject empty attribute keys', () => { + it('should not set attributes with empty string keys', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + + // Try to set an attribute with an empty key + logRecord.setAttribute('', 'value'); + + // The attribute should not be set + assert.strictEqual(logRecord.attributes[''], undefined); + assert.deepStrictEqual(logRecord.attributes, {}); + + // A warning should be logged + sinon.assert.calledWith(warnStub, 'Invalid attribute key: '); + warnStub.restore(); + }); + + it('should reject empty keys but accept other valid attributes', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + + // Set some valid attributes + logRecord.setAttribute('valid1', 'value1'); + logRecord.setAttribute('', 'should not be set'); + logRecord.setAttribute('valid2', 'value2'); + + // Only valid attributes should be set + assert.deepStrictEqual(logRecord.attributes, { + valid1: 'value1', + valid2: 'value2' + }); + + // Warning should be logged for empty key + sinon.assert.calledWith(warnStub, 'Invalid attribute key: '); + warnStub.restore(); + }); + + it('should reject empty keys in setAttributes', () => { + const warnStub = sinon.spy(diag, 'warn'); + const { logRecord } = setup(); + + // Try to set multiple attributes including empty keys + logRecord.setAttributes({ + valid: 'value', + '': 'empty key', + anotherValid: 'another value' + }); + + // Only valid attributes should be set + assert.deepStrictEqual(logRecord.attributes, { + valid: 'value', + anotherValid: 'another value' + }); + + // Warning should be logged for empty key + sinon.assert.calledWith(warnStub, 'Invalid attribute key: '); + warnStub.restore(); + }); + }); + describe('log record processor', () => { it('should call onEmit synchronously when log record is emitted', () => { let emitted = false;