-
-
Notifications
You must be signed in to change notification settings - Fork 669
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
'cme.feature_name', | ||
).from('client_metrics_env as cme'); | ||
'f.project', | ||
this.db.raw( |
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.
Group features into array, grouped by environment.
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.
✅ Code Health Quality Gates: OK
- Improving Code Health: 1 findings(s) ✅
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.
✅ Code Health Quality Gates: OK
- Improving Code Health: 1 findings(s) ✅
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 2 findings(s) 🚩
- Improving Code Health: 1 findings(s) ✅
): 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], | ||
}, | ||
}; | ||
} |
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.
✅ Getting better: Complex Method
ClientApplicationsStore.mapApplicationOverviewData decreases in cyclomatic complexity from 16 to 15, threshold = 9
private mapOldApplicationOverviewData( | ||
rows: any[], | ||
existingStrategies: string[], |
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: Complex Method
ClientApplicationsStore.mapOldApplicationOverviewData has a cyclomatic complexity of 16, threshold = 9
@@ -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'; |
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%
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 2 findings(s) 🚩
- Improving Code Health: 1 findings(s) ✅
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 2 findings(s) 🚩
- Improving Code Health: 1 findings(s) ✅
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 2 findings(s) 🚩
- Improving Code Health: 1 findings(s) ✅
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.
LGTM
We encountered an issue with a customer because this query was returning 3 million rows. The problem arose from each instance reporting approximately 100 features, with a total of 30,000 instances. The query was joining these, thus multiplying the data. This approach was fine for a reasonable number of instances, but in this extreme case, it did not perform well.
This PR modifies the logic; instead of performing outright joins, we are now grouping features by environment into an array, resulting in just one row returned per instance.
I tested locally with the same dataset. Previously, loading this large instance took about 21 seconds; now it has reduced to 2 seconds. Although this is still significant, the dataset is extensive.