Skip to content

Commit 0ccf278

Browse files
Lms24Lms24
committed
only serilialize primitive scope attributes
1 parent ee851a4 commit 0ccf278

File tree

4 files changed

+59
-242
lines changed

4 files changed

+59
-242
lines changed

‎packages/core/src/attributes.ts‎

Lines changed: 28 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
import { DEBUG_BUILD } from './debug-build';
21
import type { DurationUnit, FractionUnit, InformationUnit } from './types-hoist/measurement';
3-
import { debug } from './utils/debug-logger';
42

53
export type RawAttributes<T> = T & ValidatedAttributes<T>;
64
// eslint-disable-next-line @typescript-eslint/no-explicit-any
75
export type RawAttribute<T> = T extends { value: any } | { unit: any } ? AttributeObject : T;
86

9-
export type TypedAttributes = Record<string, TypedAttributeValue>;
7+
export type Attributes = Record<string, TypedAttributeValue>;
108

119
export type AttributeValueType = string | number | boolean | Array<string> | Array<number> | Array<boolean>;
1210

@@ -72,27 +70,36 @@ export function isAttributeObject(maybeObj: unknown): maybeObj is AttributeObjec
7270
* @param value - The value of the passed attribute.
7371
* @returns The typed attribute.
7472
*/
75-
export function attributeValueToTypedAttributeValue(rawValue: unknown): TypedAttributeValue {
73+
export function attributeValueToTypedAttributeValue(rawValue: unknown): TypedAttributeValue | void {
7674
const { value, unit } = isAttributeObject(rawValue) ? rawValue : { value: rawValue, unit: undefined };
77-
return { ...getTypedAttributeValue(value), ...(unit && typeof unit === 'string' ? { unit } : {}) };
75+
const attributeValue = getTypedAttributeValue(value);
76+
if (attributeValue) {
77+
return { ...attributeValue, ...(unit && typeof unit === 'string' ? { unit } : {}) };
78+
}
7879
}
7980

80-
// Only allow string, boolean, or number types
81-
const getPrimitiveType: (
82-
item: unknown,
83-
) => keyof Pick<AttributeTypeMap, 'string' | 'integer' | 'double' | 'boolean'> | null = item =>
84-
typeof item === 'string'
85-
? 'string'
86-
: typeof item === 'boolean'
87-
? 'boolean'
88-
: typeof item === 'number' && !Number.isNaN(item)
89-
? Number.isInteger(item)
90-
? 'integer'
91-
: 'double'
92-
: null;
93-
94-
function getTypedAttributeValue(value: unknown): TypedAttributeValue {
95-
const primitiveType = getPrimitiveType(value);
81+
/**
82+
* NOTE: We intentionally do not return anything for non-primitive values:
83+
* - array support will come in the future but if we stringify arrays now,
84+
* sending arrays (unstringified) later will be a subtle breaking change.
85+
* - Objects are not supported yet and product support is still TBD.
86+
* - We still keep the type signature for TypedAttributeValue wider to avoid a
87+
* breaking change once we add support for non-primitive values.
88+
* - Once we go back to supporting arrays and stringifying all other values,
89+
* we already implemented the serialization logic here:
90+
* https://github.com/getsentry/sentry-javascript/pull/18165
91+
*/
92+
function getTypedAttributeValue(value: unknown): TypedAttributeValue | void {
93+
const primitiveType =
94+
typeof value === 'string'
95+
? 'string'
96+
: typeof value === 'boolean'
97+
? 'boolean'
98+
: typeof value === 'number' && !Number.isNaN(value)
99+
? Number.isInteger(value)
100+
? 'integer'
101+
: 'double'
102+
: null;
96103
if (primitiveType) {
97104
// @ts-expect-error - TS complains because {@link TypedAttributeValue} is strictly typed to
98105
// avoid setting the wrong `type` on the attribute value.
@@ -102,40 +109,4 @@ function getTypedAttributeValue(value: unknown): TypedAttributeValue {
102109
// Therefore, we ignore it.
103110
return { value, type: primitiveType };
104111
}
105-
106-
if (Array.isArray(value)) {
107-
const coherentArrayType = value.reduce((acc: 'string' | 'boolean' | 'integer' | 'double' | null, item) => {
108-
if (!acc || getPrimitiveType(item) !== acc) {
109-
return null;
110-
}
111-
return acc;
112-
}, getPrimitiveType(value[0]));
113-
114-
if (coherentArrayType) {
115-
return { value, type: `${coherentArrayType}[]` };
116-
}
117-
}
118-
119-
// Fallback: stringify the passed value
120-
let fallbackValue = '';
121-
try {
122-
fallbackValue = JSON.stringify(value) ?? String(value);
123-
} catch {
124-
try {
125-
fallbackValue = String(value);
126-
} catch {
127-
DEBUG_BUILD && debug.warn('Failed to stringify attribute value', value);
128-
// ignore
129-
}
130-
}
131-
132-
// This is quite a low-quality message but we cannot safely log the original `value`
133-
// here due to String() or JSON.stringify() potentially throwing.
134-
DEBUG_BUILD &&
135-
debug.log(`Stringified attribute value to ${fallbackValue} because it's not a supported attribute value type`);
136-
137-
return {
138-
value: fallbackValue,
139-
type: 'string',
140-
};
141112
}

‎packages/core/src/logs/internal.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,10 @@ export function _INTERNAL_captureLog(
163163
severity_number: severityNumber ?? SEVERITY_TEXT_TO_SEVERITY_NUMBER[level],
164164
attributes: Object.keys(attributes).reduce(
165165
(acc, key) => {
166-
acc[key] = attributeValueToTypedAttributeValue(attributes[key]);
166+
const typedAttributeValue = attributeValueToTypedAttributeValue(attributes[key]);
167+
if (typedAttributeValue) {
168+
acc[key] = typedAttributeValue;
169+
}
167170
return acc;
168171
},
169172
{} as Record<string, TypedAttributeValue>,

‎packages/core/test/lib/attributes.test.ts‎

Lines changed: 22 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -37,41 +37,21 @@ describe('attributeValueToTypedAttributeValue', () => {
3737
});
3838

3939
describe('arrays', () => {
40-
it('converts an array of strings to a typed attribute value', () => {
41-
const result = attributeValueToTypedAttributeValue(['foo', 'bar']);
42-
expect(result).toStrictEqual({
43-
value: ['foo', 'bar'],
44-
type: 'string[]',
45-
});
46-
});
47-
48-
it('converts an array of integer numbers to a typed attribute value', () => {
49-
const result = attributeValueToTypedAttributeValue([1, 2, 3]);
50-
expect(result).toStrictEqual({
51-
value: [1, 2, 3],
52-
type: 'integer[]',
53-
});
54-
});
55-
56-
it('converts an array of double numbers to a typed attribute value', () => {
57-
const result = attributeValueToTypedAttributeValue([1.1, 2.2, 3.3]);
58-
expect(result).toStrictEqual({
59-
value: [1.1, 2.2, 3.3],
60-
type: 'double[]',
61-
});
62-
});
63-
64-
it('converts an array of booleans to a typed attribute value', () => {
65-
const result = attributeValueToTypedAttributeValue([true, false, true]);
66-
expect(result).toStrictEqual({
67-
value: [true, false, true],
68-
type: 'boolean[]',
69-
});
40+
it.each([
41+
['foo', 'bar'],
42+
[1, 2, 3],
43+
[true, false, true],
44+
[1, 'foo', true],
45+
{ foo: 'bar' },
46+
() => 'test',
47+
Symbol('test'),
48+
])('returns undefined for none-primitive values (%s)', value => {
49+
const result = attributeValueToTypedAttributeValue(value);
50+
expect(result).toBeUndefined();
7051
});
7152
});
7253

7354
describe('attribute objects without units', () => {
74-
// Note: These tests only test exemplar type and fallback behaviour (see above for more cases)
7555
it('converts a primitive value to a typed attribute value', () => {
7656
const result = attributeValueToTypedAttributeValue({ value: 123.45 });
7757
expect(result).toStrictEqual({
@@ -80,20 +60,17 @@ describe('attributeValueToTypedAttributeValue', () => {
8060
});
8161
});
8262

83-
it('converts an array of primitive values to a typed attribute value', () => {
84-
const result = attributeValueToTypedAttributeValue({ value: [true, false] });
85-
expect(result).toStrictEqual({
86-
value: [true, false],
87-
type: 'boolean[]',
88-
});
89-
});
90-
91-
it('converts an unsupported object value to a string attribute value', () => {
92-
const result = attributeValueToTypedAttributeValue({ value: { foo: 'bar' } });
93-
expect(result).toStrictEqual({
94-
value: '{"foo":"bar"}',
95-
type: 'string',
96-
});
63+
it.each([
64+
['foo', 'bar'],
65+
[1, 2, 3],
66+
[true, false, true],
67+
[1, 'foo', true],
68+
{ foo: 'bar' },
69+
() => 'test',
70+
Symbol('test'),
71+
])('returns undefined for none-primitive values (%s)', value => {
72+
const result = attributeValueToTypedAttributeValue({ value });
73+
expect(result).toBeUndefined();
9774
});
9875
});
9976

@@ -108,24 +85,6 @@ describe('attributeValueToTypedAttributeValue', () => {
10885
});
10986
});
11087

111-
it('converts an array of primitive values to a typed attribute value', () => {
112-
const result = attributeValueToTypedAttributeValue({ value: [true, false], unit: 'count' });
113-
expect(result).toStrictEqual({
114-
value: [true, false],
115-
type: 'boolean[]',
116-
unit: 'count',
117-
});
118-
});
119-
120-
it('converts an unsupported object value to a string attribute value', () => {
121-
const result = attributeValueToTypedAttributeValue({ value: { foo: 'bar' }, unit: 'bytes' });
122-
expect(result).toStrictEqual({
123-
value: '{"foo":"bar"}',
124-
type: 'string',
125-
unit: 'bytes',
126-
});
127-
});
128-
12988
it('extracts the value property of an object with a value property', () => {
13089
// and ignores other properties.
13190
// For now we're fine with this but we may reconsider in the future.
@@ -138,114 +97,6 @@ describe('attributeValueToTypedAttributeValue', () => {
13897
});
13998
});
14099

141-
describe('unsupported value types', () => {
142-
it('stringifies mixed float and integer numbers to a string attribute value', () => {
143-
const result = attributeValueToTypedAttributeValue([1, 2.2, 3]);
144-
expect(result).toStrictEqual({
145-
value: '[1,2.2,3]',
146-
type: 'string',
147-
});
148-
});
149-
150-
it('stringifies an array of allowed but incoherent types to a string attribute value', () => {
151-
const result = attributeValueToTypedAttributeValue([1, 'foo', true]);
152-
expect(result).toStrictEqual({
153-
value: '[1,"foo",true]',
154-
type: 'string',
155-
});
156-
});
157-
158-
it('stringifies an array of disallowed and incoherent types to a string attribute value', () => {
159-
const result = attributeValueToTypedAttributeValue([null, undefined, NaN]);
160-
expect(result).toStrictEqual({
161-
value: '[null,null,null]',
162-
type: 'string',
163-
});
164-
});
165-
166-
it('stringifies an object value to a string attribute value', () => {
167-
const result = attributeValueToTypedAttributeValue({ foo: 'bar' });
168-
expect(result).toStrictEqual({
169-
value: '{"foo":"bar"}',
170-
type: 'string',
171-
});
172-
});
173-
174-
it('stringifies a null value to a string attribute value', () => {
175-
const result = attributeValueToTypedAttributeValue(null);
176-
expect(result).toStrictEqual({
177-
value: 'null',
178-
type: 'string',
179-
});
180-
});
181-
182-
it('stringifies an undefined value to a string attribute value', () => {
183-
const result = attributeValueToTypedAttributeValue(undefined);
184-
expect(result).toStrictEqual({
185-
value: 'undefined',
186-
type: 'string',
187-
});
188-
});
189-
190-
it('stringifies an NaN number value to a string attribute value', () => {
191-
const result = attributeValueToTypedAttributeValue(NaN);
192-
expect(result).toStrictEqual({
193-
value: 'null',
194-
type: 'string',
195-
});
196-
});
197-
198-
it('converts an object toString if stringification fails', () => {
199-
const result = attributeValueToTypedAttributeValue({
200-
value: {
201-
toJson: () => {
202-
throw new Error('test');
203-
},
204-
},
205-
});
206-
expect(result).toStrictEqual({
207-
value: '{}',
208-
type: 'string',
209-
});
210-
});
211-
212-
it('falls back to an empty string if stringification and toString fails', () => {
213-
const result = attributeValueToTypedAttributeValue({
214-
value: {
215-
toJSON: () => {
216-
throw new Error('test');
217-
},
218-
toString: () => {
219-
throw new Error('test');
220-
},
221-
},
222-
});
223-
expect(result).toStrictEqual({
224-
value: '',
225-
type: 'string',
226-
});
227-
});
228-
229-
it('converts a function toString ', () => {
230-
const result = attributeValueToTypedAttributeValue(() => {
231-
return 'test';
232-
});
233-
234-
expect(result).toStrictEqual({
235-
value: '() => {\n return "test";\n }',
236-
type: 'string',
237-
});
238-
});
239-
240-
it('converts a symbol toString', () => {
241-
const result = attributeValueToTypedAttributeValue(Symbol('test'));
242-
expect(result).toStrictEqual({
243-
value: 'Symbol(test)',
244-
type: 'string',
245-
});
246-
});
247-
});
248-
249100
it.each([1, true, null, undefined, NaN, Symbol('test'), { foo: 'bar' }])(
250101
'ignores invalid (non-string) units (%s)',
251102
unit => {

‎packages/core/test/lib/logs/internal.test.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,12 @@ describe('_INTERNAL_captureLog', () => {
178178
scope.setClient(client);
179179

180180
scope.setAttribute('scope_1', 'attribute_value');
181-
scope.setAttribute('scope_2', { value: 38, unit: 'gigabytes' });
181+
scope.setAttribute('scope_2', { value: 38, unit: 'gigabyte' });
182182
scope.setAttributes({
183183
scope_3: true,
184+
// these are invalid since for now we don't support arrays
184185
scope_4: [1, 2, 3],
185-
scope_5: { value: [true, false, true], unit: 's' },
186+
scope_5: { value: [true, false, true], unit: 'second' },
186187
});
187188

188189
_INTERNAL_captureLog(
@@ -196,7 +197,7 @@ describe('_INTERNAL_captureLog', () => {
196197

197198
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes;
198199

199-
expect(logAttributes).toEqual({
200+
expect(logAttributes).toStrictEqual({
200201
userId: {
201202
value: '123',
202203
type: 'string',
@@ -211,22 +212,13 @@ describe('_INTERNAL_captureLog', () => {
211212
},
212213
scope_2: {
213214
type: 'integer',
214-
unit: 'gigabytes',
215+
unit: 'gigabyte',
215216
value: 38,
216217
},
217218
scope_3: {
218219
type: 'boolean',
219220
value: true,
220221
},
221-
scope_4: {
222-
type: 'integer[]',
223-
value: [1, 2, 3],
224-
},
225-
scope_5: {
226-
type: 'boolean[]',
227-
value: [true, false, true],
228-
unit: 's',
229-
},
230222
});
231223
});
232224
});

0 commit comments

Comments
 (0)