diff --git a/packages/core/src/attributes.ts b/packages/core/src/attributes.ts index f33241be2a1e..011367d5bc26 100644 --- a/packages/core/src/attributes.ts +++ b/packages/core/src/attributes.ts @@ -84,7 +84,7 @@ export function attributeValueToTypedAttributeValue( return { ...attributeValue, ...checkedUnit }; } - if (!useFallback) { + if (!useFallback || value === undefined) { return; } @@ -113,9 +113,12 @@ export function attributeValueToTypedAttributeValue( * * @returns The serialized attributes. */ -export function serializeAttributes(attributes: RawAttributes, fallback: boolean = false): Attributes { +export function serializeAttributes( + attributes: RawAttributes | undefined, + fallback: boolean = false, +): Attributes { const serializedAttributes: Attributes = {}; - for (const [key, value] of Object.entries(attributes)) { + for (const [key, value] of Object.entries(attributes ?? {})) { const typedValue = attributeValueToTypedAttributeValue(value, fallback); if (typedValue) { serializedAttributes[key] = typedValue; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e18ea294f182..3108e98f6de7 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -449,6 +449,7 @@ export type { MetricType, SerializedMetric, SerializedMetricContainer, + // eslint-disable-next-line deprecation/deprecation SerializedMetricAttributeValue, } from './types-hoist/metric'; export type { TimedEvent } from './types-hoist/timedEvent'; diff --git a/packages/core/src/metrics/internal.ts b/packages/core/src/metrics/internal.ts index 7ac1372d1285..cbc92413c7de 100644 --- a/packages/core/src/metrics/internal.ts +++ b/packages/core/src/metrics/internal.ts @@ -1,10 +1,11 @@ +import { serializeAttributes } from '../attributes'; import { getGlobalSingleton } from '../carrier'; import type { Client } from '../client'; import { getClient, getCurrentScope, getGlobalScope, getIsolationScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; import type { Scope, ScopeData } from '../scope'; import type { Integration } from '../types-hoist/integration'; -import type { Metric, SerializedMetric, SerializedMetricAttributeValue } from '../types-hoist/metric'; +import type { Metric, SerializedMetric } from '../types-hoist/metric'; import { mergeScopeData } from '../utils/applyScopeDataToEvent'; import { debug } from '../utils/debug-logger'; import { _getSpanForScope } from '../utils/spanOnScope'; @@ -14,50 +15,6 @@ import { createMetricEnvelope } from './envelope'; const MAX_METRIC_BUFFER_SIZE = 1000; -/** - * Converts a metric attribute to a serialized metric attribute. - * - * @param value - The value of the metric attribute. - * @returns The serialized metric attribute. - */ -export function metricAttributeToSerializedMetricAttribute(value: unknown): SerializedMetricAttributeValue { - switch (typeof value) { - case 'number': - if (Number.isInteger(value)) { - return { - value, - type: 'integer', - }; - } - return { - value, - type: 'double', - }; - case 'boolean': - return { - value, - type: 'boolean', - }; - case 'string': - return { - value, - type: 'string', - }; - default: { - let stringValue = ''; - try { - stringValue = JSON.stringify(value) ?? ''; - } catch { - // Do nothing - } - return { - value: stringValue, - type: 'string', - }; - } - } -} - /** * Sets a metric attribute if the value exists and the attribute key is not already present. * @@ -169,14 +126,6 @@ function _enrichMetricAttributes(beforeMetric: Metric, client: Client, currentSc * Creates a serialized metric ready to be sent to Sentry. */ function _buildSerializedMetric(metric: Metric, client: Client, currentScope: Scope): SerializedMetric { - // Serialize attributes - const serializedAttributes: Record = {}; - for (const key in metric.attributes) { - if (metric.attributes[key] !== undefined) { - serializedAttributes[key] = metricAttributeToSerializedMetricAttribute(metric.attributes[key]); - } - } - // Get trace context const [, traceContext] = _getTraceInfoFromScope(client, currentScope); const span = _getSpanForScope(currentScope); @@ -191,7 +140,7 @@ function _buildSerializedMetric(metric: Metric, client: Client, currentScope: Sc type: metric.type, unit: metric.unit, value: metric.value, - attributes: serializedAttributes, + attributes: serializeAttributes(metric.attributes, true), }; } diff --git a/packages/core/src/types-hoist/metric.ts b/packages/core/src/types-hoist/metric.ts index 6ac63da6032b..976fc9fe863f 100644 --- a/packages/core/src/types-hoist/metric.ts +++ b/packages/core/src/types-hoist/metric.ts @@ -1,3 +1,5 @@ +import type { Attributes, TypedAttributeValue } from '../attributes'; + export type MetricType = 'counter' | 'gauge' | 'distribution'; export interface Metric { @@ -27,11 +29,10 @@ export interface Metric { attributes?: Record; } -export type SerializedMetricAttributeValue = - | { value: string; type: 'string' } - | { value: number; type: 'integer' } - | { value: number; type: 'double' } - | { value: boolean; type: 'boolean' }; +/** + * @deprecated this was not intended for public consumption + */ +export type SerializedMetricAttributeValue = TypedAttributeValue; export interface SerializedMetric { /** @@ -72,7 +73,7 @@ export interface SerializedMetric { /** * Arbitrary structured data that stores information about the metric. */ - attributes?: Record; + attributes?: Attributes; } export type SerializedMetricContainer = { diff --git a/packages/core/test/lib/attributes.test.ts b/packages/core/test/lib/attributes.test.ts index 9d9b2d5c1e9a..bd87e3be1f3c 100644 --- a/packages/core/test/lib/attributes.test.ts +++ b/packages/core/test/lib/attributes.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { attributeValueToTypedAttributeValue, isAttributeObject } from '../../src/attributes'; +import { attributeValueToTypedAttributeValue, isAttributeObject, serializeAttributes } from '../../src/attributes'; describe('attributeValueToTypedAttributeValue', () => { describe('without fallback (default behavior)', () => { @@ -267,6 +267,19 @@ describe('attributeValueToTypedAttributeValue', () => { type: 'string', }); }); + + it.each([null, { value: null }, { value: null, unit: 'byte' }])('stringifies %s values', value => { + const result = attributeValueToTypedAttributeValue(value, true); + expect(result).toMatchObject({ + value: 'null', + type: 'string', + }); + }); + + it.each([undefined, { value: undefined }, { value: undefined, unit: 'byte' }])('ignores %s values', value => { + const result = attributeValueToTypedAttributeValue(value, true); + expect(result).toBeUndefined(); + }); }); }); }); @@ -297,3 +310,91 @@ describe('isAttributeObject', () => { }, ); }); + +describe('serializeAttributes', () => { + it('returns an empty object for undefined attributes', () => { + const result = serializeAttributes(undefined); + expect(result).toStrictEqual({}); + }); + + it('returns an empty object for an empty object', () => { + const result = serializeAttributes({}); + expect(result).toStrictEqual({}); + }); + + it('serializes valid, non-primitive values', () => { + const result = serializeAttributes({ foo: 'bar', bar: { value: 123 }, baz: { value: 456, unit: 'byte' } }); + expect(result).toStrictEqual({ + bar: { + type: 'integer', + value: 123, + }, + baz: { + type: 'integer', + unit: 'byte', + value: 456, + }, + foo: { + type: 'string', + value: 'bar', + }, + }); + }); + + it.each([true, false])('ignores undefined values if fallback is %s', fallback => { + const result = serializeAttributes( + { foo: undefined, bar: { value: undefined }, baz: { value: undefined, unit: 'byte' } }, + fallback, + ); + expect(result).toStrictEqual({}); + }); + + it('ignores null values by default', () => { + const result = serializeAttributes({ foo: null, bar: { value: null }, baz: { value: null, unit: 'byte' } }); + expect(result).toStrictEqual({}); + }); + + it('stringifies to `"null"` if fallback is true', () => { + const result = serializeAttributes({ foo: null, bar: { value: null }, baz: { value: null, unit: 'byte' } }, true); + expect(result).toStrictEqual({ + foo: { + type: 'string', + value: 'null', + }, + bar: { + type: 'string', + value: 'null', + }, + baz: { + type: 'string', + unit: 'byte', + value: 'null', + }, + }); + }); + + describe('invalid (non-primitive) values', () => { + it("doesn't fall back to stringification by default", () => { + const result = serializeAttributes({ foo: { some: 'object' }, bar: [1, 2, 3], baz: () => {} }); + expect(result).toStrictEqual({}); + }); + + it('falls back to stringification of unsupported non-primitive values if fallback is true', () => { + const result = serializeAttributes({ foo: { some: 'object' }, bar: [1, 2, 3], baz: () => {} }, true); + expect(result).toStrictEqual({ + bar: { + type: 'string', + value: '[1,2,3]', + }, + baz: { + type: 'string', + value: '', + }, + foo: { + type: 'string', + value: '{"some":"object"}', + }, + }); + }); + }); +}); diff --git a/packages/core/test/lib/metrics/internal.test.ts b/packages/core/test/lib/metrics/internal.test.ts index 3e479e282a0c..55753082d7ff 100644 --- a/packages/core/test/lib/metrics/internal.test.ts +++ b/packages/core/test/lib/metrics/internal.test.ts @@ -4,7 +4,6 @@ import { _INTERNAL_captureMetric, _INTERNAL_flushMetricsBuffer, _INTERNAL_getMetricBuffer, - metricAttributeToSerializedMetricAttribute, } from '../../../src/metrics/internal'; import type { Metric } from '../../../src/types-hoist/metric'; import * as loggerModule from '../../../src/utils/debug-logger'; @@ -12,74 +11,6 @@ import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; const PUBLIC_DSN = 'https://username@domain/123'; -describe('metricAttributeToSerializedMetricAttribute', () => { - it('serializes integer values', () => { - const result = metricAttributeToSerializedMetricAttribute(42); - expect(result).toEqual({ - value: 42, - type: 'integer', - }); - }); - - it('serializes double values', () => { - const result = metricAttributeToSerializedMetricAttribute(42.34); - expect(result).toEqual({ - value: 42.34, - type: 'double', - }); - }); - - it('serializes boolean values', () => { - const result = metricAttributeToSerializedMetricAttribute(true); - expect(result).toEqual({ - value: true, - type: 'boolean', - }); - }); - - it('serializes string values', () => { - const result = metricAttributeToSerializedMetricAttribute('endpoint'); - expect(result).toEqual({ - value: 'endpoint', - type: 'string', - }); - }); - - it('serializes object values as JSON strings', () => { - const obj = { name: 'John', age: 30 }; - const result = metricAttributeToSerializedMetricAttribute(obj); - expect(result).toEqual({ - value: JSON.stringify(obj), - type: 'string', - }); - }); - - it('serializes array values as JSON strings', () => { - const array = [1, 2, 3, 'test']; - const result = metricAttributeToSerializedMetricAttribute(array); - expect(result).toEqual({ - value: JSON.stringify(array), - type: 'string', - }); - }); - - it('serializes undefined values as empty strings', () => { - const result = metricAttributeToSerializedMetricAttribute(undefined); - expect(result).toEqual({ - value: '', - type: 'string', - }); - }); - - it('serializes null values as JSON strings', () => { - const result = metricAttributeToSerializedMetricAttribute(null); - expect(result).toEqual({ - value: 'null', - type: 'string', - }); - }); -}); - describe('_INTERNAL_captureMetric', () => { it('captures and sends metrics', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });