Skip to content

Commit 98ee4cc

Browse files
authored
feat(flags): capture feature flag evaluations on spans (#16485)
Based off https://github.com/getsentry/sentry-python/pull/4280/files Reopened from #16475 This PR updates our 5 browser FF integrations to capture flag evaluations on spans. This implementation avoids changes to the `Span` class, saving bundle size. - on eval, flags are added to span attributes. A global WeakMap is used to track the unique flags for a span. Updates to a flag's value is allowed. - staying consistent with the python PR: - we only capture values for the **first 10 unique flags** per span. Subsequent flags are dropped. - attribute keys have the format `flag.evaluation.{flagKey}` From @AbhiPrasad : > A method on the span break our compat with OTEL spans, which we use for duck typing in some scenarios. From @cmanallen : > For spans the oldest flag evaluations are the most important because they're likely to have the largest performance impact. We drop late arrivals due to request size concerns.
1 parent 23127b6 commit 98ee4cc

File tree

55 files changed

+829
-67
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+829
-67
lines changed
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
export const FLAG_BUFFER_SIZE = 100; // Corresponds to constant in featureFlags.ts, in browser utils.
1+
// Corresponds to constants in featureFlags.ts, in browser utils.
2+
export const FLAG_BUFFER_SIZE = 100;
3+
export const MAX_FLAGS_PER_SPAN = 10;

dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onError/basic/test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
2-
import { sentryTest } from '../../../../../utils/fixtures';
3-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
4-
import { FLAG_BUFFER_SIZE } from '../../constants';
2+
import { sentryTest } from '../../../../../../utils/fixtures';
3+
import {
4+
envelopeRequestParser,
5+
shouldSkipFeatureFlagsTest,
6+
waitForErrorRequest,
7+
} from '../../../../../../utils/helpers';
8+
import { FLAG_BUFFER_SIZE } from '../../../constants';
59

610
sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {

dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onError/withScope/test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
22
import type { Scope } from '@sentry/browser';
3-
import { sentryTest } from '../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
3+
import { sentryTest } from '../../../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipFeatureFlagsTest,
7+
waitForErrorRequest,
8+
} from '../../../../../../utils/helpers';
59

610
sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
sampleRate: 1.0,
8+
tracesSampleRate: 1.0,
9+
integrations: [
10+
Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false }),
11+
Sentry.featureFlagsIntegration(),
12+
],
13+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
const btnStartSpan = document.getElementById('btnStartSpan');
2+
const btnEndSpan = document.getElementById('btnEndSpan');
3+
const btnStartNestedSpan = document.getElementById('btnStartNestedSpan');
4+
const btnEndNestedSpan = document.getElementById('btnEndNestedSpan');
5+
6+
window.withNestedSpans = callback => {
7+
window.Sentry.startSpan({ name: 'test-root-span' }, rootSpan => {
8+
window.traceId = rootSpan.spanContext().traceId;
9+
10+
window.Sentry.startSpan({ name: 'test-span' }, _span => {
11+
window.Sentry.startSpan({ name: 'test-nested-span' }, _nestedSpan => {
12+
callback();
13+
});
14+
});
15+
});
16+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="btnStartSpan">Start Span</button>
8+
<button id="btnEndSpan">End Span</button>
9+
<button id="btnStartNestedSpan">Start Nested Span</button>
10+
<button id="btnEndNestedSpan">End Nested Span</button>
11+
</body>
12+
</html>
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../../utils/fixtures';
3+
import {
4+
type EventAndTraceHeader,
5+
eventAndTraceHeaderRequestParser,
6+
getMultipleSentryEnvelopeRequests,
7+
shouldSkipFeatureFlagsTest,
8+
shouldSkipTracingTest,
9+
} from '../../../../../utils/helpers';
10+
import { MAX_FLAGS_PER_SPAN } from '../../constants';
11+
12+
sentryTest("Feature flags are added to active span's attributes on span end.", async ({ getLocalTestUrl, page }) => {
13+
if (shouldSkipFeatureFlagsTest() || shouldSkipTracingTest()) {
14+
sentryTest.skip();
15+
}
16+
17+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
18+
return route.fulfill({
19+
status: 200,
20+
contentType: 'application/json',
21+
body: JSON.stringify({}),
22+
});
23+
});
24+
25+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
26+
await page.goto(url);
27+
28+
const envelopeRequestPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>(
29+
page,
30+
1,
31+
{},
32+
eventAndTraceHeaderRequestParser,
33+
);
34+
35+
// withNestedSpans is a util used to start 3 nested spans: root-span (not recorded in transaction_event.spans), span, and nested-span.
36+
await page.evaluate(maxFlags => {
37+
(window as any).withNestedSpans(() => {
38+
const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags');
39+
for (let i = 1; i <= maxFlags; i++) {
40+
flagsIntegration.addFeatureFlag(`feat${i}`, false);
41+
}
42+
flagsIntegration.addFeatureFlag(`feat${maxFlags + 1}`, true); // dropped flag
43+
flagsIntegration.addFeatureFlag('feat3', true); // update
44+
});
45+
return true;
46+
}, MAX_FLAGS_PER_SPAN);
47+
48+
const event = (await envelopeRequestPromise)[0][0];
49+
const innerSpan = event.spans?.[0];
50+
const outerSpan = event.spans?.[1];
51+
const outerSpanFlags = Object.entries(outerSpan?.data ?? {}).filter(([key, _val]) =>
52+
key.startsWith('flag.evaluation'),
53+
);
54+
const innerSpanFlags = Object.entries(innerSpan?.data ?? {}).filter(([key, _val]) =>
55+
key.startsWith('flag.evaluation'),
56+
);
57+
58+
expect(innerSpanFlags).toEqual([]);
59+
60+
const expectedOuterSpanFlags = [];
61+
for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
62+
expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
63+
}
64+
// Order agnostic (attribute dict is unordered).
65+
expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
66+
});

dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/basic/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/onError/basic/test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
2-
import { sentryTest } from '../../../../../utils/fixtures';
3-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
4-
import { FLAG_BUFFER_SIZE } from '../../constants';
2+
import { sentryTest } from '../../../../../../utils/fixtures';
3+
import {
4+
envelopeRequestParser,
5+
shouldSkipFeatureFlagsTest,
6+
waitForErrorRequest,
7+
} from '../../../../../../utils/helpers';
8+
import { FLAG_BUFFER_SIZE } from '../../../constants';
59

610
sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {

dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/withScope/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/onError/withScope/test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
22
import type { Scope } from '@sentry/browser';
3-
import { sentryTest } from '../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
3+
import { sentryTest } from '../../../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipFeatureFlagsTest,
7+
waitForErrorRequest,
8+
} from '../../../../../../utils/helpers';
59

610
sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
window.sentryLDIntegration = Sentry.launchDarklyIntegration();
5+
6+
Sentry.init({
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
sampleRate: 1.0,
9+
tracesSampleRate: 1.0,
10+
integrations: [
11+
Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false }),
12+
window.sentryLDIntegration,
13+
],
14+
});
15+
16+
// Manually mocking this because LD only has mock test utils for the React SDK.
17+
// Also, no SDK has mock utils for FlagUsedHandler's.
18+
const MockLaunchDarkly = {
19+
initialize(_clientId, context, options) {
20+
const flagUsedHandler = options.inspectors ? options.inspectors[0].method : undefined;
21+
22+
return {
23+
variation(key, defaultValue) {
24+
if (flagUsedHandler) {
25+
flagUsedHandler(key, { value: defaultValue }, context);
26+
}
27+
return defaultValue;
28+
},
29+
};
30+
},
31+
};
32+
33+
window.initializeLD = () => {
34+
return MockLaunchDarkly.initialize(
35+
'example-client-id',
36+
{ kind: 'user', key: 'example-context-key' },
37+
{ inspectors: [Sentry.buildLaunchDarklyFlagUsedHandler()] },
38+
);
39+
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
const btnStartSpan = document.getElementById('btnStartSpan');
2+
const btnEndSpan = document.getElementById('btnEndSpan');
3+
const btnStartNestedSpan = document.getElementById('btnStartNestedSpan');
4+
const btnEndNestedSpan = document.getElementById('btnEndNestedSpan');
5+
6+
window.withNestedSpans = callback => {
7+
window.Sentry.startSpan({ name: 'test-root-span' }, rootSpan => {
8+
window.traceId = rootSpan.spanContext().traceId;
9+
10+
window.Sentry.startSpan({ name: 'test-span' }, _span => {
11+
window.Sentry.startSpan({ name: 'test-nested-span' }, _nestedSpan => {
12+
callback();
13+
});
14+
});
15+
});
16+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="btnStartSpan">Start Span</button>
8+
<button id="btnEndSpan">End Span</button>
9+
<button id="btnStartNestedSpan">Start Nested Span</button>
10+
<button id="btnEndNestedSpan">End Nested Span</button>
11+
</body>
12+
</html>
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../../utils/fixtures';
3+
import {
4+
type EventAndTraceHeader,
5+
eventAndTraceHeaderRequestParser,
6+
getMultipleSentryEnvelopeRequests,
7+
shouldSkipFeatureFlagsTest,
8+
shouldSkipTracingTest,
9+
} from '../../../../../utils/helpers';
10+
import { MAX_FLAGS_PER_SPAN } from '../../constants';
11+
12+
sentryTest("Feature flags are added to active span's attributes on span end.", async ({ getLocalTestUrl, page }) => {
13+
if (shouldSkipFeatureFlagsTest() || shouldSkipTracingTest()) {
14+
sentryTest.skip();
15+
}
16+
17+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
18+
return route.fulfill({
19+
status: 200,
20+
contentType: 'application/json',
21+
body: JSON.stringify({}),
22+
});
23+
});
24+
25+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
26+
await page.goto(url);
27+
28+
const envelopeRequestPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>(
29+
page,
30+
1,
31+
{},
32+
eventAndTraceHeaderRequestParser,
33+
);
34+
35+
// withNestedSpans is a util used to start 3 nested spans: root-span (not recorded in transaction_event.spans), span, and nested-span.
36+
await page.evaluate(maxFlags => {
37+
(window as any).withNestedSpans(() => {
38+
const ldClient = (window as any).initializeLD();
39+
for (let i = 1; i <= maxFlags; i++) {
40+
ldClient.variation(`feat${i}`, false);
41+
}
42+
ldClient.variation(`feat${maxFlags + 1}`, true); // dropped
43+
ldClient.variation('feat3', true); // update
44+
});
45+
return true;
46+
}, MAX_FLAGS_PER_SPAN);
47+
48+
const event = (await envelopeRequestPromise)[0][0];
49+
const innerSpan = event.spans?.[0];
50+
const outerSpan = event.spans?.[1];
51+
const outerSpanFlags = Object.entries(outerSpan?.data ?? {}).filter(([key, _val]) =>
52+
key.startsWith('flag.evaluation'),
53+
);
54+
const innerSpanFlags = Object.entries(innerSpan?.data ?? {}).filter(([key, _val]) =>
55+
key.startsWith('flag.evaluation'),
56+
);
57+
58+
expect(innerSpanFlags).toEqual([]);
59+
60+
const expectedOuterSpanFlags = [];
61+
for (let i = 1; i <= MAX_FLAGS_PER_SPAN; i++) {
62+
expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, i === 3]);
63+
}
64+
// Order agnostic (attribute dict is unordered).
65+
expect(outerSpanFlags.sort()).toEqual(expectedOuterSpanFlags.sort());
66+
});

dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/basic/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/onError/basic/test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
2-
import { sentryTest } from '../../../../../utils/fixtures';
3-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
4-
import { FLAG_BUFFER_SIZE } from '../../constants';
2+
import { sentryTest } from '../../../../../../utils/fixtures';
3+
import {
4+
envelopeRequestParser,
5+
shouldSkipFeatureFlagsTest,
6+
waitForErrorRequest,
7+
} from '../../../../../../utils/helpers';
8+
import { FLAG_BUFFER_SIZE } from '../../../constants';
59

610
sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {

dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/errorHook/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/onError/errorHook/test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
2-
import { sentryTest } from '../../../../../utils/fixtures';
3-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
4-
import { FLAG_BUFFER_SIZE } from '../../constants';
2+
import { sentryTest } from '../../../../../../utils/fixtures';
3+
import {
4+
envelopeRequestParser,
5+
shouldSkipFeatureFlagsTest,
6+
waitForErrorRequest,
7+
} from '../../../../../../utils/helpers';
8+
import { FLAG_BUFFER_SIZE } from '../../../constants';
59

610
sentryTest('Flag evaluation error hook', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {

dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/withScope/test.ts renamed to dev-packages/browser-integration-tests/suites/integrations/featureFlags/openfeature/onError/withScope/test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from '@playwright/test';
22
import type { Scope } from '@sentry/browser';
3-
import { sentryTest } from '../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers';
3+
import { sentryTest } from '../../../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipFeatureFlagsTest,
7+
waitForErrorRequest,
8+
} from '../../../../../../utils/helpers';
59

610
sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ getLocalTestUrl, page }) => {
711
if (shouldSkipFeatureFlagsTest()) {
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
window.sentryOpenFeatureIntegration = Sentry.openFeatureIntegration();
5+
6+
Sentry.init({
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
sampleRate: 1.0,
9+
tracesSampleRate: 1.0,
10+
integrations: [
11+
window.sentryOpenFeatureIntegration,
12+
Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false }),
13+
],
14+
});
15+
16+
window.initialize = () => {
17+
return {
18+
getBooleanValue(flag, value) {
19+
let hook = new Sentry.OpenFeatureIntegrationHook();
20+
hook.after(null, { flagKey: flag, value: value });
21+
return value;
22+
},
23+
};
24+
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
const btnStartSpan = document.getElementById('btnStartSpan');
2+
const btnEndSpan = document.getElementById('btnEndSpan');
3+
const btnStartNestedSpan = document.getElementById('btnStartNestedSpan');
4+
const btnEndNestedSpan = document.getElementById('btnEndNestedSpan');
5+
6+
window.withNestedSpans = callback => {
7+
window.Sentry.startSpan({ name: 'test-root-span' }, rootSpan => {
8+
window.traceId = rootSpan.spanContext().traceId;
9+
10+
window.Sentry.startSpan({ name: 'test-span' }, _span => {
11+
window.Sentry.startSpan({ name: 'test-nested-span' }, _nestedSpan => {
12+
callback();
13+
});
14+
});
15+
});
16+
};

0 commit comments

Comments
 (0)