attribute objects w/o type, keep attributes as-is on scope

Lms24 · Lms24 · commit 4707fe78248e · 2025-11-18T17:51:03.000+01:00
diff --git a/packages/core/src/attributes.ts b/packages/core/src/attributes.ts
@@ -1,3 +1,7 @@
+export type RawAttributes<T> = T & ValidatedAttributes<T>;
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export type RawAttribute<T> = T extends { value: any } | { unit: any } ? AttributeWithUnit : T;
+
 export type Attributes = Record<string, TypedAttributeValue>;
 
 export type AttributeValueType = string | number | boolean | Array<string> | Array<number> | Array<boolean>;
@@ -27,38 +31,33 @@ type AttributeUnion = {
 
 export type TypedAttributeValue = AttributeUnion & { unit?: Units };
 
-type AttributeWithUnit = {
+export type AttributeWithUnit = {
   value: unknown;
-  unit: Units;
+  unit?: Units;
 };
 
+/**
+ * Unit of measurement that can be added to an attribute.
+ */
 type Units = 'ms' | 's' | 'bytes' | 'count' | 'percent';
 
-type ValidAttributeObject = AttributeWithUnit | TypedAttributeValue;
-
 /* If an attribute has either a 'value' or 'unit' property, we use the ValidAttributeObject type. */
 export type ValidatedAttributes<T> = {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  [K in keyof T]: T[K] extends { value: any } | { unit: any } ? ValidAttributeObject : unknown;
+  [K in keyof T]: T[K] extends { value: any } | { unit: any } ? AttributeWithUnit : unknown;
 };
 
 /**
  * Type-guard: The attribute object has the shape the official attribute object (value, type, unit).
  * https://develop.sentry.dev/sdk/telemetry/scopes/#setting-attributes
  */
-export function isAttributeObject(value: unknown): value is ValidAttributeObject {
-  if (typeof value !== 'object' || value === null || Array.isArray(value)) {
-    return false;
-  }
-  // MUST have a 'value' property
-  if (!Object.prototype.hasOwnProperty.call(value, 'value')) {
+export function isAttributeObject(value: unknown): value is AttributeWithUnit {
+  if (typeof value !== 'object' || value == null || Array.isArray(value)) {
     return false;
   }
-  // And it MUST have 'unit' OR 'type'
-  const hasUnit = Object.prototype.hasOwnProperty.call(value, 'unit');
-  const hasType = Object.prototype.hasOwnProperty.call(value, 'type');
 
-  return hasUnit || hasType;
+  // MUST have 'value' and 'unit' property
+  return Object.prototype.hasOwnProperty.call(value, 'value') && Object.prototype.hasOwnProperty.call(value, 'unit');
 }
 
 /**
@@ -69,62 +68,78 @@ export function isAttributeObject(value: unknown): value is ValidAttributeObject
  * @param value - The value of the passed attribute.
  * @returns The typed attribute.
  */
-export function attributeValueToTypedAttributeValue(value: unknown): TypedAttributeValue {
+export function attributeValueToTypedAttributeValue(rawValue: unknown): TypedAttributeValue {
+  const unit = isAttributeObject(rawValue) ? rawValue.unit : undefined;
+  const value = isAttributeObject(rawValue) ? rawValue.value : rawValue;
+
   switch (typeof value) {
-    case 'number':
-      if (Number.isInteger(value)) {
-        return {
-          value,
-          type: 'integer',
-        };
+    case 'number': {
+      const numberType = getNumberType(value);
+      if (!numberType) {
+        break;
       }
       return {
         value,
-        type: 'double',
+        type: numberType,
+        unit,
       };
+    }
     case 'boolean':
       return {
         value,
         type: 'boolean',
+        unit,
       };
     case 'string':
       return {
         value,
         type: 'string',
+        unit,
       };
   }
 
   if (Array.isArray(value)) {
-    if (value.every(item => typeof item === 'string')) {
-      return {
-        value,
-        type: 'string[]',
-      };
-    }
-    if (value.every(item => typeof item === 'number')) {
-      if (value.every(item => Number.isInteger(item))) {
-        return {
-          value,
-          type: 'integer[]',
-        };
-      } else if (value.every(item => !Number.isInteger(item))) {
-        return {
-          value,
-          type: 'double[]',
-        };
+    const coherentType = value.reduce((acc: 'string' | 'boolean' | 'integer' | 'double' | null, item) => {
+      if (!acc || getPrimitiveType(item) !== acc) {
+        return null;
       }
-    }
-    if (value.every(item => typeof item === 'boolean')) {
-      return {
-        value,
-        type: 'boolean[]',
-      };
+      return acc;
+    }, getPrimitiveType(value[0]));
+
+    if (coherentType) {
+      return { value, type: `${coherentType}[]`, unit };
     }
   }
 
   // Fallback: stringify the passed value
+  let fallbackValue = '';
+  try {
+    fallbackValue = JSON.stringify(value) ?? String(value);
+  } catch {
+    try {
+      fallbackValue = String(value);
+    } catch {
+      // ignore
+    }
+  }
+
   return {
-    value: JSON.stringify(value),
+    value: fallbackValue,
     type: 'string',
+    unit,
   };
 }
+
+// Disallow NaN, differentiate between integer and double
+const getNumberType: (num: number) => 'integer' | 'double' | null = item =>
+  Number.isNaN(item) ? null : Number.isInteger(item) ? 'integer' : 'double';
+
+// Only allow string, boolean, or number types
+const getPrimitiveType: (item: unknown) => 'string' | 'boolean' | 'integer' | 'double' | null = item =>
+  typeof item === 'string'
+    ? 'string'
+    : typeof item === 'boolean'
+      ? 'boolean'
+      : typeof item === 'number'
+        ? getNumberType(item)
+        : null;
diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts
@@ -1,6 +1,5 @@
 /* eslint-disable max-lines */
-import type { Attributes, AttributeValueType, TypedAttributeValue, ValidatedAttributes } from './attributes';
-import { attributeValueToTypedAttributeValue, isAttributeObject } from './attributes';
+import type { AttributeWithUnit, RawAttribute, RawAttributes } from './attributes';
 import type { Client } from './client';
 import { DEBUG_BUILD } from './debug-build';
 import { updateSession } from './session';
@@ -48,7 +47,7 @@ export interface ScopeContext {
   extra: Extras;
   contexts: Contexts;
   tags: { [key: string]: Primitive };
-  attributes?: Attributes;
+  attributes?: RawAttributes<Record<string, unknown>>;
   fingerprint: string[];
   propagationContext: PropagationContext;
 }
@@ -75,7 +74,7 @@ export interface ScopeData {
   user: User;
   tags: { [key: string]: Primitive };
   // TODO(v11): Make this a required field (could be subtly breaking if we did it today)
-  attributes?: Attributes;
+  attributes?: RawAttributes<Record<string, unknown>>;
   extra: Extras;
   contexts: Contexts;
   attachments: Attachment[];
@@ -110,7 +109,7 @@ export class Scope {
   protected _tags: { [key: string]: Primitive };
 
   /** Attributes */
-  protected _attributes: Attributes;
+  protected _attributes: RawAttributes<Record<string, unknown>>;
 
   /** Extra */
   protected _extra: Extras;
@@ -311,39 +310,23 @@ export class Scope {
    * Currently, these attributes are not applied to any telemetry data but they will be in the future.
    *
    * @param newAttributes - The attributes to set on the scope. You can either pass in key-value pairs, or
-   * an object with a concrete type declaration and an optional unit (if applicable to your attribute).
-   * You can only pass in primitive values or arrays of primitive values.
+   * an object with a `value` and an optional `unit` (if applicable to your attribute).
    *
    * @example
    * ```typescript
    * scope.setAttributes({
    *   is_admin: true,
    *   payment_selection: 'credit_card',
    *   clicked_products: [130, 554, 292],
-   *   render_duration: { value: 'render_duration', type: 'float', unit: 'ms' },
+   *   render_duration: { value: 'render_duration', unit: 'ms' },
    * });
    * ```
    */
-  public setAttributes<T extends Record<string, unknown>>(newAttributes: T & ValidatedAttributes<T>): this {
-    Object.entries(newAttributes).forEach(([key, value]) => {
-      if (isAttributeObject(value)) {
-        // Case 1: ({ value, unit })
-        if ('unit' in value && !('type' in value)) {
-          // Infer type from the inner value
-          this._attributes[key] = {
-            ...attributeValueToTypedAttributeValue(value.value),
-            unit: value.unit,
-          };
-        }
-        // Case 2: ({ value, type, unit? })
-        else {
-          this._attributes[key] = value;
-        }
-      } else {
-        // Else: (string, number, etc.) or a random object (will stringify random values).
-        this._attributes[key] = attributeValueToTypedAttributeValue(value);
-      }
-    });
+  public setAttributes<T extends Record<string, unknown>>(newAttributes: RawAttributes<T>): this {
+    this._attributes = {
+      ...this._attributes,
+      ...newAttributes,
+    };
 
     this._notifyScopeListeners();
     return this;
@@ -356,17 +339,21 @@ export class Scope {
    * Currently, these attributes are not applied to any telemetry data but they will be in the future.
    *
    * @param key - The attribute key.
-   * @param value - the attribute value. You can either pass in a raw value (primitive or array of primitives), or
-   * a typed attribute value object with a concrete type declaration and an optional unit (if applicable to your attribute).
+   * @param value - the attribute value. You can either pass in a raw value, or an attribute
+   * object with a `value` and an optional `unit` (if applicable to your attribute).
    *
    * @example
    * ```typescript
    * scope.setAttribute('is_admin', true);
    * scope.setAttribute('clicked_products', [130, 554, 292]);
-   * scope.setAttribute('render_duration', { value: 'render_duration', type: 'float', unit: 'ms' });
+   * scope.setAttribute('render_duration', { value: 'render_duration', unit: 'ms' });
    * ```
    */
-  public setAttribute(key: string, value: AttributeValueType | TypedAttributeValue): this {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  public setAttribute<T extends RawAttribute<T> extends { value: any } | { unit: any } ? AttributeWithUnit : unknown>(
+    key: string,
+    value: RawAttribute<T>,
+  ): this {
     return this.setAttributes({ [key]: value });
   }
 
diff --git a/packages/core/test/lib/attributes.test.ts b/packages/core/test/lib/attributes.test.ts
@@ -70,6 +70,16 @@ describe('attributeValueToTypedAttributeValue', () => {
     });
   });
 
+  describe('attribute objects without units', () => {
+    it('converts a number value to a typed attribute value', () => {
+      const result = attributeValueToTypedAttributeValue({ value: 123 });
+      expect(result).toEqual({
+        value: 123,
+        type: 'integer',
+      });
+    });
+  });
+
   describe('disallowed value types', () => {
     it('stringifies mixed float and integer numbers to a string attribute value', () => {
       const result = attributeValueToTypedAttributeValue([1, 2.2, 3]);
@@ -79,20 +89,52 @@ describe('attributeValueToTypedAttributeValue', () => {
       });
     });
 
-    it('stringifies an array of mixed types to a string attribute value', () => {
+    it('stringifies an array of allowed but incoherent types to a string attribute value', () => {
       const result = attributeValueToTypedAttributeValue([1, 'foo', true]);
       expect(result).toEqual({
         value: '[1,"foo",true]',
         type: 'string',
       });
     });
 
+    it('stringifies an array of disallowed and incoherent types to a string attribute value', () => {
+      const result = attributeValueToTypedAttributeValue([null, undefined, NaN]);
+      expect(result).toEqual({
+        value: '[null,null,null]',
+        type: 'string',
+      });
+    });
+
     it('stringifies an object value to a string attribute value', () => {
       const result = attributeValueToTypedAttributeValue({ foo: 'bar' });
       expect(result).toEqual({
         value: '{"foo":"bar"}',
         type: 'string',
       });
     });
+
+    it('stringifies a null value to a string attribute value', () => {
+      const result = attributeValueToTypedAttributeValue(null);
+      expect(result).toEqual({
+        value: 'null',
+        type: 'string',
+      });
+    });
+
+    it('stringifies an undefined value to a string attribute value', () => {
+      const result = attributeValueToTypedAttributeValue(undefined);
+      expect(result).toEqual({
+        value: 'undefined',
+        type: 'string',
+      });
+    });
+
+    it('stringifies an NaN number value to a string attribute value', () => {
+      const result = attributeValueToTypedAttributeValue(NaN);
+      expect(result).toEqual({
+        value: 'null',
+        type: 'string',
+      });
+    });
   });
 });
diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts
