Set attr directly on eval instead of in hook

aliu39 · aliu39 · commit 2b385a47c8f7 · 2025-06-14T17:46:29.000-07:00
diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/test.ts
@@ -58,12 +58,9 @@ sentryTest("Feature flags are added to active span's attributes on span end.", a
   expect(innerSpanFlags).toEqual([]);
 
   const expectedOuterSpanFlags = [];
-  for (let i = 1; i <= 2; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
+  for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
+    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
   }
-  for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
-  }
-  expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
-  expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
+  // Order agnostic (attribute dict is unordered).
+  expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/onSpan/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/onSpan/test.ts
@@ -58,12 +58,9 @@ sentryTest("Feature flags are added to active span's attributes on span end.", a
   expect(innerSpanFlags).toEqual([]);
 
   const expectedOuterSpanFlags = [];
-  for (let i = 1; i <= 2; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
+  for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
+    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
   }
-  for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
-  }
-  expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
-  expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
+  // Order agnostic (attribute dict is unordered).
+  expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/onSpan/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/onSpan/test.ts
@@ -58,12 +58,9 @@ sentryTest("Feature flags are added to active span's attributes on span end.", a
   expect(innerSpanFlags).toEqual([]);
 
   const expectedOuterSpanFlags = [];
-  for (let i = 1; i <= 2; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
+  for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
+    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
   }
-  for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
-  }
-  expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
-  expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
+  // Order agnostic (attribute dict is unordered).
+  expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/statsig/onSpan/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/statsig/onSpan/test.ts
@@ -62,12 +62,9 @@ sentryTest("Feature flags are added to active span's attributes on span end.", a
   expect(innerSpanFlags).toEqual([]);
 
   const expectedOuterSpanFlags = [];
-  for (let i = 1; i <= 2; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
+  for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
+    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
   }
-  for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
-  }
-  expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
-  expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
+  // Order agnostic (attribute dict is unordered).
+  expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/onSpan/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/onSpan/test.ts
@@ -60,12 +60,9 @@ sentryTest("Feature flags are added to active span's attributes on span end.", a
   expect(innerSpanFlags).toEqual([]);
 
   const expectedOuterSpanFlags = [];
-  for (let i = 1; i <= 2; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
+  for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
+    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
   }
-  for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
-    expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
-  }
-  expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
-  expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
+  // Order agnostic (attribute dict is unordered).
+  expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
 });
diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts
@@ -1,11 +1,6 @@
 import type { Client, Event, EventHint, Integration, IntegrationFn, Span } from '@sentry/core';
 import { defineIntegration } from '@sentry/core';
