Repository navigation
Conversation
…drift
Identifier fields were logged three ways across services — dotted
(sandbox.id, team.id, via the logger.With* helpers), snake_case
(team_id, build_id), and camelCase (teamID, buildID). A single concept
like a build ID appeared under 8+ spellings, so log queries and alerts had
to match each concept under several names, and cross-service correlation
(client-proxy instance_id <-> api sandbox.id <-> orchestrator instanceID)
needed manual mapping.
This standardises on the dotted "<noun>.id" convention the Vector pipeline
already normalises on (packages/local-dev/vector.toml), and prevents
regressions:
- fields.go: add canonical key constants (KeySandboxID, KeyTeamID, ...) as
the single source of truth; every With* helper now returns one of them.
- Convert all 24 raw zap.String("team_id"/"buildID"/...) identifier call
sites across api, orchestrator, client-proxy, clickhouse and dashboard-api
to the matching logger.With* helper.
- scripts/check-log-field-keys.sh: a grep gate that fails if a raw identifier
key reappears. golangci-lint's forbidigo cannot express this — it matches
the called function name, not a string-literal argument (verified), so a
script gate is the only option. Wired into Makefile (check-log-field-keys)
and the lint CI workflow.
- logger/sandbox/metadata.go: the deliberate instanceID/envID aliases kept
for the Vector migration window are marked "log-key-exempt:" and documented,
rather than silently tripping the gate.
- fields_keys_test.go: locks each helper to its canonical key, forbids
duplicates, and enforces lowercase-dotted (no snake_case/camelCase).
Vector migration window: vector.toml already reads the dotted keys
(."team.id", ."sandbox.id", ...) as well as the legacy instanceID/envID, so
nothing downstream breaks now. The legacy double-write in metadata.go can be
dropped once logs predating this change age out of retention.
AdaAibaby
requested review from
ValentaTomas,
ben-fornefeld,
dobrac and
jakubno
as code owners
October 23, 2026 08:42
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.
Closes #3738.
Problem
Identifier fields are logged three ways across services — dotted via the
logger.With*helpers (sandbox.id,team.id), snake_case (team_id,build_id), and camelCase (teamID,buildID). A build ID alone appears under 8+ spellings. The result: log queries/alerts must match each concept under several names, and cross-service correlation (client-proxyinstance_id↔apisandbox.id↔orchestratorinstanceID) is manual.Why a grep gate, not
forbidigo.golangci.ymlalready runsforbidigo, but it matches the called function name, not a string-literal argument. Verified in an isolated module: a rule onzapString("team_id"does not fire; a rule onzapStringfires on every call. So it cannot forbidzap.String("team_id", …). Hence a grep gate.Changes
fields.go: canonical key constants (KeySandboxID,KeyTeamID, …) as the single source of truth; everyWith*helper returns one of them. Standard is the dotted"<noun>.id"the Vector pipeline already normalises on (packages/local-dev/vector.toml).api,orchestrator,client-proxy,clickhouse,dashboard-apiconverted to the matchinglogger.With*helper (incl. twocluster_id/clusterIDsites the gate itself surfaced that a first manual pass missed).scripts/check-log-field-keys.sh: grep gate; fails if a raw identifier key reappears. Wired intoMakefile(make check-log-field-keys) and the lint CI workflow. Self-tested: injecting a raw key → exit 1, removing it → exit 0.logger/sandbox/metadata.go: the deliberateinstanceID/envIDaliases kept for the Vector migration window are markedlog-key-exempt:and documented, instead of silently tripping the gate.fields_keys_test.go: locks each helper to its canonical key, forbids duplicate keys, and enforces lowercase-dotted (no snake_case/camelCase).Verification
go build ./...clean for all 6 affected modules;go veton logger clean.go test ./pkg/logger/...green incl. the newTestCanonicalFieldKeys.Migration window (not in this PR)
vector.tomlalready reads both the dotted keys (."team.id",."sandbox.id", …) and the legacyinstanceID/envID, so nothing downstream breaks now. Themetadata.godouble-write can be removed once logs predating this change age out of retention; the matchingvector.tomlremap lines go with it./cc @jakubno @dobrac @ValentaTomas @arkamar @tvi @ryan-sana @ben-fornefeld Looking forward to your code review.