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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/core/src/attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function isAttributeObject(maybeObj: unknown): maybeObj is AttributeObjec
*/
export function attributeValueToTypedAttributeValue(
rawValue: unknown,
useFallback?: boolean,
useFallback?: boolean | 'skip-undefined',
): TypedAttributeValue | void {
const { value, unit } = isAttributeObject(rawValue) ? rawValue : { value: rawValue, unit: undefined };
const attributeValue = getTypedAttributeValue(value);
Expand All @@ -84,7 +84,7 @@ export function attributeValueToTypedAttributeValue(
return { ...attributeValue, ...checkedUnit };
}

if (!useFallback) {
if (!useFallback || (useFallback === 'skip-undefined' && value === undefined)) {
return;
}
Comment on lines +87 to 89
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a fun one: metrics and logs attribute setting logic already diverged prior to this PR: undefined values were

  • stringified to '' in logs
  • ignored (not sent) in metrics

I therefore had to make the fallback behavior a tri-state flag. true ignores non-conforming values, 'skip-undefined also ignores undefined in addition, false completely ignores non-conforming values.


Expand Down Expand Up @@ -113,9 +113,12 @@ export function attributeValueToTypedAttributeValue(
*
* @returns The serialized attributes.
*/
export function serializeAttributes<T>(attributes: RawAttributes<T>, fallback: boolean = false): Attributes {
export function serializeAttributes<T>(
attributes: RawAttributes<T> | undefined,
fallback: boolean | 'skip-undefined' = 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;
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
57 changes: 3 additions & 54 deletions packages/core/src/metrics/internal.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { serializeAttributes } from '../attributes';
import { getGlobalSingleton } from '../carrier';
import type { Client } from '../client';
import { getClient, getCurrentScope, getIsolationScope } from '../currentScopes';
import { DEBUG_BUILD } from '../debug-build';
import type { Scope } 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 { debug } from '../utils/debug-logger';
import { getCombinedScopeData } from '../utils/scopeData';
import { _getSpanForScope } from '../utils/spanOnScope';
Expand All @@ -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.
*
Expand Down Expand Up @@ -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<string, SerializedMetricAttributeValue> = {};
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);
Expand All @@ -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, 'skip-undefined'),
};
}

Expand Down
13 changes: 7 additions & 6 deletions packages/core/src/types-hoist/metric.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Attributes, TypedAttributeValue } from '../attributes';

export type MetricType = 'counter' | 'gauge' | 'distribution';

export interface Metric {
Expand Down Expand Up @@ -27,11 +29,10 @@ export interface Metric {
attributes?: Record<string, unknown>;
}

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 {
/**
Expand Down Expand Up @@ -72,7 +73,7 @@ export interface SerializedMetric {
/**
* Arbitrary structured data that stores information about the metric.
*/
attributes?: Record<string, SerializedMetricAttributeValue>;
attributes?: Attributes;
}

export type SerializedMetricContainer = {
Expand Down
152 changes: 151 additions & 1 deletion packages/core/test/lib/attributes.test.ts
Original file line number Diff line number Diff line change
@@ -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)', () => {
Expand Down Expand Up @@ -267,8 +267,43 @@ 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 }])('stringifies %s values to ""', value => {
const result = attributeValueToTypedAttributeValue(value, true);
expect(result).toEqual({
value: '',
type: 'string',
});
});

it('stringifies undefined values with unit to ""', () => {
const result = attributeValueToTypedAttributeValue({ value: undefined, unit: 'byte' }, true);
expect(result).toEqual({
value: '',
unit: 'byte',
type: 'string',
});
});
});
});

describe('with fallback="skip-undefined"', () => {
it.each([undefined, { value: undefined }, { value: undefined, unit: 'byte' }])(
'ignores undefined values (%s)',
value => {
const result = attributeValueToTypedAttributeValue(value, 'skip-undefined');
expect(result).toBeUndefined();
},
);
});
});

describe('isAttributeObject', () => {
Expand Down Expand Up @@ -297,3 +332,118 @@ 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('ignores undefined values if fallback is false', () => {
const result = serializeAttributes(
{ foo: undefined, bar: { value: undefined }, baz: { value: undefined, unit: 'byte' } },
false,
);
expect(result).toStrictEqual({});
});

it('ignores undefined values if fallback is "skip-undefined"', () => {
const result = serializeAttributes(
{ foo: undefined, bar: { value: undefined }, baz: { value: undefined, unit: 'byte' } },
'skip-undefined',
);
expect(result).toStrictEqual({});
});

it('stringifies undefined values to "" if fallback is true', () => {
const result = serializeAttributes(
{ foo: undefined, bar: { value: undefined }, baz: { value: undefined, unit: 'byte' } },
true,
);
expect(result).toStrictEqual({
bar: {
type: 'string',
value: '',
},
baz: {
type: 'string',
unit: 'byte',
value: '',
},
foo: { type: 'string', value: '' },
});
});

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"}',
},
});
});
});
});
Loading
Loading