-import {
-  bufferSpanFeatureFlag,
-  copyFlagsFromScopeToEvent,
-  freezeSpanFeatureFlags,
-  insertFlagToScope,
-} from '../../utils/featureFlags';
+import { addFeatureFlagToActiveSpan, copyFlagsFromScopeToEvent, insertFlagToScope } from '../../utils/featureFlags';
 
 export interface FeatureFlagsIntegration extends Integration {
   addFeatureFlag: (name: string, value: unknown) => void;
@@ -39,19 +34,13 @@ export const featureFlagsIntegration = defineIntegration(() => {
   return {
     name: 'FeatureFlags',
 
-    setup(client: Client) {
-      client.on('spanEnd', (span: Span) => {
-        freezeSpanFeatureFlags(span);
-      });
-    },
-
     processEvent(event: Event, _hint: EventHint, _client: Client): Event {
       return copyFlagsFromScopeToEvent(event);
     },
 
     addFeatureFlag(name: string, value: unknown): void {
       insertFlagToScope(name, value);
-      bufferSpanFeatureFlag(name, value);
+      addFeatureFlagToActiveSpan(name, value);
     },
   };
 }) as IntegrationFn<FeatureFlagsIntegration>;
diff --git a/packages/browser/src/integrations/featureFlags/launchdarkly/integration.ts b/packages/browser/src/integrations/featureFlags/launchdarkly/integration.ts
@@ -1,9 +1,8 @@
 import type { Client, Event, EventHint, IntegrationFn, Span } from '@sentry/core';
 import { defineIntegration } from '@sentry/core';
 import {
-  bufferSpanFeatureFlag,
+  addFeatureFlagToActiveSpan,
   copyFlagsFromScopeToEvent,
-  freezeSpanFeatureFlags,
   insertFlagToScope,
 } from '../../../utils/featureFlags';
 import type { LDContext, LDEvaluationDetail, LDInspectionFlagUsedHandler } from './types';
@@ -27,12 +26,6 @@ export const launchDarklyIntegration = defineIntegration(() => {
   return {
     name: 'LaunchDarkly',
 
-    setup(client: Client) {
-      client.on('spanEnd', (span: Span) => {
-        freezeSpanFeatureFlags(span);
-      });
-    },
-
     processEvent(event: Event, _hint: EventHint, _client: Client): Event {
       return copyFlagsFromScopeToEvent(event);
     },
@@ -57,7 +50,7 @@ export function buildLaunchDarklyFlagUsedHandler(): LDInspectionFlagUsedHandler
      */
     method: (flagKey: string, flagDetail: LDEvaluationDetail, _context: LDContext) => {
       insertFlagToScope(flagKey, flagDetail.value);
-      bufferSpanFeatureFlag(flagKey, flagDetail.value);
+      addFeatureFlagToActiveSpan(flagKey, flagDetail.value);
     },
   };
 }
diff --git a/packages/browser/src/integrations/featureFlags/openfeature/integration.ts b/packages/browser/src/integrations/featureFlags/openfeature/integration.ts
@@ -16,9 +16,8 @@
 import type { Client, Event, EventHint, IntegrationFn, Span } from '@sentry/core';
 import { defineIntegration } from '@sentry/core';
 import {
-  bufferSpanFeatureFlag,
+  addFeatureFlagToActiveSpan,
   copyFlagsFromScopeToEvent,
-  freezeSpanFeatureFlags,
   insertFlagToScope,
 } from '../../../utils/featureFlags';
 import type { EvaluationDetails, HookContext, HookHints, JsonValue, OpenFeatureHook } from './types';
@@ -27,12 +26,6 @@ export const openFeatureIntegration = defineIntegration(() => {
   return {
     name: 'OpenFeature',
 
-    setup(client: Client) {
-      client.on('spanEnd', (span: Span) => {
-        freezeSpanFeatureFlags(span);
-      });
-    },
-
     processEvent(event: Event, _hint: EventHint, _client: Client): Event {
       return copyFlagsFromScopeToEvent(event);
     },
@@ -48,14 +41,14 @@ export class OpenFeatureIntegrationHook implements OpenFeatureHook {
    */
   public after(_hookContext: Readonly<HookContext<JsonValue>>, evaluationDetails: EvaluationDetails<JsonValue>): void {
     insertFlagToScope(evaluationDetails.flagKey, evaluationDetails.value);
-    bufferSpanFeatureFlag(evaluationDetails.flagKey, evaluationDetails.value);
+    addFeatureFlagToActiveSpan(evaluationDetails.flagKey, evaluationDetails.value);
   }
 
   /**
    * On error evaluation result.
    */
   public error(hookContext: Readonly<HookContext<JsonValue>>, _error: unknown, _hookHints?: HookHints): void {
     insertFlagToScope(hookContext.flagKey, hookContext.defaultValue);
-    bufferSpanFeatureFlag(hookContext.flagKey, hookContext.defaultValue);
+    addFeatureFlagToActiveSpan(hookContext.flagKey, hookContext.defaultValue);
   }
 }
diff --git a/packages/browser/src/integrations/featureFlags/statsig/integration.ts b/packages/browser/src/integrations/featureFlags/statsig/integration.ts
@@ -1,11 +1,6 @@
 import type { Client, Event, EventHint, IntegrationFn, Span } from '@sentry/core';
 import { defineIntegration } from '@sentry/core';
-import {
-  bufferSpanFeatureFlag,
-  copyFlagsFromScopeToEvent,
-  freezeSpanFeatureFlags,
-  insertFlagToScope,
-} from '../../../utils/featureFlags';
+import { addFeatureFlagToActiveSpan, copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags';
 import type { FeatureGate, StatsigClient } from './types';
 
 /**
@@ -36,14 +31,10 @@ export const statsigIntegration = defineIntegration(
     return {
       name: 'Statsig',
 
-      setup(client: Client) {
-        client.on('spanEnd', (span: Span) => {
-          freezeSpanFeatureFlags(span);
-        });
-
+      setup(_client: Client) {
         statsigClient.on('gate_evaluation', (event: { gate: FeatureGate }) => {
           insertFlagToScope(event.gate.name, event.gate.value);
-          bufferSpanFeatureFlag(event.gate.name, event.gate.value);
+          addFeatureFlagToActiveSpan(event.gate.name, event.gate.value);
         });
       },
 
diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts
@@ -2,9 +2,8 @@ import type { Client, Event, EventHint, IntegrationFn, Span } from '@sentry/core
 import { defineIntegration, fill, logger } from '@sentry/core';
 import { DEBUG_BUILD } from '../../../debug-build';
 import {
-  bufferSpanFeatureFlag,
+  addFeatureFlagToActiveSpan,
   copyFlagsFromScopeToEvent,
-  freezeSpanFeatureFlags,
   insertFlagToScope,
 } from '../../../utils/featureFlags';
 import type { UnleashClient, UnleashClientClass } from './types';
@@ -40,12 +39,6 @@ export const unleashIntegration = defineIntegration(
     return {
       name: 'Unleash',
 
-      setup(client: Client) {
-        client.on('spanEnd', (span: Span) => {
-          freezeSpanFeatureFlags(span);
-        });
-      },
-
       setupOnce() {
         const unleashClientPrototype = unleashClientClass.prototype as UnleashClient;
         fill(unleashClientPrototype, 'isEnabled', _wrappedIsEnabled);
@@ -76,7 +69,7 @@ function _wrappedIsEnabled(
 
     if (typeof toggleName === 'string' && typeof result === 'boolean') {
       insertFlagToScope(toggleName, result);
-      bufferSpanFeatureFlag(toggleName, result);
+      addFeatureFlagToActiveSpan(toggleName, result);
     } else if (DEBUG_BUILD) {
       logger.error(
         `[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`,
diff --git a/packages/browser/src/utils/featureFlags.ts b/packages/browser/src/utils/featureFlags.ts
@@ -19,7 +19,7 @@ export const FLAG_BUFFER_SIZE = 100;
 export const MAX_FLAGS_PER_SPAN = 10;
 
 // Global map of spans to feature flag buffers. Populated by feature flag integrations.
-GLOBAL_OBJ._spanToFlagBufferMap = new WeakMap<Span, FeatureFlag[]>();
+GLOBAL_OBJ._spanToFlagBufferMap = new WeakMap<Span, Set<string>>();
 
 const SPAN_FLAG_ATTRIBUTE_PREFIX = 'flag.evaluation.';
 
@@ -116,14 +116,15 @@ export function insertToFlagBuffer(
 }
 
 /**
- * Records a feature flag evaluation for the active span, adding it to a weak map of flag buffers. This is a no-op for non-boolean values.
- * The keys in each buffer are unique. Once the buffer for a span reaches maxFlagsPerSpan, subsequent flags are dropped.
+ * Records a feature flag evaluation for the active span. This is a no-op for non-boolean values.
+ * The flag and its value is stored in span attributes with the `flag.evaluation` prefix. Once the
+ * unique flags for a span reaches maxFlagsPerSpan, subsequent flags are dropped.
  *
  * @param name             Name of the feature flag.
  * @param value            Value of the feature flag. Non-boolean values are ignored.
  * @param maxFlagsPerSpan  Max number of flags a buffer should store. Default value should always be used in production.
  */
-export function bufferSpanFeatureFlag(
+export function addFeatureFlagToActiveSpan(
   name: string,
   value: unknown,
   maxFlagsPerSpan: number = MAX_FLAGS_PER_SPAN,
@@ -135,22 +136,13 @@ export function bufferSpanFeatureFlag(
 
   const span = getActiveSpan();
   if (span) {
-    const flags = spanFlagMap.get(span) || [];
-    insertToFlagBuffer(flags, name, value, maxFlagsPerSpan, false);
+    const flags = spanFlagMap.get(span) || new Set<string>();
+    if (flags.has(name)) {
+      span.setAttribute(`${SPAN_FLAG_ATTRIBUTE_PREFIX}${name}`, value);
+    } else if (flags.size < maxFlagsPerSpan) {
+      flags.add(name);
+      span.setAttribute(`${SPAN_FLAG_ATTRIBUTE_PREFIX}${name}`, value);
+    }
     spanFlagMap.set(span, flags);
   }
 }
-
-/**
- * Add the buffered feature flags for a span to the span attributes. Call this on span end.
- *
- * @param span         Span to add flags to.
- */
-export function freezeSpanFeatureFlags(span: Span): void {
-  const flags = GLOBAL_OBJ._spanToFlagBufferMap?.get(span);
-  if (flags) {
-    span.setAttributes(
-      Object.fromEntries(flags.map(flag => [`${SPAN_FLAG_ATTRIBUTE_PREFIX}${flag.flag}`, flag.result])),
-    );
-  }
-}
diff --git a/packages/core/src/utils-hoist/worldwide.ts b/packages/core/src/utils-hoist/worldwide.ts
@@ -59,9 +59,9 @@ export type InternalGlobal = {
   _sentryModuleMetadata?: Record<string, any>;
   _sentryEsmLoaderHookRegistered?: boolean;
   /**
-   * A map of spans to feature flag buffers. Populated by feature flag integrations.
+   * A map of spans to evaluated feature flags. Populated by feature flag integrations.
    */
-  _spanToFlagBufferMap?: WeakMap<Span, FeatureFlag[]>;
+  _spanToFlagBufferMap?: WeakMap<Span, Set<string>>;
 } & Carrier;
 
 /** Get's the global object for the current JavaScript runtime */
