Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-07-26 13:48:33 +02:00
Code Issues Projects Releases Wiki Activity

feat: include segment usage in CRs when showing usage in projects and flags (#5327)

Browse Source 
This PR updates the segment usage counting to also include segment usage
in pending change requests.

The changes include:
- Updating the schema to explicitly call out that change request usage
is included.
- Adding two tests to verify the new features
- Writing an alternate query to count this data

Specifically, it'll update the part of the UI that tells you how many
places a segment is used:


![image](https://github.com/Unleash/unleash/assets/17786332/a77cf932-d735-4a13-ae43-a2840f7106cb)

## Implementation

Implementing this was a little tricky. Previously, we'd just count
distinct instances of feature names and project names on the
feature_strategy table. However, to merge this with change request data,
we can't just count existing usage and change request usage separately,
because that could cause duplicates.

Instead of turning this into a complex DB query, I've broken it up into
a few separate queries and done the merging in JS. I think that's more
readable and it was easier to reason about.

Here's the breakdown:
1. Get the list of pending change requests. We need their IDs and their
project.
2. Get the list of updateStrategy and addStrategy events that have
segment data.
3. Take the result from step 2 and turn it into a dictionary of segment
id to usage data.
4. Query the feature_strategy_segment and feature_strategies table, to
get existing segment usage data
5. Fold that data into the change request data.
6. Perform the preexisting segment query (without counting logic) to get
other segment data
7. Enrich the results of the query from step 2 with usage data.

## Discussion points

I feel like this could be done in a nicer way, so any ideas on how to
achieve that (whether that's as a db query or just breaking up the code
differently) is very welcome.

Second, using multiple queries obviously yields more overhead than just
a single one. However, I do not think this is in the hot path, so I
don't consider performance to be critical here, but I'm open to hearing
opposing thoughts on this of course.
This commit is contained in:
Thomas Heartman 2023-11-14 08:49:32 +01:00 committed by GitHub
parent ddd718fd23
commit a115f89183
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 335 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

186
src/lib/db/segment-store.test.ts
View File

@ -8,7 +8,13 @@ let db;
let segmentStore: ISegmentStore;
beforeAll(async () => {
db = await dbInit('segment_store_serial', getLogger);
db = await dbInit('segment_store_serial', getLogger, {
experimental: {
flags: {
detectSegmentUsageInChangeRequests: true,
},
},
});
stores = db.stores;
segmentStore = stores.segmentStore;
});
@ -28,3 +34,181 @@ describe('unexpected input handling for get segment', () => {
}
});
});
describe('usage counting', () => {
let user;
beforeAll(async () => {
user = await db.stores.userStore.insert({
username: 'test',
});
});
afterEach(async () => {
await db.stores.featureToggleStore.deleteAll();
await db.stores.segmentStore.deleteAll();
await db.rawDatabase.table('change_requests').delete();
});
test('segment usage in active CRs is also counted', async () => {
const CR_ID = 54321;
const flag1 = await db.stores.featureToggleStore.create('default', {
name: 'test',
});
const flag2 = await db.stores.featureToggleStore.create('default', {
name: 'test2',
});
const segment = await segmentStore.create(
{
name: 'cr-segment',
constraints: [],
createdAt: new Date(),
},
user,
);
await db.rawDatabase.table('change_requests').insert({
id: CR_ID,
environment: 'default',
state: 'In Review',
project: 'default',
created_by: user.id,
created_at: '2023-01-01 00:00:00',
min_approvals: 1,
title: 'My change request',
});
await db.rawDatabase.table('change_request_events').insert({
feature: flag1.name,
action: 'addStrategy',
payload: {
name: 'flexibleRollout',
title: '',
disabled: false,
segments: [segment.id],
variants: [],
parameters: {
groupId: flag1.name,
rollout: '100',
stickiness: 'default',
},
constraints: [],
},
created_at: '2023-01-01 00:01:00',
change_request_id: CR_ID,
created_by: user.id,
});
await db.rawDatabase.table('change_request_events').insert({
feature: flag2.name,
action: 'updateStrategy',
payload: {
strategyId: 'not-a-real-strategy-id',
name: 'flexibleRollout',
title: '',
disabled: false,
segments: [segment.id],
variants: [],
parameters: {
groupId: flag2.name,
rollout: '100',
stickiness: 'default',
},
constraints: [],
},
created_at: '2023-01-01 00:01:00',
change_request_id: CR_ID,
created_by: user.id,
});
const [storedSegment] = await segmentStore.getAll();
expect(storedSegment.usedInFeatures).toBe(2);
expect(storedSegment.usedInProjects).toBe(1);
});
test('Segment usage is only counted once per feature', async () => {
// if the segment is used on a feature, but there is also a
// change request updateStrategy event for the same feature, the
// feature should only be counted once
const CR_ID = 54321;
const flag = await db.stores.featureToggleStore.create('default', {
name: 'test',
});
const segment1 = await segmentStore.create(
{
name: 'cr-segment',
constraints: [],
createdAt: new Date(),
},
user,
);
const segment2 = await segmentStore.create(
{
name: 'cr-segment-2',
constraints: [],
createdAt: new Date(),
},
user,
);
const strategy =
await stores.featureStrategiesStore.createStrategyFeatureEnv({
featureName: flag.name,
projectId: 'default',
environment: 'default',
strategyName: 'flexibleRollout',
segments: [segment1.id],
parameters: {
groupId: flag.name,
rollout: '100',
stickiness: 'default',
},
});
await db.rawDatabase.table('change_requests').insert({
id: CR_ID,
environment: 'default',
state: 'In Review',
project: 'default',
created_by: user.id,
created_at: '2023-01-01 00:00:00',
min_approvals: 1,
title: 'My change request',
});
await db.rawDatabase.table('change_request_events').insert({
feature: flag.name,
action: 'updateStrategy',
payload: {
strategyId: strategy.id,
name: 'flexibleRollout',
title: '',
disabled: false,
segments: [segment1.id, segment2.id],
variants: [],
parameters: {
groupId: flag.name,
rollout: '100',
stickiness: 'default',
},
constraints: [],
},
created_at: '2023-01-01 00:01:00',
change_request_id: CR_ID,
created_by: user.id,
});
const storedSegments = await segmentStore.getAll();
expect(storedSegments).toMatchObject([
{ usedInFeatures: 1, usedInProjects: 1 },
{ usedInFeatures: 1, usedInProjects: 1 },
]);
});
});

