Skip to content

Commit 7c2c161

Browse files
aliu39aliu39
committed
Fix global type, add unit tests, add generic ffs test
1 parent ffe9bcd commit 7c2c161

File tree

19 files changed

+211
-16
lines changed

19 files changed

+211
-16
lines changed

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/constants.ts

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/onError/basic/test.ts

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

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

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

Lines changed: 5 additions & 1 deletion
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';
33
import { sentryTest } from '../../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../../utils/helpers';
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()) {

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/init.js

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://[email protected]/1337',
7+
sampleRate: 1.0,
8+
tracesSampleRate: 1.0,
9+
integrations: [
10+
Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false }),
11+
Sentry.featureFlagsIntegration(),
12+
],
13+
});

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/subject.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
};
17+
18+
// btnStartNestedSpan.addEventListener('click', () => {
19+
// Sentry.startSpanManual(
20+
// { name: 'test-nested-span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } },
21+
// async span => {
22+
// await new Promise(resolve => {
23+
// btnEndNestedSpan.addEventListener('click', resolve);
24+
// });
25+
// span.end();
26+
// },
27+
// );
28+
// });

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/template.html

Lines changed: 12 additions & 0 deletions
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>

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/onSpan/test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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+
{}, // envelopeType: 'transaction' },
32+
eventAndTraceHeaderRequestParser, // properFullEnvelopeRequestParser
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+
const event = (await envelopeRequestPromise)[0][0];
48+
const innerSpan = event.spans?.[0];
49+
const outerSpan = event.spans?.[1];
50+
const outerSpanFlags = Object.entries(outerSpan?.data ?? {}).filter(([key, _val]) =>
51+
key.startsWith('flag.evaluation'),
52+
);
53+
const innerSpanFlags = Object.entries(innerSpan?.data ?? {}).filter(([key, _val]) =>
54+
key.startsWith('flag.evaluation'),
55+
);
56+
57+
expect(innerSpanFlags).toEqual([]);
58+
59+
const expectedOuterSpanFlags = [];
60+
for (let i = 1; i <= 2; i++) {
61+
expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
62+
}
63+
for (let i = 4; i <= MAX_FLAGS_PER_SPAN; i++) {
64+
expectedOuterSpanFlags.push([`flag.evaluation.feat${i}`, false]);
65+
}
66+
expectedOuterSpanFlags.push(['flag.evaluation.feat3', true]);
67+
expect(outerSpanFlags).toEqual(expectedOuterSpanFlags);
68+
});

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

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

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

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

Lines changed: 5 additions & 1 deletion
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';
33
import { sentryTest } from '../../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../../utils/helpers';
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()) {

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

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

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

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

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

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

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

Lines changed: 5 additions & 1 deletion
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';
33
import { sentryTest } from '../../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../../utils/helpers';
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()) {

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/statsig/onError/basic/test.ts

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

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

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/statsig/onError/withScope/test.ts

Lines changed: 5 additions & 1 deletion
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';
33
import { sentryTest } from '../../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../../utils/helpers';
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()) {

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/onError/basic/test.ts

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

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

‎dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/onError/withScope/test.ts

Lines changed: 5 additions & 1 deletion
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';
33
import { sentryTest } from '../../../../../../utils/fixtures';
4-
import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../../utils/helpers';
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()) {

‎packages/browser/src/utils/featureFlags.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function copyFlagsFromScopeToEvent(event: Event): Event {
5252
* @param name Name of the feature flag to insert.
5353
* @param value Value of the feature flag.
5454
* @param maxSize Max number of flags the buffer should store. Default value should always be used in production.
55-
*/
55+
*/
5656
export function insertFlagToScope(name: string, value: unknown, maxSize: number = FLAG_BUFFER_SIZE): void {
5757
const scopeContexts = getCurrentScope().getScopeData().contexts;
5858
if (!scopeContexts.flags) {
@@ -75,7 +75,13 @@ export function insertFlagToScope(name: string, value: unknown, maxSize: number
7575
* @param maxSize Max number of flags the buffer should store. Default value should always be used in production.
7676
* @param allowEviction If true, the oldest flag is evicted when the buffer is full. Otherwise the new flag is dropped.
7777
*/
78-
export function insertToFlagBuffer(flags: FeatureFlag[], name: string, value: unknown, maxSize: number, allowEviction: boolean = true): void {
78+
export function insertToFlagBuffer(
79+
flags: FeatureFlag[],
80+
name: string,
81+
value: unknown,
82+
maxSize: number,
83+
allowEviction: boolean = true,
84+
): void {
7985
if (typeof value !== 'boolean') {
8086
return;
8187
}
@@ -98,6 +104,7 @@ export function insertToFlagBuffer(flags: FeatureFlag[], name: string, value: un
98104
// If at capacity, pop the earliest flag - O(n)
99105
flags.shift();
100106
} else {
107+
101108
return;
102109
}
103110
}
@@ -143,6 +150,8 @@ export function bufferSpanFeatureFlag(
143150
export function freezeSpanFeatureFlags(span: Span): void {
144151
const flags = GLOBAL_OBJ._spanToFlagBufferMap?.get(span);
145152
if (flags) {
146-
span.setAttributes(Object.fromEntries(flags.map(flag => [`${SPAN_FLAG_ATTRIBUTE_PREFIX}${flag.flag}`, flag.result])));
153+
span.setAttributes(
154+
Object.fromEntries(flags.map(flag => [`${SPAN_FLAG_ATTRIBUTE_PREFIX}${flag.flag}`, flag.result])),
155+
);
147156
}
148157
}

‎packages/browser/test/utils/featureFlags.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,25 @@ describe('flags', () => {
5959
]);
6060
});
6161

62+
it('drops new entries when allowEviction is false and buffer is full', () => {
63+
const buffer: FeatureFlag[] = [];
64+
const maxSize = 0;
65+
insertToFlagBuffer(buffer, 'feat1', true, maxSize, false);
66+
insertToFlagBuffer(buffer, 'feat2', true, maxSize, false);
67+
insertToFlagBuffer(buffer, 'feat3', true, maxSize, false);
68+
69+
expect(buffer).toEqual([]);
70+
});
71+
72+
it('still updates order and values when allowEviction is false and buffer is full', () => {
73+
const buffer: FeatureFlag[] = [];
74+
const maxSize = 1;
75+
insertToFlagBuffer(buffer, 'feat1', false, maxSize, false);
76+
insertToFlagBuffer(buffer, 'feat1', true, maxSize, false);
77+
78+
expect(buffer).toEqual([{ flag: 'feat1', result: true }]);
79+
});
80+
6281
it('does not allocate unnecessary space', () => {
6382
const buffer: FeatureFlag[] = [];
6483
const maxSize = 1000;

‎packages/core/src/utils-hoist/worldwide.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export type InternalGlobal = {
6161
/**
6262
* A map of spans to feature flag buffers. Populated by feature flag integrations.
6363
*/
64-
_spanToFlagBufferMap?: WeakMap<Span, Set<FeatureFlag>>;
64+
_spanToFlagBufferMap?: WeakMap<Span, FeatureFlag[]>;
6565
} & Carrier;
6666

6767
/** Get's the global object for the current JavaScript runtime */

0 commit comments

Comments
 (0)