Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: exclude archived features in max reporting #7559

Merged
merged 3 commits into from
Jul 10, 2024
Merged

feat: exclude archived features in max reporting #7559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10876af
feat: exclude archived features in max reporting
kwasniew Jul 9, 2024
5814fc5
feat: exclude archived features in max reporting
kwasniew Jul 9, 2024
9916b18
feat: exclude archived features in max reporting
kwasniew Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/lib/features/feature-toggle/feature-strategies-read-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,23 @@ export class FeatureStrategiesReadModel implements IFeatureStrategiesReadModel {
constructor(db: Db) {
this.db = db;
}

private activeStrategies() {
return this.db('feature_strategies')
.leftJoin(
'features',
'features.name',
'feature_strategies.feature_name',
)
.where('features.archived_at', null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder: if a feature is archived and then revived, is archived_at set to null again? I know we've run into issues with this before, but I think that archived_at might have been the solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this manually and reviving brings the feature back to the reported metrics

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one test to show archived feature exclusion.

}

async getMaxFeatureEnvironmentStrategies(): Promise<{
feature: string;
environment: string;
count: number;
} | null> {
const rows = await this.db('feature_strategies')
const rows = await this.activeStrategies()
.select('feature_name', 'environment')
.count('id as strategy_count')
.groupBy('feature_name', 'environment')
Expand All @@ -32,7 +43,7 @@ export class FeatureStrategiesReadModel implements IFeatureStrategiesReadModel {
feature: string;
count: number;
} | null> {
const rows = await this.db('feature_strategies')
const rows = await this.activeStrategies()
.select('feature_name')
.count('id as strategy_count')
.groupBy('feature_name')
Expand All @@ -52,17 +63,17 @@ export class FeatureStrategiesReadModel implements IFeatureStrategiesReadModel {
environment: string;
count: number;
} | null> {
const rows = await this.db('feature_strategies')
const rows = await this.activeStrategies()
.select(
'feature_name',
'environment',
this.db.raw(
"MAX(coalesce(jsonb_array_length(constraint_value->'values'), 0)) as max_values_count",
),
)
.from(
.crossJoin(
this.db.raw(
'feature_strategies, jsonb_array_elements(constraints) AS constraint_value',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had to be rewritten to cross join after adding join to the feature table so that the syntax is correct

`jsonb_array_elements(constraints) AS constraint_value`,
),
)
.groupBy('feature_name', 'environment')
Expand All @@ -77,12 +88,13 @@ export class FeatureStrategiesReadModel implements IFeatureStrategiesReadModel {
}
: null;
}

async getMaxConstraintsPerStrategy(): Promise<{
feature: string;
environment: string;
count: number;
} | null> {
const rows = await this.db('feature_strategies')
const rows = await this.activeStrategies()
.select(
'feature_name',
'environment',
Expand Down
84 changes: 48 additions & 36 deletions src/lib/features/feature-toggle/tests/feature-toggle-strategies-store.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import dbInit, {
} from '../../../../test/e2e/helpers/database-init';
import getLogger from '../../../../test/fixtures/no-logger';
import type {
IConstraint,
IFeatureStrategiesReadModel,
IProjectStore,
IUnleashStores,
Expand Down Expand Up @@ -294,6 +295,32 @@ describe('max metrics collection', () => {
});
});

const bigConstraint = (maxValueCount: number) => {
return {
values: Array.from({ length: maxValueCount }, (_, i) =>
i.toString(),
),
operator: 'IN',
contextName: 'appName',
} as const;
};

const strategyWithConstraints = (
feature: string,
constraint: IConstraint,
) => {
return {
strategyName: 'gradualRollout',
projectId: 'default',
environment: 'default',
featureName: feature,
constraints: [constraint],

sortOrder: 0,
parameters: {},
};
};

test('Read feature with max number of constraint values', async () => {
const flagA = await featureToggleStore.create('default', {
name: randomId(),
Expand All @@ -305,48 +332,33 @@ describe('max metrics collection', () => {
createdByUserId: 9999,
});

const flagC = await featureToggleStore.create('default', {
name: randomId(),
createdByUserId: 9999,
});

const maxConstraintValuesBefore =
await featureStrategiesReadModel.getMaxConstraintValues();
expect(maxConstraintValuesBefore).toBe(null);

const maxValueCount = 100;
await featureStrategiesStore.createStrategyFeatureEnv({
strategyName: 'gradualRollout',
projectId: 'default',
environment: 'default',
featureName: flagA.name,
constraints: [
{
values: ['only one'],
operator: 'IN',
contextName: 'appName',
},
{
values: Array.from({ length: maxValueCount }, (_, i) =>
i.toString(),
),
operator: 'IN',
contextName: 'appName',
},
],
await featureStrategiesStore.createStrategyFeatureEnv(
strategyWithConstraints(flagA.name, bigConstraint(maxValueCount)),
);
await featureStrategiesStore.createStrategyFeatureEnv(
strategyWithConstraints(flagB.name, {
operator: 'IN',
contextName: 'appName',
}),
);
await featureStrategiesStore.createStrategyFeatureEnv(
strategyWithConstraints(
flagC.name,
bigConstraint(maxValueCount + 1),
),
);

sortOrder: 0,
parameters: {},
});
await featureStrategiesStore.createStrategyFeatureEnv({
strategyName: 'gradualRollout',
projectId: 'default',
environment: 'default',
featureName: flagB.name,
constraints: [
{
operator: 'IN',
contextName: 'appName',
},
],
sortOrder: 0,
parameters: {},
});
await featureToggleStore.archive(flagC.name);

const maxConstraintValues =
await featureStrategiesReadModel.getMaxConstraintValues();
Expand Down
Loading