-
-
Notifications
You must be signed in to change notification settings - Fork 684
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: optimize applications overview query #6883
Changes from 6 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ import type { Db } from './db'; | |
import type { IApplicationOverview } from '../features/metrics/instance/models'; | ||
import { applySearchFilters } from '../features/feature-search/search-utils'; | ||
import type { IFlagResolver } from '../types'; | ||
import metricsHelper from '../util/metrics-helper'; | ||
import { DB_TIME } from '../metric-events'; | ||
|
||
const COLUMNS = [ | ||
'app_name', | ||
|
@@ -118,6 +120,8 @@ export default class ClientApplicationsStore | |
|
||
private logger: Logger; | ||
|
||
private timer: Function; | ||
|
||
private flagResolver: IFlagResolver; | ||
|
||
constructor( | ||
|
@@ -129,6 +133,11 @@ export default class ClientApplicationsStore | |
this.db = db; | ||
this.flagResolver = flagResolver; | ||
this.logger = getLogger('client-applications-store.ts'); | ||
this.timer = (action: string) => | ||
metricsHelper.wrapTimer(eventBus, DB_TIME, { | ||
store: 'client-applications', | ||
action, | ||
}); | ||
} | ||
|
||
async upsert(details: Partial<IClientApplication>): Promise<void> { | ||
|
@@ -292,6 +301,60 @@ export default class ClientApplicationsStore | |
async getApplicationOverview( | ||
appName: string, | ||
): Promise<IApplicationOverview> { | ||
if (!this.flagResolver.isEnabled('applicationOverviewNewQuery')) { | ||
return this.getOldApplicationOverview(appName); | ||
} | ||
const stopTimer = this.timer('getApplicationOverview'); | ||
const query = this.db | ||
.with('metrics', (qb) => { | ||
qb.select([ | ||
'cme.app_name', | ||
'cme.environment', | ||
'f.project', | ||
this.db.raw( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Group features into array, grouped by environment. |
||
'array_agg(DISTINCT cme.feature_name) as features', | ||
), | ||
]) | ||
.from('client_metrics_env as cme') | ||
.leftJoin('features as f', 'f.name', 'cme.feature_name') | ||
.groupBy('cme.app_name', 'cme.environment', 'f.project'); | ||
}) | ||
.select([ | ||
'm.project', | ||
'm.environment', | ||
'm.features', | ||
'ci.instance_id', | ||
'ci.sdk_version', | ||
'ci.last_seen', | ||
'a.strategies', | ||
]) | ||
.from({ a: 'client_applications' }) | ||
.leftJoin('metrics as m', 'm.app_name', 'a.app_name') | ||
.leftJoin('client_instances as ci', function () { | ||
this.on('ci.app_name', '=', 'm.app_name').andOn( | ||
'ci.environment', | ||
'=', | ||
'm.environment', | ||
); | ||
}) | ||
.where('a.app_name', appName) | ||
.orderBy('m.environment', 'asc'); | ||
const rows = await query; | ||
stopTimer(); | ||
if (!rows.length) { | ||
throw new NotFoundError(`Could not find appName=${appName}`); | ||
} | ||
const existingStrategies: string[] = await this.db | ||
.select('name') | ||
.from('strategies') | ||
.pluck('name'); | ||
return this.mapApplicationOverviewData(rows, existingStrategies); | ||
} | ||
|
||
async getOldApplicationOverview( | ||
appName: string, | ||
): Promise<IApplicationOverview> { | ||
const stopTimer = this.timer('getApplicationOverviewOld'); | ||
const query = this.db | ||
.with('metrics', (qb) => { | ||
qb.distinct( | ||
|
@@ -322,6 +385,7 @@ export default class ClientApplicationsStore | |
.where('a.app_name', appName) | ||
.orderBy('cme.environment', 'asc'); | ||
const rows = await query; | ||
stopTimer(); | ||
if (!rows.length) { | ||
throw new NotFoundError(`Could not find appName=${appName}`); | ||
} | ||
|
@@ -335,6 +399,96 @@ export default class ClientApplicationsStore | |
mapApplicationOverviewData( | ||
rows: any[], | ||
existingStrategies: string[], | ||
): IApplicationOverview { | ||
if (!this.flagResolver.isEnabled('applicationOverviewNewQuery')) { | ||
return this.mapOldApplicationOverviewData(rows, existingStrategies); | ||
} | ||
const featureCount = new Set(rows.flatMap((row) => row.features)).size; | ||
const missingStrategies: Set<string> = new Set(); | ||
|
||
const environments = rows.reduce((acc, row) => { | ||
const { | ||
environment, | ||
instance_id, | ||
sdk_version, | ||
last_seen, | ||
project, | ||
features, | ||
strategies, | ||
} = row; | ||
|
||
if (!environment) return acc; | ||
|
||
strategies.forEach((strategy) => { | ||
if ( | ||
!DEPRECATED_STRATEGIES.includes(strategy) && | ||
!existingStrategies.includes(strategy) | ||
) { | ||
missingStrategies.add(strategy); | ||
} | ||
}); | ||
|
||
const featuresNotMappedToProject = !project; | ||
|
||
let env = acc.find((e) => e.name === environment); | ||
if (!env) { | ||
env = { | ||
name: environment, | ||
instanceCount: instance_id ? 1 : 0, | ||
sdks: sdk_version ? [sdk_version] : [], | ||
lastSeen: last_seen, | ||
uniqueInstanceIds: new Set( | ||
instance_id ? [instance_id] : [], | ||
), | ||
issues: { | ||
missingFeatures: featuresNotMappedToProject | ||
? features | ||
: [], | ||
}, | ||
}; | ||
acc.push(env); | ||
} else { | ||
if (instance_id) { | ||
env.uniqueInstanceIds.add(instance_id); | ||
env.instanceCount = env.uniqueInstanceIds.size; | ||
} | ||
if (featuresNotMappedToProject) { | ||
env.issues.missingFeatures = features; | ||
} | ||
if (sdk_version && !env.sdks.includes(sdk_version)) { | ||
env.sdks.push(sdk_version); | ||
} | ||
if (new Date(last_seen) > new Date(env.lastSeen)) { | ||
env.lastSeen = last_seen; | ||
} | ||
} | ||
|
||
return acc; | ||
}, []); | ||
environments.forEach((env) => { | ||
delete env.uniqueInstanceIds; | ||
env.sdks.sort(); | ||
}); | ||
|
||
return { | ||
projects: [ | ||
...new Set( | ||
rows | ||
.filter((row) => row.project != null) | ||
.map((row) => row.project), | ||
), | ||
], | ||
featureCount, | ||
environments, | ||
issues: { | ||
missingStrategies: [...missingStrategies], | ||
}, | ||
}; | ||
} | ||
Comment on lines
+402
to
+487
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Getting better: Complex Method |
||
|
||
private mapOldApplicationOverviewData( | ||
rows: any[], | ||
existingStrategies: string[], | ||
Comment on lines
+489
to
+491
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ New issue: Complex Method |
||
): IApplicationOverview { | ||
const featureCount = new Set(rows.map((row) => row.feature_name)).size; | ||
const missingStrategies: Set<string> = new Set(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: String Heavy Function Arguments
In this module, 39.1% of all arguments to its 21 functions are strings. The threshold for string arguments is 39.0%
Suppress