Repository navigation
Conversation
Wire BigQuery as a selectable metrics storage backend in `cmd/metrics_ingester` alongside ClickHouse, completing the Pub/Sub -> BigQuery ingestion path. Key additions: - Extend `Config` / `RegisterFlags` / `LoadConfig` with `-metrics_store` (`METRICS_STORE`, defaulting to `clickhouse`), `-bigquery_project_id` (`BIGQUERY_PROJECT_ID`, falling back to `ProjectID`), `-bigquery_dataset` (`BIGQUERY_DATASET`, defaulting to `xmc_metrics`), and `-bigquery_table` (`BIGQUERY_TABLE`, defaulting to `xmc_metrics`), rejecting unknown store names and slash-containing resource identifiers at startup. - Add the Tier 2 composition root (`RuntimeStore` / `OpenRuntimeStore` / `OpenBigQueryStream`) with LIFO error rollback, `sync.Once` idempotent `Close()` (ignoring normal `io.EOF` on healthy `ManagedStream.Close()`), and a zero-cost `Table.Metadata` startup check and `/readyz` `Pinger`. - Refactor `cmd/metrics_ingester/main.go` onto `os.Exit(mainExitCode())` so all deferred store, Pub/Sub, and HTTP server cleanups execute before exit, keeping the root `ManagedStream` context alive across `Ingester`'s 10s `SIGTERM` shutdown flush. TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: Ib647b2fa5bab569fa00e1f00d53a0d5c6dfbffb1
There was a problem hiding this comment.
Code Review
This pull request introduces support for Google Cloud BigQuery as an alternative metrics storage backend alongside the existing ClickHouse implementation. The changes include refactoring the metrics ingester to use a configurable storage interface, updating the configuration and health checks to be store-agnostic, and adding the necessary dependencies and tests. A review comment suggests improving the readability of a function signature in runtime.go by replacing the blank identifier with a descriptive name for the return value.
…Options Name the first return parameter `store` alongside `retErr` in `openBigQueryStoreWithOptions` to avoid mixing a blank-identifier return parameter with a named error return parameter. Comment link: #1406 (comment) TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: Ibc02e0c2d48b38762a76767acb0eafcb59613970
…/metrics-ingester-bigquery-config Sync downstream stack branch with PR #1405 review fixes. TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: I27c9fb9e4325e0f34ff68d219f66ce5cdcc4df5c
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @VLukiantsev on commit c3e17b1.
…t chunk duplicates Enable `managedwriter.EnableWriteRetries(true)` in `OpenBigQueryStream` so transient `UNAVAILABLE` / `RESOURCE_EXHAUSTED` `AppendRows` errors are retried automatically within the write timeout before nacking a batch, and document multi-chunk partial-commit duplicate semantics in `cmd/metrics_ingester/README.md`. TAG=agy CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd Change-Id: I24c7bda6a023878b3d22c38e06f5fce9cd150dc6
There was a problem hiding this comment.
🤖 Incremental PR Approval
PR author is a code owner and changes since last approval (3 lines, excluding 36 test file lines) are within the 50-line grace delta.
Preserving prior approval by @VLukiantsev on commit c3e17b1.
Summary
Wires BigQuery as a selectable metrics storage backend (
-metrics_store=bigquery/METRICS_STORE=bigquery) incmd/metrics_ingester, adding flag/env validation, the Tier 2RuntimeStorecomposition root, a zero-costTable.Metadatastartup and/readyzprobe, and CLI entrypoint cleanup handling. Stacked on #1405 (feat/metrics-ingester-bigquery-writer).Motivation & Context
With
BigQueryWriterandNormalizeMetricsin place,cmd/metrics_ingesterneeds runtime configuration, Storage Write API stream lifecycle management, and Kubernetes readiness probing so operators can runPub/Sub -> BigQuerywithout deploying ClickHouse. Following the repository's Two-Tier Composition Root architecture keeps Cloud SDK client construction (bigquery.Client,managedwriter.Client,ManagedStream) isolated inruntime.gowith deterministic LIFO rollback on startup errors and idempotentClose().For
/readyzand startup verification, probingTable.Metadata(ctx)verifies dataset/table existence and IAM permissions via a zero-cost metadata RPC (rather than executing billable BigQuery SQL queries every few seconds). Passing the uncancelled root context toNewManagedStreamwhile passing theSIGINT/SIGTERMcontext toIngester.Runensures the Storage Write stream stays open duringIngester's 10s shutdown flush beforedefer store.Close()runs.Key Changes
cmd/metrics_ingester/metricsingester/{config.go, config_test.go}: Adds-metrics_store(clickhouse|bigquery, defaultclickhouse),-bigquery_project_id(BIGQUERY_PROJECT_ID, falling back toProjectID),-bigquery_dataset(BIGQUERY_DATASET, defaultxmc_metrics), and-bigquery_table(BIGQUERY_TABLE, defaultxmc_metrics), rejecting unknown stores and slash-containing BigQuery resource IDs.cmd/metrics_ingester/metricsingester/{runtime.go, runtime_test.go, health.go, BUILD.bazel}: AddsRuntimeStore,OpenRuntimeStore, andOpenBigQueryStreamwith LIFO error rollback,sync.OnceClose()(filtering expectedio.EOFon healthyManagedStream.Close()), andtableMetadataPinger, tested hermetically against in-process fake BigQuery REST and gRPC Storage Write servers.cmd/metrics_ingester/{main.go, BUILD.bazel, README.md}: Adoptsos.Exit(mainExitCode())so deferred cleanups always run, wiresOpenRuntimeStore, and documents BigQuery configuration,roles/bigquery.dataEditorIAM requirements, and/readyzbehavior.Assumptions
-metrics_store/METRICS_STOREdefaults to"clickhouse"when unset — chosen to preserve zero-configuration backward compatibility for existing ClickHouse deployments — deployments running BigQuery without ClickHouse must explicitly setMETRICS_STORE=bigquery(or-metrics_store=bigquery), otherwise startup will attempt to dial ClickHouse./readyzprobes verify BigQuery reachability viaTable.Metadata(ctx)rather thanSELECT 1SQL queries — chosen becauseTable.Metadatachecks dataset/table existence and IAM permissions via a zero-cost metadata RPC without incurring per-probe BigQuery slot/scan charges — if the BigQuery Storage Write gRPC endpoint experiences an isolated outage while the BigQuery REST metadata endpoint stays healthy,/readyzwill remain green and surface errors onAppendRowsretries instead.OpenBigQueryStreambinds the long-lived Storage Write default stream tocontext.Background()rather than theSIGINT/SIGTERMsignal context — chosen so the write stream stays open whileIngester.Runflushes in-flight batches during its 10-second graceful shutdown window beforedefer store.Close()executes — startup readiness verification remains bounded bystartupPingTimeoutand the caller context.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-commitconfig.go(70/70) andhealth.go(31/31), and 98.1% (103/105) inruntime.go.Related Issues / Bugs
N/A
TAG=agy
CONV=0fafd02b-f3b1-4563-a73d-49ec0845afbd