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.

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: statistics for orphaned tokens #7568

Merged
merged 10 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion src/lib/__snapshots__/create-config.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ exports[`should create default config 1`] = `
"flagResolver": FlagResolver {
"experiments": {
"adminTokenKillSwitch": false,
"allowOrphanedWildcardTokens": false,
"allowOrphanedWildcardTokens": true,
"anonymiseEventLog": false,
"anonymizeProjectOwners": false,
"automatedActions": false,
Expand Down
42 changes: 41 additions & 1 deletion src/lib/db/api-token-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ const createTokenRowReducer =
if (
!allowOrphanedWildcardTokens &&
!tokenRow.project &&
!tokenRow.secret.startsWith('*:') &&
tokenRow.secret.includes(':') && // Exclude v1 tokens
!tokenRow.secret.startsWith('*:') && // Exclude intentionally wildcard
(tokenRow.type === ApiTokenType.CLIENT ||
tokenRow.type === ApiTokenType.FRONTEND)
) {
Expand Down Expand Up @@ -270,4 +271,43 @@ export class ApiTokenStore implements IApiTokenStore {
this.logger.error('Could not update lastSeen, error: ', err);
}
}

async countDeprecatedTokens(): Promise<{
orphanedTokens: number;
activeOrphanedTokens: number;
legacyTokens: number;
activeLegacyTokens: number;
}> {
const allLegacy = await this.makeTokenProjectQuery()
.whereNull('token_project_link.project')
.andWhere('tokens.secret', 'NOT LIKE', '%:%');

const activeLegacy = await this.makeTokenProjectQuery()
Tymek marked this conversation as resolved.
Show resolved Hide resolved
.whereNull('token_project_link.project')
.andWhere('tokens.secret', 'NOT LIKE', '%:%')
.andWhereRaw("tokens.seen_at > NOW() - INTERVAL '3 MONTH'");

const orphanedTokensQuery = this.makeTokenProjectQuery()
.whereNull('token_project_link.project')
.andWhere('tokens.secret', 'NOT LIKE', '*:%') // Exclude intentionally wildcard tokens
.andWhere('tokens.secret', 'LIKE', '%:%') // Exclude legacy tokens
.andWhere((builder) => {
builder
.where('tokens.type', ApiTokenType.CLIENT)
.orWhere('tokens.type', ApiTokenType.FRONTEND);
});

const allOrphaned = await orphanedTokensQuery.clone();

const activeOrphaned = await orphanedTokensQuery
.clone()
.andWhereRaw("tokens.seen_at > NOW() - INTERVAL '3 MONTH'");

return {
orphanedTokens: allOrphaned.length,
Tymek marked this conversation as resolved.
Show resolved Hide resolved
activeOrphanedTokens: activeOrphaned.length,
legacyTokens: allLegacy.length,
activeLegacyTokens: activeLegacy.length,
};
}
}
9 changes: 9 additions & 0 deletions src/lib/features/instance-stats/instance-stats-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ export interface InstanceStats {
enabledCount: number;
variantCount: number;
};
deprecatedTokens: {
orphanedTokens: number;
activeOrphanedTokens: number;
legacyTokens: number;
activeLegacyTokens: number;
};
}

export type InstanceStatsSigned = Omit<InstanceStats, 'projects'> & {
Expand Down Expand Up @@ -250,6 +256,7 @@ export class InstanceStatsService {
featureImports,
productionChanges,
previousDayMetricsBucketsCount,
deprecatedTokens,
] = await Promise.all([
this.getToggleCount(),
this.getArchivedToggleCount(),
Expand All @@ -273,6 +280,7 @@ export class InstanceStatsService {
this.eventStore.filteredCount({ type: FEATURES_IMPORTED }),
this.getProductionChanges(),
this.clientMetricsStore.countPreviousDayHourlyMetricsBuckets(),
this.apiTokenStore.countDeprecatedTokens(),
]);

return {
Expand Down Expand Up @@ -305,6 +313,7 @@ export class InstanceStatsService {
featureImports,
productionChanges,
previousDayMetricsBucketsCount,
deprecatedTokens,
Tymek marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
4 changes: 3 additions & 1 deletion src/lib/features/project/project-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ beforeAll(async () => {
const config = createTestConfig({
getLogger,
experimental: {
flags: {},
flags: {
cleanApiTokenWhenOrphaned: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

That's fun. Killswitch being disabled by default was masking that tests related to other flag don't pass.

},
},
});
eventService = new EventService(stores, config);
Expand Down
36 changes: 36 additions & 0 deletions src/lib/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,26 @@ export default class MetricsMonitor {
labelNames: ['project_id'],
});

const orphanedTokensTotal = createGauge({
name: 'orphaned_api_tokens_total',
help: 'Number of API tokens without a project',
});

const orphanedTokensActive = createGauge({
name: 'orphaned_api_tokens_active',
help: 'Number of API tokens without a project, last seen within 3 months',
});

const legacyTokensTotal = createGauge({
name: 'legacy_api_tokens_total',
help: 'Number of API tokens with v1 format',
});

const legacyTokensActive = createGauge({
name: 'legacy_api_tokens_active',
help: 'Number of API tokens with v1 format, last seen within 3 months',
});

async function collectStaticCounters() {
try {
const stats = await instanceStatsService.getStats();
Expand Down Expand Up @@ -394,6 +414,22 @@ export default class MetricsMonitor {
apiTokens.labels({ type }).set(value);
}

orphanedTokensTotal.reset();
orphanedTokensTotal.set(stats.deprecatedTokens.orphanedTokens);

orphanedTokensActive.reset();
orphanedTokensActive.set(
stats.deprecatedTokens.activeOrphanedTokens,
);

legacyTokensTotal.reset();
legacyTokensTotal.set(stats.deprecatedTokens.legacyTokens);

legacyTokensActive.reset();
legacyTokensActive.set(
stats.deprecatedTokens.activeLegacyTokens,
);

if (maxEnvironmentStrategies) {
maxFeatureEnvironmentStrategies.reset();
maxFeatureEnvironmentStrategies
Expand Down
34 changes: 34 additions & 0 deletions src/lib/openapi/spec/instance-admin-stats-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,40 @@ export const instanceAdminStatsSchema = {
example:
'b023323477abb1eb145bebf3cdb30a1c2063e3edc1f7ae474ed8ed6c80de9a3b',
},
deprecatedTokens: {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't add this info to instance stats and only to metrics.ts then this part is not needed

Copy link
Member Author

@Tymek Tymek Jul 10, 2024

Choose a reason for hiding this comment

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

👍🏻 I'll get metrics from store, in collectStaticCounters, skipping instance stats

type: 'object',
description:
'The number of active users in the last 7, 30 and 90 days',
properties: {
orphanedTokens: {
type: 'number',
description:
'The number of API tokens that had all projects deleted',
example: 5,
minimum: 0,
},
activeOrphanedTokens: {
type: 'number',
description:
'The number of orphaned tokens active in the last 90 days',
example: 2,
minimum: 0,
},
legacyTokens: {
type: 'number',
description: 'The number of API tokens in the v1 format',
example: 4,
minimum: 0,
},
activeLegacyTokens: {
type: 'number',
description:
'The number of legacy tokens active in the last 90 days',
example: 1,
minimum: 0,
},
},
},
},
components: {},
} as const;
Expand Down
6 changes: 6 additions & 0 deletions src/lib/routes/admin-api/instance-admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ class InstanceAdminController extends Controller {
variantCount: 100,
enabledCount: 200,
},
deprecatedTokens: {
orphanedTokens: 0,
activeOrphanedTokens: 0,
legacyTokens: 0,
activeLegacyTokens: 0,
},
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/types/experimental.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ const flags: IFlags = {
),
allowOrphanedWildcardTokens: parseEnvVarBoolean(
process.env.UNLEASH_ORPHANED_TOKENS_KILL_SWITCH,
false,
true,
),
extendedMetrics: parseEnvVarBoolean(
process.env.UNLEASH_EXPERIMENTAL_EXTENDED_METRICS,
Expand Down
6 changes: 6 additions & 0 deletions src/lib/types/stores/api-token-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,10 @@ export interface IApiTokenStore extends Store<IApiToken, string> {
markSeenAt(secrets: string[]): Promise<void>;
count(): Promise<number>;
countByType(): Promise<Map<string, number>>;
countDeprecatedTokens(): Promise<{
orphanedTokens: number;
activeOrphanedTokens: number;
legacyTokens: number;
activeLegacyTokens: number;
}>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,17 @@ const feature3 = 'f3.p2.token.access';

beforeAll(async () => {
db = await dbInit('feature_api_api_access_client_deletion', getLogger);
app = await setupAppWithAuth(db.stores, {}, db.rawDatabase);
app = await setupAppWithAuth(
db.stores,
{
experimental: {
flags: {
cleanApiTokenWhenOrphaned: true,
},
},
},
db.rawDatabase,
);
apiTokenService = app.services.apiTokenService;

const { featureToggleServiceV2, environmentService } = app.services;
Expand Down
2 changes: 2 additions & 0 deletions src/test/e2e/helpers/database-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ async function resetDatabase(knex) {
knex.table('tag_types').del(),
knex.table('addons').del(),
knex.table('users').del(),
knex.table('api_tokens').del(),
knex.table('api_token_project').del(),
knex
.table('reset_tokens')
.del(),
Expand Down
Loading
Loading