streamline isAttributeObject

Author: Lms24

packages/core/src/attributes.ts

@@ -1,6 +1,6 @@
 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 RawAttribute<T> = T extends { value: any } | { unit: any } ? AttributeObject : T;
 
 export type Attributes = Record<string, TypedAttributeValue>;
 
@@ -31,7 +31,7 @@ type AttributeUnion = {
 
 export type TypedAttributeValue = AttributeUnion & { unit?: Units };
 
-export type AttributeWithUnit = {
+export type AttributeObject = {
   value: unknown;
   unit?: Units;
 };
@@ -44,35 +44,20 @@ type Units = 'ms' | 's' | 'bytes' | 'count' | 'percent' | string;
 /* 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 } ? AttributeWithUnit : unknown;
+  [K in keyof T]: T[K] extends { value: any } | { unit: any } ? AttributeObject : 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(maybeObj: unknown): maybeObj is AttributeWithUnit {
-  if (typeof maybeObj !== 'object' || maybeObj == null || Array.isArray(maybeObj)) {
-    return false;
-  }
-
-  // MUST have 'value' property
-  // MAY have 'unit' property
-  // MUST NOT have other properties
-  const keys = Object.keys(maybeObj);
-
-  // MUST have 'value'
-  if (!keys.includes('value')) {
-    return false;
-  }
-
-  // ALLOWED keys: 'value', optionally 'unit'
-  if (keys.some(k => k !== 'value' && k !== 'unit')) {
-    return false;
-  }
-
-  // All checks passed
-  return true;
+export function isAttributeObject(maybeObj: unknown): maybeObj is AttributeObject {
+  return (
+    typeof maybeObj === 'object' &&
+    maybeObj != null &&
+    !Array.isArray(maybeObj) &&
+    Object.keys(maybeObj).includes('value')
+  );
 }
 
 /**

packages/core/src/scope.ts

@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */
-import type { AttributeWithUnit, RawAttribute, RawAttributes } from './attributes';
+import type { AttributeObject, RawAttribute, RawAttributes } from './attributes';
 import type { Client } from './client';
 import { DEBUG_BUILD } from './debug-build';
 import { updateSession } from './session';
@@ -350,7 +350,7 @@ export class Scope {
    * ```
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  public setAttribute<T extends RawAttribute<T> extends { value: any } | { unit: any } ? AttributeWithUnit : unknown>(
+  public setAttribute<T extends RawAttribute<T> extends { value: any } | { unit: any } ? AttributeObject : unknown>(
     key: string,
     value: RawAttribute<T>,
   ): this {

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

@@ -125,6 +125,17 @@ describe('attributeValueToTypedAttributeValue', () => {
         unit: 'bytes',
       });
     });
+
+    it('extracts the value property of an object with a value property', () => {
+      // and ignores other properties.
+      // For now we're fine with this but we may reconsider in the future.
+      const result = attributeValueToTypedAttributeValue({ value: 'foo', unit: 'ms', bar: 'baz' });
+      expect(result).toStrictEqual({
+        value: 'foo',
+        unit: 'ms',
+        type: 'string',
+      });
+    });
   });
 
   describe('unsupported value types', () => {
@@ -233,22 +244,6 @@ describe('attributeValueToTypedAttributeValue', () => {
         type: 'string',
       });
     });
-
-    it('stringifies an attribute-object-like object with additional properties to a string attribute value', () => {
-      const result = attributeValueToTypedAttributeValue({ value: 'foo', bar: 'baz' });
-      expect(result).toStrictEqual({
-        value: '{"value":"foo","bar":"baz"}',
-        type: 'string',
-      });
-    });
-
-    it('stringifies an attribute-object-like object with a unit property to a string attribute value', () => {
-      const result = attributeValueToTypedAttributeValue({ value: 'foo', unit: 'ms', bar: 'baz' });
-      expect(result).toStrictEqual({
-        value: '{"value":"foo","unit":"ms","bar":"baz"}',
-        type: 'string',
-      });
-    });
   });
 
   it.each([1, true, null, undefined, NaN, Symbol('test'), { foo: 'bar' }])(
@@ -275,18 +270,17 @@ describe('isAttributeObject', () => {
     expect(result).toBe(true);
   });
 
-  it.each([
-    1,
-    true,
-    'test',
-    null,
-    undefined,
-    NaN,
-    Symbol('test'),
-    { value: { foo: 'bar' }, bar: 'baz' },
-    { value: 1, unit: 'ms', anotherProperty: 'test' },
-  ])('returns false for an invalid attribute object (%s)', obj => {
-    const result = isAttributeObject(obj);
-    expect(result).toBe(false);
+  it('returns true for an object with a value property', () => {
+    // Explicitly demonstrate this behaviour which for now we're fine with.
+    // We may reconsider in the future.
+    expect(isAttributeObject({ value: 123.45, some: 'other property' })).toBe(true);
   });
+
+  it.each([1, true, 'test', null, undefined, NaN, Symbol('test')])(
+    'returns false for an invalid attribute object (%s)',
+    obj => {
+      const result = isAttributeObject(obj);
+      expect(result).toBe(false);
+    },
+  );
 });