Skip to content

fix(ndt7): enumerate TCPInfo/BBRInfo fields to match legacy BigQuery order - #433

Merged
robertodauria merged 1 commit into
mainfrom
sandbox-roberto-ndt7-tcpinfo-order
Jun 3, 2026
Merged

robertodauria merged 1 commit into
mainfrom
sandbox-roberto-ndt7-tcpinfo-order

Conversation

@robertodauria

@robertodauria robertodauria commented Jun 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

De-embed tcp.LinuxTCPInfo/inetdiag.BBRInfo in the ndt7 model and enumerate their fields explicitly, ordered to reproduce the legacy ndt7 BigQuery table; reorder Measurement to [AppInfo, BBRInfo, TCPInfo, ConnectionInfo]. Adds NewTCPInfo/NewBBRInfo constructors; the measurer uses them.

bigquery.InferSchema previously emitted TCPInfo.ElapsedTime last (an artifact of embedding + a trailing field), so newly-autoloaded ndt7 tables diverged from the legacy parser table's nested field order. UNION ALL [BY NAME] coerces nested structs positionally, silently corrupting TCPInfo values (m-lab/etl-schema#195). Explicit enumeration makes the generated schema match the legacy table and enforces append-new-fields-at-the-end going forward.

Verification

  • Regenerated ndt7 schema: full TCPInfo order is identical to measurement-lab.ndt.ndt7 (55/55 fields), and Measurement matches [AppInfo, BBRInfo, TCPInfo, ConnectionInfo].
  • JSON wire format unchanged (identical field names) → no impact on emitted data or existing parsers.
  • go build ./... and go vet ./ndt7/... pass.

Part of the fix for m-lab/etl-schema#195. Follow-ups (separate PRs): deploy the schema to GCS, then recreate the existing autojoin ndt7_raw tables.


This change is Reviewable

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 26833216177

Coverage increased (+0.5%) to 76.444%

Details

  • Coverage increased (+0.5%) from the base build.
  • Patch coverage: 69 of 69 lines across 2 files are fully covered (100%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
ndt7/measurer/measurer.go 2 89.61%

Coverage Stats

Coverage Status
Relevant Lines: 2390
Covered Lines: 1827
Line Coverage: 76.44%
Coverage Strength: 1.54 hits per line

💛 - Coveralls

@bassosimone bassosimone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🥳

@robertodauria
robertodauria merged commit a925f07 into main Jun 3, 2026
4 checks passed
@robertodauria
robertodauria deleted the sandbox-roberto-ndt7-tcpinfo-order branch June 3, 2026 12:27
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.

3 participants