Repository navigation
feat(metrics): add BigQuery schema, row normalization and writer - #1405
Merged
Merged
Conversation
Add the canonical BigQuery `xmc_metrics` table schema, MV-equivalent Go row normalization, and Tier 1 `BigQueryWriter` core for `cmd/metrics_ingester` as the first step toward supporting BigQuery alongside ClickHouse for experiment metrics storage. Unlike ClickHouse, where `xmc_metrics_pubsub_mv` filters invalid records, defaults `replica_id`, and clamps out-of-range timestamps inside the database, BigQuery appends directly into `xmc_metrics`. `NormalizeMetrics` reproduces the exact `xmc_metrics_pubsub_mv` semantics in Go (`xid != 0`, `metric_name != ""`, `execution_id != 0`, `replica_id` defaulting to `"0"`, and `[2020-01-01, 2050-01-01]` UTC timestamp clamping) so both storage backends persist identical rows. `BigQueryWriter` encodes rows using proto2 `LABEL_REQUIRED` wire format via `protowire` so zero values (`step = 0`, `metric_value = 0.0`, `job_name = ""`) are always serialized explicitly rather than omitted as `NULL`, and chunks serialized payloads under an 8 MiB ceiling below the BigQuery Storage Write API's 10 MB request limit. Schema parity tests in both Python (`schema_test.py`) and Go (`bigquery_writer_test.go`) lock `deploy/bigquery/schema.json`, `deploy/clickhouse/schema.sql`, `MetricRowDescriptor()`, and `marshalMetricRow` together. TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: Ied7abfc7a1b75b937caef8c64f05c30c3d905e1d
There was a problem hiding this comment.
Code Review
This pull request adds a new BigQuery writer for the metrics ingester, including the necessary schema definitions, normalization logic, and corresponding tests. The reviewer suggested adding context cancellation checks in the chunking loop, adding a nil check for metrics in the normalization function, and improving the robustness of the schema test's type mapping logic.
… schema test Address automated code review feedback on PR #1405: - Check `ctx.Err()` at the start of each chunk iteration in `BigQueryWriter.appendChunked` so canceled contexts abort immediately without issuing further `AppendRows` calls, with unit test coverage. - Add an explicit `m == nil` guard at the start of `normalizeSingle` so nil slice entries do not rely implicitly on generated protobuf getter nil-safety. - Broaden `_map_clickhouse_type` in `deploy/bigquery/schema_test.py` to match `f"{prefix} "` so ClickHouse column modifiers beyond `DEFAULT` (such as `CODEC`, `COMMENT`, or `TTL`) map cleanly. Comment link: #1405 (comment) Comment link: #1405 (comment) Comment link: #1405 (comment) TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: I2caa4dd820ce9f76ba2b9dd28eb416f0ff79d1c9
AntonTyb
marked this pull request as ready for review
October 25, 2026 12:29
janzak-dev
reviewed
Oct 26, 2026
…w schema Add TestMetricProtoMatchesBigQueryRowSchema using protoreflect to assert that MetricRowDescriptor() matches api/protos/metrics/metrics.proto (pb.Metric) in field count (+ created_at), field numbers, names, and descriptor types, and that every dynamically populated pb.Metric field survives NormalizeMetrics and marshalMetricRow without loss. Comment link: #1405 (comment) TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: Ied450259fa838c0de06c77fd64bb866bee71f555
This was referenced Oct 26, 2026
AntonTyb
added a commit
that referenced
this pull request
Oct 26, 2026
…/metrics-ingester-bigquery-config Sync downstream stack branch with PR #1405 review fixes. TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: I27c9fb9e4325e0f34ff68d219f66ce5cdcc4df5c
janzak-dev
approved these changes
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the canonical BigQuery
xmc_metricstable schema (deploy/bigquery/schema.json), MV-equivalent Go row normalization (NormalizeMetrics), and the Tier 1BigQueryWritercore forcmd/metrics_ingester(wired into the CLI runtime in #1406).Motivation & Context
XMC deployments need the option to store experiment metrics in Google Cloud BigQuery instead of ClickHouse. In ClickHouse, incoming metrics land in
xmc_metrics_pubsub_queue(ENGINE = Null) and pass throughxmc_metrics_pubsub_mv, which drops rows withxid = 0, emptymetric_name, orexecution_id = 0, defaults emptyreplica_idto'0', and clamps timestamps outside[2020-01-01, 2050-01-01]UTC to ingestion time. Because BigQuery Storage Write API appends directly intoxmc_metrics,NormalizeMetricsimplements those exact rules in Go so both storage backends store identical rows.BigQuery Storage Write API also requires proto2 wire encoding: using proto3 implicit field presence would omit zero-valued fields (
step = 0,metric_value = 0.0,job_name = "") on the wire and either violateREQUIREDcolumn constraints or insertNULLs that change SQL aggregation semantics (AVG,= '').MetricRowDescriptor()declares all 10 fieldsLABEL_REQUIREDandmarshalMetricRowencodes every field unconditionally viaprotowire.Key Changes
deploy/bigquery/{schema.json, schema_test.py, README.md, BUILD.bazel}: Defines the 10-columnREQUIREDBigQueryxmc_metricsschema with dailyDATE(timestamp)partitioning (require_partition_filter = false) andxid, metric_name, work_unit_idclustering, plus a 1-to-1 column/type/non-nullability parity test againstdeploy/clickhouse/schema.sql.cmd/metrics_ingester/metricsingester/{normalize.go, normalize_test.go}: ImplementsNormalizeMetricsmatchingxmc_metrics_pubsub_mvfiltering,replica_iddefaulting, and UTC microsecond timestamp clamping.cmd/metrics_ingester/metricsingester/{bigquery_writer.go, bigquery_writer_test.go, writer.go, BUILD.bazel}: ImplementsBigQueryWriter,RowAppender,MetricRowDescriptor(), and 8 MiB chunked proto2 wire serialization, verified bit-for-bit againstdeploy/bigquery/schema.jsonandapi/protos/metrics/metrics.proto(pb.Metric).go.mod,go.sum,MODULE.bazel: Addscloud.google.com/go/bigquery v1.77.0aligned with existing Google Cloud SDK versions without bumping unrelated modules.Assumptions
[2020-01-01T00:00:00Z, 2050-01-01T00:00:00Z]are clamped to ingestion time, and rows withxid = 0, emptymetric_name, orexecution_id = 0are dropped (returningnilwhen an entire batch is filtered so malformed messages are acked rather than redelivered indefinitely) — chosen to match ClickHouse'sxmc_metrics_pubsub_mvsanitization rules indeploy/clickhouse/schema.sql— if historical replay workloads intentionally emit pre-2020 or post-2050 metric timestamps, those timestamps will be clamped to ingestion time unless the shared bounds are widened.AppendRowsrequests are chunked atdefaultMaxAppendBytes = 8 MiB— chosen to stay safely below the BigQuery Storage Write API 10 MiB gRPC request limit while leaving headroom for protobuf framing — if a singleNormalizedMetricrow ever exceeded 8 MiB, it would be sent as a single-row chunk and rejected by BigQuery if above 10 MiB.deploy/bigquery/schema.jsonconfigures dailyDATE(timestamp)partitioning withrequire_partition_filter = falseand clusters byxid, metric_name, work_unit_id— chosen because XMC dashboard queries filter byxid = ?(pruned via BigQuery block clustering) without requiring a mandatory timestamp window — ad-hoc manual queries that omit bothxidandtimestamppredicates will scan all partitions unless a filter is supplied.Verification & Testing
bazel test --@io_bazel_rules_go//go/config:race --runs_per_test=10 //cmd/metrics_ingester/... //deploy/bigquery/... //deploy/clickhouse/...bazel coverage --combined_report=lcov //cmd/metrics_ingester/metricsingester:metricsingester_test./.githooks/pre-commitbigquery_writer.go,normalize.go, andwriter.go.Related Issues / Bugs
N/A
TAG=agy
CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd