Skip to content

Commit 2e83639

Browse files
nscuronscuro
authored
Merge pull request #6655 from nscuro/reduce-project-metrics-update-io-cost
Reduce resource footprint of project metrics updates
2 parents 341c917 + d4c7216 commit 2e83639

5 files changed

Lines changed: 1003 additions & 357 deletions

File tree

‎apiserver/src/test/java/org/dependencytrack/metrics/UpdateProjectMetricsActivityTest.java‎

Lines changed: 310 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.dependencytrack.proto.internal.workflow.v1.UpdateProjectMetricsArg;
3636
import org.junit.jupiter.api.Test;
3737

38+
import java.time.Duration;
3839
import java.time.Instant;
3940
import java.util.Date;
4041
import java.util.List;
@@ -93,27 +94,98 @@ void shouldUpdateMetricsEmpty() throws Exception {
9394
}
9495

9596
@Test
96-
void shouldUpdateMetricsUnchanged() throws Exception {
97+
void shouldNotCreateNewRowsWhenMetricsUnchanged() throws Exception {
9798
final var project = new Project();
9899
project.setName("acme-app");
99100
qm.createProject(project, List.of(), false);
100101

102+
final var component = new Component();
103+
component.setProject(project);
104+
component.setName("acme-lib-a");
105+
qm.createComponent(component, false);
106+
101107
// Create risk score configproperties
102108
createTestConfigProperties();
103109

104-
// Record initial project metrics
110+
// Record initial metrics
105111
executeActivity(project);
106-
final ProjectMetrics metrics = withJdbiHandle(handle -> handle.attach(MetricsDao.class).getMostRecentProjectMetrics(project.getId()));
107-
assertThat(metrics.getLastOccurrence()).isEqualTo(metrics.getFirstOccurrence());
112+
113+
final ProjectMetrics initialProjectMetrics = withJdbiHandle(
114+
handle -> handle
115+
.attach(MetricsDao.class)
116+
.getMostRecentProjectMetrics(project.getId()));
117+
assertThat(initialProjectMetrics.getLastOccurrence())
118+
.isEqualTo(initialProjectMetrics.getFirstOccurrence());
108119

109120
// Run the task a second time, without any metric being changed
110-
final var beforeSecondRun = new Date();
111121
executeActivity(project);
112122

113-
// Two records should be created in today's partition since it's append-only
114-
final var recentMetrics = withJdbiHandle(handle -> handle.attach(MetricsDao.class).getMostRecentProjectMetrics(project.getId()));
115-
assertThat(recentMetrics.getLastOccurrence()).isNotEqualTo(metrics.getFirstOccurrence());
116-
assertThat(recentMetrics.getLastOccurrence()).isAfterOrEqualTo(beforeSecondRun);
123+
// No new row must have been created, and the existing row's timestamp must remain untouched
124+
final List<ProjectMetrics> projectMetrics = withJdbiHandle(
125+
handle -> handle
126+
.attach(MetricsDao.class)
127+
.getProjectMetricsSince(project.getId(), Instant.EPOCH));
128+
assertThat(projectMetrics)
129+
.hasSize(1)
130+
.first()
131+
.extracting(ProjectMetrics::getLastOccurrence)
132+
.isEqualTo(initialProjectMetrics.getLastOccurrence());
133+
134+
final List<DependencyMetrics> componentMetrics = withJdbiHandle(
135+
handle -> handle
136+
.attach(MetricsDao.class)
137+
.getDependencyMetricsSince(component.getId(), Instant.EPOCH));
138+
assertThat(componentMetrics).hasSize(1);
139+
}
140+
141+
@Test
142+
void shouldCreateNewRowsWhenMetricsChanged() throws Exception {
143+
createTestConfigProperties();
144+
145+
final var project = new Project();
146+
project.setName("acme-app");
147+
qm.createProject(project, List.of(), false);
148+
149+
final var component = new Component();
150+
component.setProject(project);
151+
component.setName("acme-lib-a");
152+
qm.createComponent(component, false);
153+
154+
// Populate initial metrics.
155+
executeActivity(project);
156+
157+
// Make component affected by a vuln so its metrics values
158+
// will differ from the initial ones.
159+
var vuln = new Vulnerability();
160+
vuln.setVulnId("INTERNAL-001");
161+
vuln.setSource(Vulnerability.Source.INTERNAL);
162+
vuln.setSeverity(Severity.HIGH);
163+
vuln = qm.createVulnerability(vuln);
164+
qm.addVulnerability(vuln, component, "none");
165+
166+
// Calculate metrics again.
167+
executeActivity(project);
168+
169+
// New rows must be created despite snapshots existing for the current day.
170+
final List<ProjectMetrics> projectMetrics = withJdbiHandle(
171+
handle -> handle
172+
.attach(MetricsDao.class)
173+
.getProjectMetricsSince(project.getId(), Instant.EPOCH));
174+
assertThat(projectMetrics)
175+
.hasSize(2)
176+
.last()
177+
.extracting(ProjectMetrics::getVulnerabilities)
178+
.isEqualTo(1);
179+
180+
final List<DependencyMetrics> componentMetrics = withJdbiHandle(
181+
handle -> handle
182+
.attach(MetricsDao.class)
183+
.getDependencyMetricsSince(component.getId(), Instant.EPOCH));
184+
assertThat(componentMetrics)
185+
.hasSize(2)
186+
.last()
187+
.extracting(DependencyMetrics::getVulnerabilities)
188+
.isEqualTo(1);
117189
}
118190

119191
@Test
@@ -336,6 +408,235 @@ void shouldUpdateMetricsPolicyViolations() throws Exception {
336408
assertThat(componentSuppressed.getLastInheritedRiskScore()).isZero();
337409
}
338410

411+
@Test
412+
void shouldComputeSameMetricsAsComponentLevelComputation() throws Exception {
413+
// NB: Project metrics calculate metrics of all components within the project
414+
// in bulk. The corresponding logic differs from the one used to update metrics
415+
// for individual components (they're different stored procs).
416+
//
417+
// To ensure consistency between the two paths, it's CRITICAL that both compute
418+
// the exact same data. This test is meant to assert this to detect unintended
419+
// drift later down the road.
420+
421+
final var project = new Project();
422+
project.setName("acme-app");
423+
qm.createProject(project, List.of(), false);
424+
425+
createTestConfigProperties();
426+
427+
var vuln = new Vulnerability();
428+
vuln.setVulnId("INTERNAL-001");
429+
vuln.setSource(Vulnerability.Source.INTERNAL);
430+
vuln.setSeverity(Severity.HIGH);
431+
vuln = qm.createVulnerability(vuln);
432+
433+
final var componentClean = new Component();
434+
componentClean.setProject(project);
435+
componentClean.setName("acme-lib-clean");
436+
qm.createComponent(componentClean, false);
437+
438+
final var componentVulnUnaudited = new Component();
439+
componentVulnUnaudited.setProject(project);
440+
componentVulnUnaudited.setName("acme-lib-vuln-unaudited");
441+
qm.createComponent(componentVulnUnaudited, false);
442+
qm.addVulnerability(vuln, componentVulnUnaudited, "none");
443+
444+
final var componentVulnAudited = new Component();
445+
componentVulnAudited.setProject(project);
446+
componentVulnAudited.setName("acme-lib-vuln-audited");
447+
qm.createComponent(componentVulnAudited, false);
448+
qm.addVulnerability(vuln, componentVulnAudited, "none");
449+
qm.makeAnalysis(
450+
new MakeAnalysisCommand(componentVulnAudited, vuln)
451+
.withState(AnalysisState.NOT_AFFECTED));
452+
453+
final var componentVulnSuppressed = new Component();
454+
componentVulnSuppressed.setProject(project);
455+
componentVulnSuppressed.setName("acme-lib-vuln-suppressed");
456+
qm.createComponent(componentVulnSuppressed, false);
457+
qm.addVulnerability(vuln, componentVulnSuppressed, "none");
458+
qm.makeAnalysis(
459+
new MakeAnalysisCommand(componentVulnSuppressed, vuln)
460+
.withState(AnalysisState.FALSE_POSITIVE)
461+
.withSuppress(true));
462+
463+
final var componentViolationUnaudited = new Component();
464+
componentViolationUnaudited.setProject(project);
465+
componentViolationUnaudited.setName("acme-lib-violation-unaudited");
466+
qm.createComponent(componentViolationUnaudited, false);
467+
createPolicyViolation(
468+
componentViolationUnaudited,
469+
Policy.ViolationState.FAIL,
470+
PolicyViolation.Type.LICENSE);
471+
472+
final var componentViolationAudited = new Component();
473+
componentViolationAudited.setProject(project);
474+
componentViolationAudited.setName("acme-lib-violation-audited");
475+
qm.createComponent(componentViolationAudited, false);
476+
final var violationAudited = createPolicyViolation(
477+
componentViolationAudited,
478+
Policy.ViolationState.WARN,
479+
PolicyViolation.Type.OPERATIONAL);
480+
qm.makeViolationAnalysis(
481+
new MakeViolationAnalysisCommand(componentViolationAudited, violationAudited)
482+
.withState(ViolationAnalysisState.APPROVED));
483+
484+
final var componentViolationSuppressed = new Component();
485+
componentViolationSuppressed.setProject(project);
486+
componentViolationSuppressed.setName("acme-lib-violation-suppressed");
487+
qm.createComponent(componentViolationSuppressed, false);
488+
final var violationSuppressed = createPolicyViolation(
489+
componentViolationSuppressed,
490+
Policy.ViolationState.INFO,
491+
PolicyViolation.Type.SECURITY);
492+
qm.makeViolationAnalysis(
493+
new MakeViolationAnalysisCommand(componentViolationSuppressed, violationSuppressed)
494+
.withState(ViolationAnalysisState.REJECTED)
495+
.withSuppress(true));
496+
497+
final List<Component> components = List.of(
498+
componentClean,
499+
componentVulnUnaudited,
500+
componentVulnAudited,
501+
componentVulnSuppressed,
502+
componentViolationUnaudited,
503+
componentViolationAudited,
504+
componentViolationSuppressed);
505+
506+
executeActivity(project);
507+
508+
final List<DependencyMetrics> projectLevelMetrics = components.stream()
509+
.map(component -> withJdbiHandle(handle -> handle
510+
.attach(MetricsDao.class)
511+
.getMostRecentDependencyMetrics(component.getId())))
512+
.toList();
513+
514+
// Wipe all component metrics so the single-component procedure's
515+
// change detection cannot skip any insert.
516+
useJdbiHandle(handle -> handle.execute("DELETE FROM \"DEPENDENCYMETRICS\""));
517+
518+
useJdbiHandle(handle -> {
519+
final var dao = handle.attach(MetricsDao.class);
520+
components.forEach(component -> dao.updateComponentMetrics(component.getUuid()));
521+
});
522+
523+
for (int i = 0; i < components.size(); i++) {
524+
final long componentId = components.get(i).getId();
525+
final DependencyMetrics componentMetrics = withJdbiHandle(
526+
handle -> handle
527+
.attach(MetricsDao.class)
528+
.getMostRecentDependencyMetrics(componentId));
529+
assertThat(componentMetrics)
530+
.usingRecursiveComparison()
531+
.ignoringFields("firstOccurrence", "lastOccurrence")
532+
.isEqualTo(projectLevelMetrics.get(i));
533+
}
534+
}
535+
536+
@Test
537+
void shouldCreateNewRowsOnNewDay() throws Exception {
538+
// Daily datapoint queries (portfolio metrics, PORTFOLIOMETRICS_GLOBAL, collection metrics)
539+
// rely on at least one PROJECTMETRICS row existing per project per day.
540+
// Snapshots of a previous day must never suppress an insert, even when their values are identical.
541+
542+
createTestConfigProperties();
543+
544+
final var project = new Project();
545+
project.setName("acme-app");
546+
qm.createProject(project, List.of(), false);
547+
548+
final var component = new Component();
549+
component.setProject(project);
550+
component.setName("acme-lib-a");
551+
qm.createComponent(component, false);
552+
553+
// Seed yesterday's rows, with values identical to what the metrics update will compute.
554+
final Date yesterday = Date.from(Instant.now().minus(Duration.ofDays(1)));
555+
useJdbiHandle(handle -> {
556+
final var dao = handle.attach(MetricsTestDao.class);
557+
dao.createPartitionForDaysAgo("PROJECTMETRICS", 1);
558+
dao.createPartitionForDaysAgo("DEPENDENCYMETRICS", 1);
559+
560+
final var projectMetrics = new ProjectMetrics();
561+
projectMetrics.setProjectId(project.getId());
562+
projectMetrics.setComponents(1);
563+
projectMetrics.setFirstOccurrence(yesterday);
564+
projectMetrics.setLastOccurrence(yesterday);
565+
dao.createProjectMetrics(projectMetrics);
566+
567+
final var dependencyMetrics = new DependencyMetrics();
568+
dependencyMetrics.setProjectId(project.getId());
569+
dependencyMetrics.setComponentId(component.getId());
570+
dependencyMetrics.setFirstOccurrence(yesterday);
571+
dependencyMetrics.setLastOccurrence(yesterday);
572+
dao.createDependencyMetrics(dependencyMetrics);
573+
});
574+
575+
executeActivity(project);
576+
577+
final List<ProjectMetrics> projectMetrics = withJdbiHandle(
578+
handle -> handle
579+
.attach(MetricsDao.class)
580+
.getProjectMetricsSince(project.getId(), Instant.EPOCH));
581+
assertThat(projectMetrics).hasSize(2);
582+
assertThat(projectMetrics.getLast().getLastOccurrence()).isAfter(yesterday);
583+
584+
final List<DependencyMetrics> componentMetrics = withJdbiHandle(
585+
handle -> handle
586+
.attach(MetricsDao.class)
587+
.getDependencyMetricsSince(component.getId(), Instant.EPOCH));
588+
assertThat(componentMetrics).hasSize(2);
589+
assertThat(componentMetrics.getLast().getLastOccurrence()).isAfter(yesterday);
590+
}
591+
592+
@Test
593+
void shouldCreateNewRowsWhenLatestRowHasNullColumns() throws Exception {
594+
// Some metric columns are nullable and may be NULL in rows migrated
595+
// from v4. Change detection must treat NULL as changed and insert a fresh row,
596+
// rather than silently skipping.
597+
598+
createTestConfigProperties();
599+
600+
final var project = new Project();
601+
project.setName("acme-app");
602+
qm.createProject(project, List.of(), false);
603+
604+
final var component = new Component();
605+
component.setProject(project);
606+
component.setName("acme-lib-a");
607+
qm.createComponent(component, false);
608+
609+
// Seed a row for the current day whose values match what the
610+
// metrics update will compute, then NULL out its nullable columns.
611+
useJdbiHandle(handle -> {
612+
final var dependencyMetrics = new DependencyMetrics();
613+
dependencyMetrics.setProjectId(project.getId());
614+
dependencyMetrics.setComponentId(component.getId());
615+
dependencyMetrics.setFirstOccurrence(Date.from(Instant.now().minus(Duration.ofMinutes(1))));
616+
dependencyMetrics.setLastOccurrence(Date.from(Instant.now().minus(Duration.ofMinutes(1))));
617+
handle.attach(MetricsTestDao.class).createDependencyMetrics(dependencyMetrics);
618+
619+
handle.createUpdate("""
620+
UPDATE "DEPENDENCYMETRICS"
621+
SET "FINDINGS_TOTAL" = NULL
622+
, "FINDINGS_AUDITED" = NULL
623+
, "FINDINGS_UNAUDITED" = NULL
624+
, "UNASSIGNED_SEVERITY" = NULL
625+
WHERE "COMPONENT_ID" = :componentId
626+
""")
627+
.bind("componentId", component.getId())
628+
.execute();
629+
});
630+
631+
executeActivity(project);
632+
633+
final List<DependencyMetrics> componentMetrics = withJdbiHandle(
634+
handle -> handle
635+
.attach(MetricsDao.class)
636+
.getDependencyMetricsSince(component.getId(), Instant.EPOCH));
637+
assertThat(componentMetrics).hasSize(2);
638+
}
639+
339640
private void executeActivity(Project project) throws Exception {
340641
activity.execute(null, UpdateProjectMetricsArg.newBuilder()
341642
.setProjectUuid(project.getUuid().toString())

0 commit comments

Comments
 (0)