161
src/lib/db/segment-store.ts
View File

@ -112,33 +112,142 @@ export default class SegmentStore implements ISegmentStore {
}
async getAll(): Promise<ISegment[]> {
const rows: ISegmentRow[] = await this.db
.select(
this.prefixColumns(),
'used_in_projects',
'used_in_features',
)
.countDistinct(
`${T.featureStrategies}.project_name AS used_in_projects`,
)
.countDistinct(
`${T.featureStrategies}.feature_name AS used_in_features`,
)
.from(T.segments)
.leftJoin(
T.featureStrategySegment,
`${T.segments}.id`,
`${T.featureStrategySegment}.segment_id`,
)
.leftJoin(
T.featureStrategies,
`${T.featureStrategies}.id`,
`${T.featureStrategySegment}.feature_strategy_id`,
)
.groupBy(this.prefixColumns())
.orderBy('name', 'asc');
if (this.flagResolver.isEnabled('detectSegmentUsageInChangeRequests')) {
const pendingCRs = await this.db
.select('id', 'project')
.from('change_requests')
.whereNotIn('state', ['Applied', 'Rejected', 'Cancelled']);
return rows.map(this.mapRow);
const pendingChangeRequestIds = pendingCRs.map((cr) => cr.id);
const crFeatures = await this.db
.select(
'payload',
'feature',
'change_request_id as changeRequestId',
)
.from('change_request_events')
.whereIn('change_request_id', pendingChangeRequestIds)
.whereIn('action', ['addStrategy', 'updateStrategy'])
.andWhereRaw("jsonb_array_length(payload -> 'segments') > 0");
const changeRequestToProjectMap = pendingCRs.reduce(
(acc, { id, project }) => {
acc[id] = project;
return acc;
},
{},
);
const combinedUsageData = crFeatures.reduce((acc, segmentEvent) => {
const { payload, changeRequestId, feature } = segmentEvent;
const project = changeRequestToProjectMap[changeRequestId];
for (const segmentId of payload.segments) {
const existingData = acc[segmentId];
if (existingData) {
acc[segmentId] = {
features: existingData.features.add(feature),
projects: existingData.projects.add(project),
};
} else {
acc[segmentId] = {
features: new Set([feature]),
projects: new Set([project]),
};
}
}
return acc;
}, {});
const currentSegmentUsage = await this.db
.select(
`${T.featureStrategies}.feature_name as featureName`,
`${T.featureStrategies}.project_name as projectName`,
'segment_id as segmentId',
)
.from(T.featureStrategySegment)
.leftJoin(
T.featureStrategies,
`${T.featureStrategies}.id`,
`${T.featureStrategySegment}.feature_strategy_id`,
);
currentSegmentUsage.forEach(
({ segmentId, featureName, projectName }) => {
const usage = combinedUsageData[segmentId];
if (usage) {
combinedUsageData[segmentId] = {
features: usage.features.add(featureName),
projects: usage.projects.add(projectName),
};
} else {
combinedUsageData[segmentId] = {
features: new Set([featureName]),
projects: new Set([projectName]),
};
}
},
);
const rows: ISegmentRow[] = await this.db
.select(this.prefixColumns())
.from(T.segments)
.leftJoin(
T.featureStrategySegment,
`${T.segments}.id`,
`${T.featureStrategySegment}.segment_id`,
)
.groupBy(this.prefixColumns())
.orderBy('name', 'asc');
const rowsWithUsageData: ISegmentRow[] = rows.map((row) => {
const usageData = combinedUsageData[row.id];
if (usageData) {
return {
...row,
used_in_features: usageData.features.size,
used_in_projects: usageData.projects.size,
};
} else {
return {
...row,
used_in_features: 0,
used_in_projects: 0,
};
}
});
return rowsWithUsageData.map(this.mapRow);
} else {
const rows: ISegmentRow[] = await this.db
.select(
this.prefixColumns(),
'used_in_projects',
'used_in_features',
)
.countDistinct(
`${T.featureStrategies}.project_name AS used_in_projects`,
)
.countDistinct(
`${T.featureStrategies}.feature_name AS used_in_features`,
)
.from(T.segments)
.leftJoin(
T.featureStrategySegment,
`${T.segments}.id`,
`${T.featureStrategySegment}.segment_id`,
)
.leftJoin(
T.featureStrategies,
`${T.featureStrategies}.id`,
`${T.featureStrategySegment}.feature_strategy_id`,
)
.groupBy(this.prefixColumns())
.orderBy('name', 'asc');
return rows.map(this.mapRow);
}
}
async getActive(): Promise<ISegment[]> {

6
src/lib/openapi/spec/admin-segment-schema.ts
View File

@ -37,14 +37,16 @@ export const adminSegmentSchema = {
usedInFeatures: {
type: 'integer',
minimum: 0,
description: 'The number of projects that use this segment',
description:
'The number of feature flags that use this segment. The number also includes the any flags with pending change requests that would add this segment.',
example: 3,
nullable: true,
},
usedInProjects: {
type: 'integer',
minimum: 0,
description: 'The number of projects that use this segment',
description:
'The number of projects that use this segment. The number includes any projects with pending change requests that would add this segment.',
example: 2,
nullable: true,
},

12
src/test/e2e/api/client/segment.e2e.test.ts
View File

@ -183,7 +183,17 @@ const createTestSegments = async () => {
beforeAll(async () => {
db = await dbInit('segments', getLogger);
app = await setupAppWithCustomConfig(db.stores, {}, db.rawDatabase);
app = await setupAppWithCustomConfig(
db.stores,
{
experimental: {
flags: {
detectSegmentUsageInChangeRequests: true,
},
},
},
db.rawDatabase,
);
});
afterAll(async () => {