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

Improve performance of findings retrieval #3869

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

nscuro
Copy link
Member

@nscuro nscuro commented Jun 23, 2024

Description

The /v1/finding/{projectUuid} endpoint has historically been slow to respond (#3811). While the "main" query behind it is somewhat optimized SQL already, it still suffered from various performance killers:

  • Filtering of suppressed findings was done in-memory, and required fetching of individual Analysis records for every single finding.
  • Clob fields were not mapped directly from the SQL query result, but instead by re-fetching Component and Vulnerability records for every single finding, such that the ORM would provide properly String-ified field values.
  • Aliases were fetched for every single finding individually.
  • Latest component versions were fetched for every single finding individually.

Performance was improved via the following changes:

  1. Filtering of suppressed findings is moved to the main SQL query, voiding the need to fetch individual Analysis records later. This also reduces the overall result set that needs to be transferred and mapped.
  2. Mapping of Clob fields is done within the Finding constructor, voiding the need to re-fetch Vulnerability records in order to retrieve String values for them.
  3. Aliases are loaded in bulk, and in a way that avoids redundant queries if the same Vulnerability appears multiple times within a list of Findings.
  4. Latest component versions are loaded in bulk, and in a way that avoids redundant queries if the same Component appears multiple times within a list of Findings.

Because the modified functionality is re-used across the code base, multiple features benefit from this enhancement:

  • /v1/finding/{projectUuid} endpoint
    • Corresponds to the Audit Vulnerabilities tab in the UI
  • /v1/project/{projectUuid}/export endpoint
  • CycloneDX exports for Inventory with Vulnerabilities, VDR, and VEX
  • Fortify, Kenna, and DefectDojo integrations

Note

Tested with H2, MSSQL, and PostgreSQL.

Addressed Issue

Relates to #3811

Additional Details

Performance comparison with local Docker Compose setup:

cd dev
docker compose -f docker-compose.yml -f docker-compose.postgres.yml up -d

Details about the setup:

  • Internal analyzer enabled
  • OSS Index enabled
  • NVD mirrored completely
  • GitHub Advisories mirrored completely; Alias sync enabled
  • Uploaded bom-bloated.json (9056 unique components)
  • 1652 vulnerabilities identified

API server images compared:

  • snapshot@sha256:42b94f37ad5d4c9965fc89d48524bddb84c6142ee574a0c01c72ab3b667d321f
  • local (local build of this PR)

Load tested the /api/v1/finding/${projectUuid} endpoint using hey:

hey -H "X-Api-Key: $APIKEY" \
    http://localhost:8080/api/v1/finding/project/d7b6156e-c58b-4b85-b3ef-db61e9667cd4
Results for snapshot
Summary:
  Total:	41.4942 secs
  Slowest:	11.3247 secs
  Fastest:	9.3886 secs
  Average:	10.3208 secs
  Requests/sec:	4.8199


Response time histogram:
  9.389 [1]	|■
  9.582 [3]	|■■■
  9.776 [11]	|■■■■■■■■■■
  9.969 [20]	|■■■■■■■■■■■■■■■■■■
  10.163 [35]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  10.357 [44]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  10.550 [34]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  10.744 [18]	|■■■■■■■■■■■■■■■■
  10.937 [17]	|■■■■■■■■■■■■■■■
  11.131 [11]	|■■■■■■■■■■
  11.325 [6]	|■■■■■


Latency distribution:
  10% in 9.8559 secs
  25% in 10.0557 secs
  50% in 10.2913 secs
  75% in 10.5717 secs
  90% in 10.9142 secs
  95% in 11.0154 secs
  99% in 11.2493 secs

Details (average, fastest, slowest):
  DNS+dialup:	0.0010 secs, 9.3886 secs, 11.3247 secs
  DNS-lookup:	0.0004 secs, 0.0000 secs, 0.0025 secs
  req write:	0.0000 secs, 0.0000 secs, 0.0008 secs
  resp wait:	10.2449 secs, 9.3476 secs, 11.2671 secs
  resp read:	0.0749 secs, 0.0295 secs, 0.1779 secs

Status code distribution:
  [200]	200 responses
Results for local
Summary:
  Total:	3.6591 secs
  Slowest:	1.5565 secs
  Fastest:	0.1958 secs
  Average:	0.8520 secs
  Requests/sec:	54.6580


Response time histogram:
  0.196 [1]	|■
  0.332 [4]	|■■■■
  0.468 [10]	|■■■■■■■■■■
  0.604 [16]	|■■■■■■■■■■■■■■■
  0.740 [39]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  0.876 [31]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  1.012 [42]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  1.148 [35]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  1.284 [14]	|■■■■■■■■■■■■■
  1.420 [7]	|■■■■■■■
  1.557 [1]	|■


Latency distribution:
  10% in 0.5335 secs
  25% in 0.6879 secs
  50% in 0.8691 secs
  75% in 1.0409 secs
  90% in 1.1557 secs
  95% in 1.2686 secs
  99% in 1.4125 secs

Details (average, fastest, slowest):
  DNS+dialup:	0.0010 secs, 0.1958 secs, 1.5565 secs
  DNS-lookup:	0.0005 secs, 0.0000 secs, 0.0025 secs
  req write:	0.0000 secs, 0.0000 secs, 0.0008 secs
  resp wait:	0.6837 secs, 0.1470 secs, 1.4515 secs
  resp read:	0.1673 secs, 0.0281 secs, 0.5001 secs

Status code distribution:
  [200]	200 responses

Summary

  • Requests per second: 4.8 -> 54.7
  • p99 latency: 11.2s -> 1.4s
  • p75 latency: 10.5s -> 1s
  • p50 latency: 10.3s -> 0.9s

Coming out to a fairly consistent 10x improvement.

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have added corresponding update logic
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

@nscuro nscuro added enhancement New feature or request performance labels Jun 23, 2024
@nscuro nscuro added this to the 4.12 milestone Jun 23, 2024
The `/v1/finding/{projectUuid}` endpoint has historically been slow to respond (DependencyTrack#3811). While the "main" query behind it is somewhat optimized SQL already, it still suffered from various performance killers:

* Filtering of suppressed findings was done in-memory, and required fetching of individual `Analysis` records *for every single finding*.
* `Clob` fields were not mapped directly from the SQL query result, but instead by re-fetching `Component` and `Vulnerability` records *for every single finding*, such that the ORM would provide properly `String`-ified field values.
* Aliases were fetched *for every single finding* individually.
* Latest component versions were fetched *for every single finding* individually.

Performance was improved via the following changes:

1. Filtering of suppressed findings is moved to the main SQL query, voiding the need to fetch individual `Analysis` records later. This also reduces the overall result set that needs to be transferred and mapped.
2. Mapping of `Clob` fields is done within the `Finding` constructor, voiding the need to re-fetch `Vulnerability` records in order to retrieve `String` values for them.
3. Aliases are loaded in bulk, and in a way that avoids redundant queries if the same `Vulnerability` appears multiple times within a list of `Finding`s.
4. Latest component versions are loaded in bulk, and in a way that avoids redundant queries if the same `Component` appears multiple times within a list of `Finding`s.

Because the modified functionality is re-used across the code base, multiple features benefit from this enhancement:

* `/v1/finding/{projectUuid}` endpoint
    * Corresponds to the *Audit Vulnerabilities* tab in the UI
* `/v1/project/{projectUuid}/export` endpoint
* CycloneDX exports for *Inventory with Vulnerabilities*, *VDR*, and *VEX*
* Fortify, Kenna, and DefectDojo integrations

Signed-off-by: nscuro <nscuro@protonmail.com>
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.04% (target: -1.00%) 71.56% (target: 70.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (bc2de42) 21698 16498 76.03%
Head commit (ba17eb2) 21787 (+89) 16556 (+58) 75.99% (-0.04%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3869) 109 78 71.56%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences


🚀 Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@nscuro nscuro merged commit 04f5ccc into DependencyTrack:master Jun 23, 2024
11 checks passed
@nscuro nscuro deleted the issue-3811 branch June 23, 2024 20:00
nscuro added a commit to DependencyTrack/hyades-apiserver that referenced this pull request Jun 24, 2024
Ports DependencyTrack/dependency-track#3869 from Dependency-Track v4.12.0-SNAPSHOT.

Signed-off-by: nscuro <nscuro@protonmail.com>
nscuro added a commit to DependencyTrack/hyades-apiserver that referenced this pull request Jun 24, 2024
Ports DependencyTrack/dependency-track#3869 from Dependency-Track v4.12.0-SNAPSHOT.

Signed-off-by: nscuro <nscuro@protonmail.com>
nscuro added a commit to DependencyTrack/hyades-apiserver that referenced this pull request Jun 25, 2024
Ports DependencyTrack/dependency-track#3869 from Dependency-Track v4.12.0-SNAPSHOT.

Signed-off-by: nscuro <nscuro@protonmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant