Skip to content

feat: add phase-level latency metrics for container scans#804

Open
bdemeo12 wants to merge 2 commits intomainfrom
POC/add-latency-metrics
Open

feat: add phase-level latency metrics for container scans#804
bdemeo12 wants to merge 2 commits intomainfrom
POC/add-latency-metrics

Conversation

@bdemeo12
Copy link
Copy Markdown
Contributor

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Adds per-phase latency timing to the container scan flow. Each major phase (image extraction, OS package analysis, JAR/Go/Node/PHP/Python analysis, binary detection, dep tree building) is timed with Date.now() and returned on the PluginResponse via a new optional analytics field. A companion CLI change wires these timings into CLI analytics metadata as containerPluginTimings.

Any background context you want to provide?

The container plugin has no latency visibility, this PR is to collect latency metrics and surface them in the CLI analytics.

What are the relevant tickets?

https://snyksec.atlassian.net/browse/CN-1170
https://snyksec.atlassian.net/browse/CN-1054

@bdemeo12 bdemeo12 requested review from a team as code owners April 28, 2026 19:53
@bdemeo12 bdemeo12 requested a review from jcambier April 28, 2026 19:53
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unsafe Spread of Optional Timings 🟡 [minor]

In analyzeStatically, the code spreads staticAnalysis.timings. While the interface StaticAnalysis in lib/analyzer/types.ts defines timings as optional, the analyze function in static-analyzer.ts always initializes it. However, if staticAnalysis.timings were ever undefined (e.g., from a mock or future code path), spreading it into a new object is safe in modern JS, but it could lead to missing data in the containerPluginTimings analytics event without a fallback.

const timings: Record<string, number> = {
  ...staticAnalysis.timings,
  depTreeBuildingMs,
  totalMs,
};
📚 Repository Context Analyzed

This review considered 15 relevant code sections from 10 files (average relevance: 0.72)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant