fix(bigtable): eliminate stats-handler MD race in internal/metrics tracer (#20158) ## Summary - Hoists per-attempt cluster/zone/transport/server-latency extraction out of `HandleRPC`'s `stats.End` branch and into a new `ingestMetadata` helper that runs on the `InHeader`/`InTrailer` dispatches. `End` now reads only already-parsed primitives — never the raw `metadata.MD` — so the cross-goroutine race on the map is gone. - Serializes the primitive-field writes vs reads with a per-`Tracer` `sync.Mutex`. Only held around the field access, never across `metric.Record` calls. `CreateTracer` now returns `*Tracer` so the embedded mutex isn't copied out of the factory. - Adds a deterministic `-race` regression test (`tracer_race_test.go`) that reproduced the failing interleave on the pre-fix code and passes cleanly on the fix. ## Race being fixed Reported in the wild by the nightly integration suite; race dump captured from `TestIntegration_Presidents`: ``` WARNING: DATA RACE Read at 0x…5ac8 by goroutine 5793 (csAttempt.finish → HandleRPC[*stats.End]) bigtable/internal/metrics/tracer.go:867 Previous write at 0x…5ac8 by goroutine 1478 (http2Client.operateHeaders → HandleRPC[*stats.InTrailer]) bigtable/internal/metrics/tracer.go:859 ``` Two follow-on races on the same run against the underlying `metadata.MD` map (`MD.Copy` in `operateHeaders` vs `MD.Get` in `extractLocation`). All three came from the same design flaw: `HandleRPC` stored `ev.Header` / `ev.Trailer` on the current attempt from `InHeader` / `InTrailer`, then re-read them under `stats.End`. gRPC's `stats.Handler` contract does **not** promise `InHeader → InTrailer → End` dispatch on a single goroutine — under cancel / deadline / GOAWAY, `csAttempt.finish` (`google.golang.org/grpc@1.82.0/stream.go:1251`) fires `End` from the caller goroutine while the transport reader (`http2_client.go:1650`) is still processing the trailer frame. Also plausibly implicated in the FlakyBot P1 burst against `30b1dfa0db` on the same nightly (#20147, #20148, #20150, #20151, #20152), all of which are TestIntegration_* failures with no attached stack — the Sponge log is Google-internal, but the timing (all P1s stem from the same invocation, and the metrics refactor #20099 that introduced this pattern shipped six days earlier) strongly suggests the same underlying race. ## Design (why this exact fix) Option A of the several considered — the alternatives were: - **B: add a mutex around `metadata.MD.Get`** inside `RecordAttemptCompletion`. Fixes the primitive-field race but not the map-content race (gRPC's transport owns the map and mutates it inside `operateHeaders`). Rejected. - **C: plumb server headers/trailers through `stats.End`.** Requires a gRPC-side change. Not viable short-term. Option A eliminates cross-goroutine access to the MD entirely — the raw map is read exactly once on the goroutine that received it, then dropped. The mutex only guards the small set of already-parsed primitive fields that `End` needs to read. `extractLocation` / `extractPeerInfo` / `extractServerLatency` all check header first then trailer, so passing one MD at a time (with the `locationExtracted` / `peerInfoExtracted` booleans + the `serverLatency == 0` gate) preserves the exact header-preferred-trailer-fallback behaviour. ## Test plan - [x] `go test -race -count=5 -run TestHandleRPC_ConcurrentInTrailerAndEnd_NoRace -timeout=120s ./bigtable/internal/metrics/` — passes; same test tripped the race on the pre-fix code. - [x] `go test -race -count=1 -timeout=120s ./bigtable/internal/metrics/...` — full package suite passes under race. - [x] `go vet ./...` clean. - [x] `go build ./...` clean. - [ ] Follow-up: rerun the nightly integration suite to close out the sibling FlakyBot issues if the fix eliminates them.
Go packages for Google Cloud Platform services.
go get cloud.google.com/go/firestore@latest # Replace firestore with the package you want to use.
NOTE: Some of these packages are under development, and may occasionally make backwards-incompatible changes.
For an updated list of all of our released APIs please see our reference docs.
Our libraries are compatible with the two most recent major Go releases, the same policy the Go programming language follows. This means the currently supported versions are:
By default, each client library will use Application Default Credentials (ADC) to automatically configure the credentials used in calling the API endpoint. When using the libraries in a Google Cloud Platform environment such as Compute Engine, Kubernetes Engine, or App Engine, no additional authentication steps are necessary. See Authentication methods at Google and Authenticate for using client libraries for more information.
client, err := storage.NewClient(ctx)
For applications running elsewhere, such as your local development environment, you can use the gcloud auth application-default login command from the Google Cloud CLI to set user credentials in your local filesystem. Application Default Credentials will automatically detect these credentials. See Set up ADC for a local development environment for more information.
Alternately, you may need to provide an explicit path to your credentials. To authenticate using a service account key file, either set the GOOGLE_APPLICATION_CREDENTIALS environment variable to the path to your key file, or programmatically pass option.WithCredentialsFile to the NewClient function of the desired package. For example:
client, err := storage.NewClient(ctx, option.WithCredentialsFile("path/to/keyfile.json"))
You can exert even more control over authentication by using the credentials package to create an auth.Credentials. Then pass option.WithAuthCredentials to the NewClient function:
creds, err := credentials.DetectDefault(&credentials.DetectOptions{...})
...
client, err := storage.NewClient(ctx, option.WithAuthCredentials(creds))
Contributions are welcome. Please, see the CONTRIBUTING document for details.
Please note that this project is released with a Contributor Code of Conduct. By participating in this project you agree to abide by its terms. See Contributor Code of Conduct for more information.