From a9fc71873618b6fcfae51a97697e1a8db76647ec Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Tue, 6 Oct 2026 09:31:58 +0200 Subject: [PATCH] fix: record analytics against the canonical feature name evaluateFlag bucketed by the caller's raw string rather than the resolved flag, so getValue('MyFlag') and getValue('myflag') hit the same flag but reported as two separate features. The API resolves analytics with a case-sensitive name__in against Feature.name, so neither spelling resolves unless it matches the feature name exactly - meaning flags in projects that don't use lower-case names never register as in use, which drives stale flag detection. evaluateFlag, the Sentry addFeatureFlag call and trackExposureEvent now all report the resolved flag's name, falling back to the normalised key for flags that don't exist so two spellings of a missing flag collapse to one bucket. Co-Authored-By: Claude Opus 5 (1M context) --- flagsmith-core.ts | 28 ++++++++++++++-------- test/feature-name-casing.test.ts | 41 +++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/flagsmith-core.ts b/flagsmith-core.ts index fc5fce6..ab5ffe7 100644 --- a/flagsmith-core.ts +++ b/flagsmith-core.ts @@ -704,7 +704,7 @@ const Flagsmith = class { } if (!options?.skipAnalytics && !skipAnalytics) { - this.evaluateFlag(key, "VALUE"); + this.evaluateFlag(key, "VALUE", flag); } if (res === null && typeof options?.fallback !== 'undefined') { @@ -829,13 +829,13 @@ const Flagsmith = class { res = true; } if ((usingNewOptions && !options.skipAnalytics) || !options) { - this.evaluateFlag(key, "ENABLED"); + this.evaluateFlag(key, "ENABLED", flag); } if(this.sentryClient) { try { this.sentryClient.getIntegrationByName( "FeatureFlags", - )?.addFeatureFlag?.(key, res); + )?.addFeatureFlag?.(flag?.name || normalizeFlagKey(key), res); } catch (e) { console.error(e) } @@ -950,15 +950,20 @@ const Flagsmith = class { } } - private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED') => { + private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED', flag?: IFlagsmithFeature | null) => { + // The API resolves analytics by exact feature name, so always report the + // feature's own name rather than whatever casing the caller happened to use. + // Without this, getValue('My Flag') and getValue('my_flag') report as two + // separate features and neither resolves unless it matches the name exactly. + const analyticsKey = flag?.name || normalizeFlagKey(key); if (this.datadogRum) { if (!this.datadogRum!.client!.addFeatureFlagEvaluation) { console.error('Flagsmith: Your datadog RUM client does not support the function addFeatureFlagEvaluation, please update it.'); } else { if (method === 'VALUE') { - this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + key, this.getValue(key, {}, true)); + this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + analyticsKey, this.getValue(key, {}, true)); } else { - this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + key, this.hasFeature(key, true)); + this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + analyticsKey, this.hasFeature(key, true)); } } } @@ -968,10 +973,10 @@ const Flagsmith = class { if (!this.evaluationEvent[this.evaluationContext.environment.apiKey]) { this.evaluationEvent[this.evaluationContext.environment.apiKey] = {}; } - if (this.evaluationEvent[this.evaluationContext.environment.apiKey][key] === undefined) { - this.evaluationEvent[this.evaluationContext.environment.apiKey][key] = 0; + if (this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] === undefined) { + this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] = 0; } - this.evaluationEvent[this.evaluationContext.environment.apiKey][key] += 1; + this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] += 1; } this.updateEventStorage(); @@ -1010,8 +1015,11 @@ const Flagsmith = class { this.log(`Flagsmith: trackExposureEvent called for "${featureName}" without an identity; call identify() (optionally with transient: true) or pass opts.identifier. No exposure recorded.`); return; } + // As with evaluateFlag, report the feature's own name so that exposures for the + // same flag aggregate regardless of the casing the caller used. + const key = normalizeFlagKey(featureName); this.eventProcessor.trackExposureEvent({ - featureName, + featureName: this.flags?.[key]?.name || key, identifier, value: opts?.value ?? null, traits: resolveTraitValues(opts?.traits ?? this.evaluationContext.identity?.traits), diff --git a/test/feature-name-casing.test.ts b/test/feature-name-casing.test.ts index d55eade..5be6919 100644 --- a/test/feature-name-casing.test.ts +++ b/test/feature-name-casing.test.ts @@ -1,5 +1,5 @@ import { IInitConfig } from '../types'; -import { getFlagsmith } from './test-constants'; +import { delay, getFlagsmith } from './test-constants'; const mixedCaseFlags = [ { @@ -14,6 +14,14 @@ const mixedCaseFlags = [ }, ]; +const instances: any[] = []; + +afterEach(() => { + // enableAnalytics starts an interval that nothing public tears down. + instances.forEach((flagsmith) => clearInterval(flagsmith.analyticsInterval)); + instances.length = 0; +}); + async function initWithMixedCaseFlags(config: Partial = {}) { const { flagsmith, initConfig, mockFetch } = getFlagsmith(config); mockFetch.mockImplementation(async (url: string) => { @@ -26,9 +34,15 @@ async function initWithMixedCaseFlags(config: Partial = {}) { throw new Error('Please mock the call to ' + url); }); await flagsmith.init(initConfig); + instances.push(flagsmith); return { flagsmith, mockFetch }; } +const postedAnalytics = (mockFetch: jest.Mock) => { + const call = mockFetch.mock.calls.find(([url]: [string]) => url.includes('analytics/flags')); + return call ? JSON.parse(call[1].body) : null; +}; + describe('feature name casing', () => { test('exposes the original feature name on each flag', async () => { const { flagsmith } = await initWithMixedCaseFlags(); @@ -44,4 +58,29 @@ describe('feature name casing', () => { expect(flagsmith.getValue('myfeatureflag')).toBe('on'); expect(flagsmith.hasFeature('MYFEATUREFLAG')).toBe(true); }); + + test('records analytics against the canonical feature name, whatever casing the caller used', async () => { + const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true }); + await delay(1); // evaluationEvent is restored from storage asynchronously + + flagsmith.getValue('myfeatureflag'); + flagsmith.getValue('MyFeatureFlag'); + flagsmith.hasFeature('MYFEATUREFLAG'); + + // @ts-ignore internal, normally driven by an interval + await flagsmith.analyticsFlags(); + expect(postedAnalytics(mockFetch)).toEqual({ MyFeatureFlag: 3 }); + }); + + test('buckets unknown flags under the normalised key rather than the caller casing', async () => { + const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true }); + await delay(1); + + flagsmith.getValue('Does Not Exist'); + flagsmith.getValue('does_not_exist'); + + // @ts-ignore internal, normally driven by an interval + await flagsmith.analyticsFlags(); + expect(postedAnalytics(mockFetch)).toEqual({ does_not_exist: 2 }); + }); });