ref(core): Use serializeAttributes for metric attribute serialization (#18582)

pre-work for #18160 

Use the same attibute serialization logic we already use in logs.

Bundle size impact:
- size decreases for users with logs and metrics
- size increases for metrics-only users. I think this is because
previously the separate serialization logic could be inlined, while now
it has to inline the slightly larger logic. Since it's not used in any
non-treeshakeable default behavior (yet), this results in higher bundle
size. Given we'll use this in our span serialization logic in v11, it
will become part of the minimum SDK anyway (without metrics), so it
_should_ be fine to do this now.
- Because we include metrics (but not logs) in all our CDN bundles by
accident, this now also increases CDN bundle size 😬. We're tracking
removal of this for most bundles in v11 (#18583)

I think the largest positive long-term aspect of this refactor is that
we'll re-use this logic more and more going forward (scope attributes on
metrics, spansv2, and other telemetry items in the future), so I'd like
to have it unified.

Author: Lms24

packages/core/src/attributes.ts

@@ -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);
@@ -84,7 +84,7 @@ export function attributeValueToTypedAttributeValue(
     return { ...attributeValue, ...checkedUnit };
   }
 
-  if (!useFallback) {
+  if (!useFallback || (useFallback === 'skip-undefined' && value === undefined)) {
     return;
   }
 
@@ -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;

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';

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

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<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 {
   /**
@@ -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 = {

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,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', () => {
@@ -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"}',
+        },
+      });
+    });
+  });
+});