Skip to content

✨ policy: send per-scan statistics with ReportUploadCompleted#3103

Merged
jaym merged 19 commits into
mainfrom
jdm/scan-statistics
Jul 21, 2026
Merged

✨ policy: send per-scan statistics with ReportUploadCompleted#3103
jaym merged 19 commits into
mainfrom
jdm/scan-statistics

Conversation

@jaym

@jaym jaym commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What

Adds per-scan statistics to the ReportUploadCompleted RPC so the Mondoo platform receives metrics about each scan upload.

  • Extensible completion payload. New google.protobuf.Any details = 3 field on the (generic, multi-upload-kind) ReportUploadCompletedReq. For scan-database uploads it carries a new ScanStatistics { repeated Metric }, where each Metric is a namespaced name + unit + oneof value (int64/double/bool/string). Adding a new metric — or a new upload kind's payload — needs no change to the base message.
  • New policy/scanstats package. A concurrency-safe Collector (AddInt/AddDuration/AddDouble/AddBoolToProto) and a CountByKind classifier.
  • v1 metrics (all cnspec.scan.*): duration (ms), upload_size (bytes), per-kind counts checks, data_queries, policies, controls, frameworks, and checks_errored / data_queries_errored.
  • Accurate counting. Counts derive from the resolved policy's reporting jobs (deduped by qr_id, excluding the asset-root aggregate and risk factors) plus the finalized scores/data results — an honest reflection of what was scored, not inflated write-op counts. Wiring is best-effort: a statistics failure logs a warning and never blocks the upload confirmation.

Why

We want visibility into scan cost and shape: how long a scan took, how large the upload is, and a per-kind breakdown of what was evaluated (checks, data queries, policies, controls, frameworks) plus error counts. The namespaced Metric bag inside an Any payload makes this extensible — future provider-contributed metrics (cloud API calls, throttling, peak memory) can be reported under a provider.<name>.* namespace with no schema change.

Testing

  • proto and vtproto round-trip of the Any(ScanStatistics) payload (de-risks the new Any pattern)
  • scanstats.Collector and CountByKind unit tests: per-kind counts, error classification, root/risk-factor exclusion, CHECK_AND_DATA_QUERY handling, nil-safety
  • upload-size helper + wiring tests
  • -race clean across touched packages; golangci-lint v2 reports 0 issues

🤖 Generated with Claude Code

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds per-scan statistics (check/query/policy counts, errors, duration, upload size) to the ReportUploadCompleted RPC payload.

Comment thread policy/executor/graph.go
Comment thread policy/scanstats/collector.go
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Test Results

  1 files  ± 0   52 suites  +1   2m 9s ⏱️ +38s
926 tests +17  925 ✅ +17  1 💤 ±0  0 ❌ ±0 
927 runs  +17  926 ✅ +17  1 💤 ±0  0 ❌ ±0 

Results for commit 7cbeb08. ± Comparison against base commit f8ff343.

♻️ This comment has been updated with latest results.

jaym and others added 19 commits July 21, 2026 05:45
Design for attaching per-scan statistics to the ReportUploadCompleted RPC
via a google.protobuf.Any completion payload (Level 1, keeps the base
message generic across upload kinds) carrying a flat, namespaced
ScanStatistics metric list (Level 2). Collector seam makes adding metrics
(incl. future provider metrics) easy without proto churn.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pleted

Adds google.protobuf.Any details field to ReportUploadCompletedReq so
upload-kind-specific payloads can be attached without changing the
message. Introduces ScanStatistics and Metric messages as the
scan-database upload payload. Round-trip tests confirm both stock proto
and vtproto marshaling work correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Plumb a scanstats.Collector through the upload path: records scan
duration, query executed/errored counts, and upload size (bytes), then
packs them as google.protobuf.Any into Details on the
ReportUploadCompleted RPC. Stats collection is guarded so a nil or
unencodable payload never breaks the upload confirmation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, comment)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ounters

Replace inflated write-counters (ExecutedCount/ErroredCount on the wrapper) with
per-kind counts derived from the resolved policy via scanstats.CountByKind.
Capture the inmemory.Db returned by NewServices so GetResolvedPolicy reads the
local resolved policy; stream scores and data from the finalized store. Remove
the MetricQueriesExecuted/MetricQueriesErrored constants, the atomic counter
fields/getters from ScanDataStoreWrapper, and the obsolete wrapper stats test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… test cases

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The design spec and implementation plan were working artifacts; they should
not ship in the product PR. Removed so the PR diff contains only code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ror column)

- Add `error TEXT NOT NULL DEFAULT ''` column to the `data` table, populated from `llx.Result.GetError()` on write
- Add `ErroredScoreQrIds` query (filter scores by type) and `CountErroredData` query (COUNT on non-empty error column)
- Regenerate sqlc: `InsertDataParams` gains `Error string`; new methods `ErroredScoreQrIds(ctx, type_ int64) ([]string, error)` and `CountErroredData(ctx) (int64, error)`
- Add `ErroredScoreQrIds` and `ErroredDataCount` wrapper methods on `SqliteScanDataStore`
- Add `scan_data_store_counts_test.go` covering both new methods and the empty-store case

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- policy/scanstats: add ContextWithCollector/CollectorFromContext helpers so
  downstream code can retrieve the stats collector from ctx without signature
  changes; add RecordKindCounts to write all 7 per-kind metrics in one call.
- policy/executor: add countingCollector (ScoreCollector + DatapointCollector)
  that accumulates per-kind executed counts from the resolved policy and tracks
  errored checks/data with last-write-wins semantics per qr_id; wire it into
  ExecuteResolvedPolicy behind a ctx guard — installed only when a
  scanstats.Collector is present, so existing behavior is fully unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…can DB counting

Put the scanstats.Collector in ctx via ContextWithCollector before calling f
so the graph executor's countingCollector (added in 95e3296) records per-kind
metrics during execution. Remove recordKindMetrics and the now-obsolete
Option B DB counting: drop the data.error column, remove InsertData's Error
field, delete ErroredScoreQrIds/CountErroredData SQL queries and their
generated methods, and remove the corresponding store methods and test file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mantics

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jaym
jaym force-pushed the jdm/scan-statistics branch from 1bd37bf to 7cbeb08 Compare July 21, 2026 11:10
@jaym
jaym merged commit e0a7653 into main Jul 21, 2026
19 checks passed
@jaym
jaym deleted the jdm/scan-statistics branch July 21, 2026 17:27
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant