Skip to content

fix: generate mandatory analytics.pb in fabricated scanner report#480

Merged
joshua-quek-sonarsource merged 1 commit into
mainfrom
fix/issue-478-analytics-pb
Jul 9, 2026
Merged

fix: generate mandatory analytics.pb in fabricated scanner report#480
joshua-quek-sonarsource merged 1 commit into
mainfrom
fix/issue-478-analytics-pb

Conversation

@okorach-sonar

@okorach-sonar okorach-sonar commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Compute Engine now hard-rejects fabricated scanner reports missing analytics.pb (server-side change, RP-1034, effective 2026-07-06)
  • Added the Analytics message to scanner-report.proto (mirrors the ScannerReport.Analytics schema from sonar-scanner-engine) and regenerated the Go bindings
  • PackageReport() now writes analytics.pb with a single property ci_name=sonar-migration-tool, so migrated analyses are accepted by CE and identifiable as coming from this tool

Fixes #478

Test plan

  • go build ./...
  • go vet ./...
  • go test ./internal/scanreport/... (updated TestPackageReportMinimal to assert analytics.pb is present in the zip)
  • Manual: run a migration and confirm CE accepts the report (no "mandatory file 'analytics.pb' is missing" error)

🤖 Generated with Claude Code

Compute Engine now hard-rejects analysis reports missing analytics.pb
(SonarQube Server/Cloud change as of 2026-07-06). Add the Analytics
message to the scanner report proto and write analytics.pb with
ci_name=sonar-migration-tool so migrated analyses are accepted and
distinguishable from real scanner runs.

Fixes #478

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@okorach-sonar
okorach-sonar requested a review from a team as a code owner July 8, 2026 16:35
@gitar-bot

gitar-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Adds mandatory analytics.pb file to fabricated scanner reports to satisfy Compute Engine schema requirements. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@okorach-sonar
okorach-sonar requested review from joshua-quek-sonarsource and removed request for a team July 8, 2026 20:59
@joshua-quek-sonarsource

Copy link
Copy Markdown
Contributor

Code Review:

  • Proto (scanner-report.proto:82-91) — the added Analytics message is byte-for-byte identical to the authoritative cloud scanner_report.proto in sonar-scanner-engine (field 1 property map<string,string>, field 2 multiValueProperties map<string,Values>, nested Values.values repeated string @1). CE reads it via ScannerReportReader.readAnalytics(), which requires the file to be present + parseable and does no content validation — so writing ci_name=sonar-migration-tool satisfies the new hard-reject rule.

  • Generated pb.go — faithful protoc output; the only changes to pre-existing messages are mechanical index renumbering (Analytics inserted at msg position 2), no field-number/type/wire changes. Toolchain header consistent with the repo's other .pb.go. NumMessages/depIdxs/enumTypes counts all internally balanced.

  • packager.go (packager.go:90-94) — addAnalytics is wired into PackageReport, which is the only zip.NewWriter in the codebase and the single call site (tasks_projectdata.go:545) runs for both the main branch and every non-main branch — so analytics.pb lands in every fabricated report, not just main.

  • Build / vet / test — all clean; scanreport tests pass (updated TestPackageReportMinimal asserts analytics.pb in the zip).

@joshua-quek-sonarsource

Copy link
Copy Markdown
Contributor

Review + live e2e — safe to merge ✅

Reviewed the change and ran a full live transfer end-to-end (SonarQube Server 2026.3 → SonarQube Cloud staging) against okorach-oss_sonar-tools (~1.7k issues, 6 branches).

Code review

  • The Analytics message added to scanner-report.proto is byte-for-byte identical to the authoritative cloud scanner_report.proto in sonar-scanner-engine (property = field 1 map<string,string>, multiValueProperties = field 2, nested Values.values = repeated string @1). CE reads it via ScannerReportReader.readAnalytics(), which only requires the file to be present + parseable — so writing ci_name=sonar-migration-tool satisfies the new hard-reject rule (and no content is validated).
  • Regenerated scanner-report.pb.go is a faithful protoc output: changes to pre-existing messages are mechanical index renumbering only (no field-number/type/wire changes), and the toolchain header matches the repo's other .pb.go.
  • addAnalytics is wired into PackageReport, which is the only report-zip assembler and runs for the main branch and every non-main branch — so analytics.pb lands in every fabricated report, not just main.
  • go build / go vet / go test ./internal/scanreport/... all clean.

Live e2e

  • analytics.pb accepted by the Compute Engine for all 5 migrated branches — zero "mandatory file 'analytics.pb' is missing" rejections. This is exactly the behavior the PR targets.
  • Transfer exited 0, overall_status: success.
  • Core data migrated correctly: issues (target 1303, consistent with the documented CLOSED/FIXED-skip semantics), 5 long-lived branches, ncloc exact parity (17,379).

No regression attributable to this change.

Unrelated aside from the same run: the e2e surfaced a pre-existing hotspot-migration gap (target 0 vs 31 on source, all in REVIEWED state on the source). This PR touches only report packaging — the hotspot migration/extract code is unchanged — so it's out of scope here, but worth a separate look.

@joshua-quek-sonarsource joshua-quek-sonarsource left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All Good

@joshua-quek-sonarsource

Copy link
Copy Markdown
Contributor

Did a Live Test and it runs properly.
However we have one Regression

The "Safe" hotspots did NOT get migrated.
@okorach-sonar I will approve this first, and then we can fix the "SAFE marked hotspot not getting migrated" in another Github Issue and PR

@joshua-quek-sonarsource
joshua-quek-sonarsource merged commit 381b068 into main Jul 9, 2026
10 checks passed
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.

CE rejects migrated analysis: Analysis report is malformed: mandatory file 'analytics.pb' is missing

3 participants