From 595408ba131ae2878848eb1d1ea5e68fc3b5d695 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:40:17 +0800 Subject: [PATCH] chore(logger): unify structured-log identifier keys and gate against drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ".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. --- .github/workflows/lint.yml | 15 +++++ Makefile | 4 ++ .../handlers/admin_kill_team_sandboxes.go | 2 +- .../api/internal/handlers/sandbox_resume.go | 3 +- .../api/internal/orchestrator/analytics.go | 2 +- packages/api/internal/orchestrator/client.go | 2 +- .../orchestrator/nodemanager/sandboxes.go | 4 +- .../template-manager/cancel_team_builds.go | 8 +-- .../template-manager/template_manager.go | 6 +- .../clickhouse/pkg/sandboxlogs/sandboxlogs.go | 2 +- packages/client-proxy/main.go | 2 +- packages/dashboard-api/main.go | 2 +- .../orchestrator/cmd/create-build/main.go | 2 +- packages/orchestrator/pkg/sandbox/sandbox.go | 4 +- packages/orchestrator/pkg/server/sandboxes.go | 4 +- .../build/phases/ensurefreedisk/builder.go | 2 +- packages/shared/pkg/logger/fields.go | 45 +++++++++---- .../shared/pkg/logger/fields_keys_test.go | 64 +++++++++++++++++++ .../shared/pkg/logger/sandbox/metadata.go | 8 ++- scripts/check-log-field-keys.sh | 61 ++++++++++++++++++ 20 files changed, 204 insertions(+), 38 deletions(-) create mode 100644 packages/shared/pkg/logger/fields_keys_test.go create mode 100755 scripts/check-log-field-keys.sh diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 88e62aa094..38774720cd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -93,10 +93,20 @@ jobs: # The command is terminated due to an error: [../../.golangci.yml] validate: compile schema: failing loading "https://golangci-lint.run/jsonschema/golangci.v2.11.jsonschema.json": Get "https://golangci-lint.run/jsonschema/golangci.v2.11.jsonschema.json": context deadline exceeded (Client.Timeout exceeded while awaiting headers) verify: 'false' + log-field-keys: + if: ${{ inputs.run-lint == true }} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Check canonical structured-log field keys + run: ./scripts/check-log-field-keys.sh + lint: needs: - detect-modules - golangci-lint + - log-field-keys if: ${{ always() }} runs-on: ubuntu-latest timeout-minutes: 5 @@ -119,4 +129,9 @@ jobs: exit 1 fi + if [[ "${{ needs.log-field-keys.result }}" != "success" ]]; then + echo "log-field-keys result: ${{ needs.log-field-keys.result }}" + exit 1 + fi + echo "lint succeeded" diff --git a/Makefile b/Makefile index 596d503d12..f5605aee90 100644 --- a/Makefile +++ b/Makefile @@ -145,6 +145,10 @@ fmt: lint: go work edit -json | jq -r '.Use[].DiskPath' | xargs -P 4 -I{} golangci-lint run {}/... --fix +.PHONY: check-log-field-keys +check-log-field-keys: + @scripts/check-log-field-keys.sh + .PHONY: generate-mocks generate-mocks: GOOS=linux mockery diff --git a/packages/api/internal/handlers/admin_kill_team_sandboxes.go b/packages/api/internal/handlers/admin_kill_team_sandboxes.go index 691e7135f4..62a45e7845 100644 --- a/packages/api/internal/handlers/admin_kill_team_sandboxes.go +++ b/packages/api/internal/handlers/admin_kill_team_sandboxes.go @@ -41,7 +41,7 @@ func (a *APIStore) PostAdminTeamsTeamIDSandboxesKill(c *gin.Context, teamID uuid } logger.L().Info(ctx, "Completed killing team sandboxes", - zap.String("teamID", teamID.String()), + logger.WithTeamID(teamID.String()), zap.Int("killed", killedCount), zap.Int("failed", failedCount), ) diff --git a/packages/api/internal/handlers/sandbox_resume.go b/packages/api/internal/handlers/sandbox_resume.go index e5c2cec730..09fc212a22 100644 --- a/packages/api/internal/handlers/sandbox_resume.go +++ b/packages/api/internal/handlers/sandbox_resume.go @@ -9,7 +9,6 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" "go.opentelemetry.io/otel/trace" - "go.uber.org/zap" "github.com/e2b-dev/infra/packages/api/internal/api" snapshotcache "github.com/e2b-dev/infra/packages/api/internal/cache/snapshots" @@ -135,7 +134,7 @@ func (a *APIStore) PostSandboxesSandboxIDResume(c *gin.Context, sandboxID api.Sa logger.WithSandboxID(sandboxID), logger.Time("end_time", sandboxData.EndTime), logger.Time("start_time", sandboxData.StartTime), - zap.String("node_id", sandboxData.NodeID), + logger.WithNodeID(sandboxData.NodeID), ) return diff --git a/packages/api/internal/orchestrator/analytics.go b/packages/api/internal/orchestrator/analytics.go index 0b04494786..e5b2b70879 100644 --- a/packages/api/internal/orchestrator/analytics.go +++ b/packages/api/internal/orchestrator/analytics.go @@ -176,7 +176,7 @@ func logSandboxCreated(ctx context.Context, sbx sandbox.Sandbox) { zap.String("end_time", endTimeStr), zap.String("auto_resume_policy", autoResumePolicy), zap.Bool("auto_pause", sbx.AutoPause), - zap.String("template_id", sbx.BaseTemplateID), + logger.WithTemplateID(sbx.BaseTemplateID), ) } diff --git a/packages/api/internal/orchestrator/client.go b/packages/api/internal/orchestrator/client.go index e60bfb77e2..8a4cf2d8d1 100644 --- a/packages/api/internal/orchestrator/client.go +++ b/packages/api/internal/orchestrator/client.go @@ -176,7 +176,7 @@ func (o *Orchestrator) getOrConnectNode(ctx context.Context, clusterID uuid.UUID logger.L().Warn(ctx, "Node not found in cache, attempting on-demand connection", logger.WithNodeID(nodeID), - zap.String("cluster_id", clusterID.String()), + logger.WithClusterID(clusterID), ) scopedKey := o.scopedNodeID(clusterID, nodeID) diff --git a/packages/api/internal/orchestrator/nodemanager/sandboxes.go b/packages/api/internal/orchestrator/nodemanager/sandboxes.go index 4fcb3a78d4..2bc9c1ed71 100644 --- a/packages/api/internal/orchestrator/nodemanager/sandboxes.go +++ b/packages/api/internal/orchestrator/nodemanager/sandboxes.go @@ -58,7 +58,7 @@ func (n *Node) GetOrphanCandidates(ctx context.Context) ([]sandbox.NodeSandbox, // running under some other identity. if sandboxID == "" { logger.L().Error(childCtx, "Skipping sandbox with no sandbox ID during node sync", - zap.String("team_id", rawTeamID), + logger.WithTeamID(rawTeamID), logger.WithNodeID(n.ID), ) @@ -69,7 +69,7 @@ func (n *Node) GetOrphanCandidates(ctx context.Context) ([]sandbox.NodeSandbox, if parseErr != nil { logger.L().Error(childCtx, "Skipping sandbox with unparseable team ID during node sync", zap.Error(parseErr), - zap.String("team_id", rawTeamID), + logger.WithTeamID(rawTeamID), logger.WithSandboxID(sandboxID), logger.WithNodeID(n.ID), ) diff --git a/packages/api/internal/template-manager/cancel_team_builds.go b/packages/api/internal/template-manager/cancel_team_builds.go index 3fb4b3dc9c..88dd35c6a5 100644 --- a/packages/api/internal/template-manager/cancel_team_builds.go +++ b/packages/api/internal/template-manager/cancel_team_builds.go @@ -54,8 +54,8 @@ func (tm *TemplateManager) CancelTeamBuilds(ctx context.Context, teamID uuid.UUI err := cancelBuild(ctx, tm, b, reason) if err != nil { logger.L().Error(ctx, "Failed to cancel build", - zap.String("buildID", b.BuildID.String()), - zap.String("templateID", b.TemplateID), + logger.WithBuildID(b.BuildID.String()), + logger.WithTemplateID(b.TemplateID), logger.WithTeamID(teamID.String()), zap.Error(err)) failedCount.Add(1) @@ -64,8 +64,8 @@ func (tm *TemplateManager) CancelTeamBuilds(ctx context.Context, teamID uuid.UUI } logger.L().Debug(ctx, "Successfully cancelled build", - zap.String("buildID", b.BuildID.String()), - zap.String("templateID", b.TemplateID), + logger.WithBuildID(b.BuildID.String()), + logger.WithTemplateID(b.TemplateID), logger.WithTeamID(teamID.String())) cancelledCount.Add(1) diff --git a/packages/api/internal/template-manager/template_manager.go b/packages/api/internal/template-manager/template_manager.go index da7b62034a..ea8d55e003 100644 --- a/packages/api/internal/template-manager/template_manager.go +++ b/packages/api/internal/template-manager/template_manager.go @@ -98,7 +98,7 @@ func (tm *TemplateManager) BuildsStatusPeriodicalSync(ctx context.Context) { go func(b queries.GetInProgressTemplateBuildsRow) { err := tm.BuildStatusSync(ctx, b.EnvBuild.ID, b.ActiveEnv.ID, clustersshared.WithClusterFallback(b.ActiveEnv.ClusterID), b.EnvBuild.ClusterNodeID) if err != nil { - logger.L().Error(ctx, "Error syncing build status", zap.Error(err), zap.String("buildID", b.EnvBuild.ID.String())) + logger.L().Error(ctx, "Error syncing build status", zap.Error(err), logger.WithBuildID(b.EnvBuild.ID.String())) } }(b) } @@ -123,7 +123,7 @@ func (tm *TemplateManager) GetAvailableBuildClient(ctx context.Context, clusterI if err != nil { if errors.Is(err, clusters.ErrAvailableTemplateBuilderNotFound) { // Fallback to any template builder - logger.L().Warn(ctx, "No available template builder found with the specified machine info, falling back to any available template builder", zap.String("clusterID", clusterID.String())) + logger.L().Warn(ctx, "No available template builder found with the specified machine info, falling back to any available template builder", logger.WithClusterID(clusterID)) builder, err = cluster.GetAvailableTemplateBuilder(ctx, machineinfo.MachineInfo{}) if err != nil { @@ -181,7 +181,7 @@ func (tm *TemplateManager) DeleteBuild(ctx context.Context, buildID uuid.UUID, t } nodeID = node.NodeID - logger.L().Info(ctx, "Fallback to available node", zap.String("nodeID", nodeID), zap.String("clusterID", clusterID.String())) + logger.L().Info(ctx, "Fallback to available node", logger.WithNodeID(nodeID), logger.WithClusterID(clusterID)) client, err = tm.GetClusterBuildClient(clusterID, nodeID) if err != nil { return fmt.Errorf("failed to get builder client: %w", err) diff --git a/packages/clickhouse/pkg/sandboxlogs/sandboxlogs.go b/packages/clickhouse/pkg/sandboxlogs/sandboxlogs.go index b9af8c1089..e6bb2a4c04 100644 --- a/packages/clickhouse/pkg/sandboxlogs/sandboxlogs.go +++ b/packages/clickhouse/pkg/sandboxlogs/sandboxlogs.go @@ -222,7 +222,7 @@ func (r *Reader) scanSandboxLogs(ctx context.Context, q string, args ...any) ([] entry := rr.toLogEntry(func(err error) { logger.L().Warn(ctx, "failed to parse sandbox log fields", - zap.String("sandbox_id", rr.SandboxID), + logger.WithSandboxID(rr.SandboxID), logger.Time("timestamp", rr.Timestamp), zap.Error(err), ) diff --git a/packages/client-proxy/main.go b/packages/client-proxy/main.go index bfd75b48c2..931ae10f00 100644 --- a/packages/client-proxy/main.go +++ b/packages/client-proxy/main.go @@ -99,7 +99,7 @@ func run() int { signalCtx, sigCancel := signal.NotifyContext(ctx, syscall.SIGTERM, syscall.SIGINT) defer sigCancel() - l.Info(ctx, "Starting client proxy", zap.String("commit", commitSHA), zap.String("instance_id", instanceID)) + l.Info(ctx, "Starting client proxy", zap.String("commit", commitSHA), logger.WithServiceInstanceID(instanceID)) featureFlagsClient, err := featureflags.NewClient(config.DeploymentEnvironment, serviceName, version, featureflags.WithStartOnInitTimeout()) if err != nil { diff --git a/packages/dashboard-api/main.go b/packages/dashboard-api/main.go index 39e42e01aa..40c4871b0e 100644 --- a/packages/dashboard-api/main.go +++ b/packages/dashboard-api/main.go @@ -120,7 +120,7 @@ func run() int { return 1 } - l.Info(ctx, "Starting dashboard-api service...", zap.String("commit_sha", commitSHA), zap.String("instance_id", serviceInstanceID)) + l.Info(ctx, "Starting dashboard-api service...", zap.String("commit_sha", commitSHA), logger.WithServiceInstanceID(serviceInstanceID)) expectedMigration, err := strconv.ParseInt(expectedMigrationTimestamp, 10, 64) if err != nil { diff --git a/packages/orchestrator/cmd/create-build/main.go b/packages/orchestrator/cmd/create-build/main.go index 99cea52e6d..1192904ae1 100644 --- a/packages/orchestrator/cmd/create-build/main.go +++ b/packages/orchestrator/cmd/create-build/main.go @@ -406,7 +406,7 @@ func doBuild( uploads, ) - l = l.With(zap.String("envID", templateID)).With(zap.String("buildID", buildID)) + l = l.With(logger.WithTemplateID(templateID)).With(logger.WithBuildID(buildID)) force := true diff --git a/packages/orchestrator/pkg/sandbox/sandbox.go b/packages/orchestrator/pkg/sandbox/sandbox.go index 02e351b627..c5f777707e 100644 --- a/packages/orchestrator/pkg/sandbox/sandbox.go +++ b/packages/orchestrator/pkg/sandbox/sandbox.go @@ -3348,7 +3348,7 @@ func (s *Sandbox) runDeferredMemoryExport( // when a window fails or the tripwire fires, and trace ids are // sampled — the sandbox/build identity must be on the line itself. sbxlogger.I(s).Error(ctx, "deferred memory export failed", - zap.String("build_id", buildID.String()), zap.Error(err)) + logger.WithBuildID(buildID.String()), zap.Error(err)) // Drain in-flight fault-path copies before closing the cache: Sweep // returns on cancel WITHOUT waiting them out. sharedCacheWriter's // lock makes a straggler fail cleanly (ErrCacheClosed) rather than @@ -3358,7 +3358,7 @@ func (s *Sandbox) runDeferredMemoryExport( window.CancelAndDrain(err) if closeErr := cache.Close(); closeErr != nil { sbxlogger.I(s).Warn(ctx, "closing memory export cache", - zap.String("build_id", buildID.String()), zap.Error(closeErr)) + logger.WithBuildID(buildID.String()), zap.Error(closeErr)) } // The SANDBOX is unharmed: a failed or cancelled capture loses only // THIS checkpoint's artifact. Unlike the rootfs seal (whose failure diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go index 6abe238390..8e24d3b597 100644 --- a/packages/orchestrator/pkg/server/sandboxes.go +++ b/packages/orchestrator/pkg/server/sandboxes.go @@ -2245,7 +2245,7 @@ func (s *Server) snapshotAndCacheSandbox( } if err := s.peerRegistry.Unregister(ctx, meta.Template.BuildID); err != nil { - logger.L().Warn(ctx, "failed to unregister peer address from routing", zap.String("build_id", meta.Template.BuildID), zap.Error(err)) + logger.L().Warn(ctx, "failed to unregister peer address from routing", logger.WithBuildID(meta.Template.BuildID), zap.Error(err)) } } @@ -2261,7 +2261,7 @@ func (s *Server) snapshotAndCacheSandbox( if peerEnabled { if err := s.peerRegistry.Register(ctx, meta.Template.BuildID, redisPeerKeyTTL); err != nil { - logger.L().Warn(ctx, "failed to register peer address for routing", zap.String("build_id", meta.Template.BuildID), zap.Error(err)) + logger.L().Warn(ctx, "failed to register peer address for routing", logger.WithBuildID(meta.Template.BuildID), zap.Error(err)) } } diff --git a/packages/orchestrator/pkg/template/build/phases/ensurefreedisk/builder.go b/packages/orchestrator/pkg/template/build/phases/ensurefreedisk/builder.go index 475d0ddba2..ac2af489fb 100644 --- a/packages/orchestrator/pkg/template/build/phases/ensurefreedisk/builder.go +++ b/packages/orchestrator/pkg/template/build/phases/ensurefreedisk/builder.go @@ -123,7 +123,7 @@ func (b *EnsureFreeDiskBuilder) Layer( logger.L().Info(ctx, "resize-disk artifact unavailable in cache, building it", zap.Error(err), zap.String("hash", hash), - zap.String("build_id", layerMeta.Template.BuildID), + logger.WithBuildID(layerMeta.Template.BuildID), ) return notCachedResult, nil //nolint:nilerr // An unavailable cached artifact must be rebuilt. diff --git a/packages/shared/pkg/logger/fields.go b/packages/shared/pkg/logger/fields.go index f59e05243c..907d402656 100644 --- a/packages/shared/pkg/logger/fields.go +++ b/packages/shared/pkg/logger/fields.go @@ -11,52 +11,73 @@ import ( "go.uber.org/zap/zapcore" ) +// Canonical structured-log field keys. These are the single source of truth +// for identifier field names across every service (api, orchestrator, +// client-proxy, ...). Always log an identifier through its With* helper below +// rather than a raw zap.String("...", v); scripts/check-log-field-keys.sh +// enforces this. The dotted ".id" convention is what the Vector pipeline +// normalizes on (packages/local-dev/vector.toml). +const ( + KeySandboxID = "sandbox.id" + KeyLifecycleID = "sandbox.lifecycle.id" + KeyTemplateID = "template.id" + KeyBuildID = "build.id" + KeyExecutionID = "execution.id" + KeyUserID = "user.id" + KeyTeamID = "team.id" + KeyNodeID = "node.id" + KeyClusterID = "cluster.id" + KeyEdgeTraceID = "edge.trace.id" + KeyServiceInstance = "service.instance.id" + KeySandboxIP = "sandbox.ip" +) + func WithSandboxID(sandboxID string) zap.Field { - return zap.String("sandbox.id", sandboxID) + return zap.String(KeySandboxID, sandboxID) } func WithLifecycleID(lifecycleID string) zap.Field { - return zap.String("sandbox.lifecycle.id", lifecycleID) + return zap.String(KeyLifecycleID, lifecycleID) } func WithTemplateID(templateID string) zap.Field { - return zap.String("template.id", templateID) + return zap.String(KeyTemplateID, templateID) } func WithBuildID(buildID string) zap.Field { - return zap.String("build.id", buildID) + return zap.String(KeyBuildID, buildID) } func WithExecutionID(executionID string) zap.Field { - return zap.String("execution.id", executionID) + return zap.String(KeyExecutionID, executionID) } func WithUserID(userID string) zap.Field { - return zap.String("user.id", userID) + return zap.String(KeyUserID, userID) } func WithTeamID(teamID string) zap.Field { - return zap.String("team.id", teamID) + return zap.String(KeyTeamID, teamID) } func WithNodeID(nodeID string) zap.Field { - return zap.String("node.id", nodeID) + return zap.String(KeyNodeID, nodeID) } func WithClusterID(clusterID uuid.UUID) zap.Field { - return zap.String("cluster.id", clusterID.String()) + return zap.String(KeyClusterID, clusterID.String()) } func WithEdgeTraceID(traceID string) zap.Field { - return zap.String("edge.trace.id", traceID) + return zap.String(KeyEdgeTraceID, traceID) } func WithServiceInstanceID(instanceID string) zap.Field { - return zap.String("service.instance.id", instanceID) + return zap.String(KeyServiceInstance, instanceID) } func WithSandboxIP(sandboxIP string) zap.Field { - return zap.String("sandbox.ip", sandboxIP) + return zap.String(KeySandboxIP, sandboxIP) } func WithEnvdVersion(envdVersion string) zap.Field { diff --git a/packages/shared/pkg/logger/fields_keys_test.go b/packages/shared/pkg/logger/fields_keys_test.go new file mode 100644 index 0000000000..cbd117a023 --- /dev/null +++ b/packages/shared/pkg/logger/fields_keys_test.go @@ -0,0 +1,64 @@ +package logger + +import ( + "testing" + + "github.com/google/uuid" + "go.uber.org/zap" +) + +// Locks the canonical structured-log field keys. If a helper's key changes, +// the Vector pipeline remap (packages/local-dev/vector.toml) and every log +// query/alert that matches these keys must change with it — so a silent edit +// here should fail loudly. +func TestCanonicalFieldKeys(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + field zap.Field + wantKey string + }{ + {"sandbox", WithSandboxID("x"), KeySandboxID}, + {"lifecycle", WithLifecycleID("x"), KeyLifecycleID}, + {"template", WithTemplateID("x"), KeyTemplateID}, + {"build", WithBuildID("x"), KeyBuildID}, + {"execution", WithExecutionID("x"), KeyExecutionID}, + {"user", WithUserID("x"), KeyUserID}, + {"team", WithTeamID("x"), KeyTeamID}, + {"node", WithNodeID("x"), KeyNodeID}, + {"cluster", WithClusterID(uuid.Nil), KeyClusterID}, + {"edgeTrace", WithEdgeTraceID("x"), KeyEdgeTraceID}, + {"serviceInstance", WithServiceInstanceID("x"), KeyServiceInstance}, + {"sandboxIP", WithSandboxIP("x"), KeySandboxIP}, + } + + // Guard against duplicate key strings across the canonical set. + seen := map[string]string{} + + for _, tt := range tests { + if tt.field.Key != tt.wantKey { + t.Errorf("%s: key = %q, want %q", tt.name, tt.field.Key, tt.wantKey) + } + + if prev, ok := seen[tt.wantKey]; ok { + t.Errorf("duplicate canonical key %q used by both %q and %q", tt.wantKey, prev, tt.name) + } + + seen[tt.wantKey] = tt.name + } + + // Every key uses the dotted ".id"/".ip" convention, not + // snake_case or camelCase, so queries can rely on a single spelling. + for key := range seen { + for _, c := range key { + if c >= 'A' && c <= 'Z' { + t.Errorf("canonical key %q must be lowercase dotted, found uppercase", key) + } + + if c == '_' { + t.Errorf("canonical key %q must use dots, not underscores", key) + } + } + } +} diff --git a/packages/shared/pkg/logger/sandbox/metadata.go b/packages/shared/pkg/logger/sandbox/metadata.go index 1156573f14..f15ecb3910 100644 --- a/packages/shared/pkg/logger/sandbox/metadata.go +++ b/packages/shared/pkg/logger/sandbox/metadata.go @@ -26,8 +26,10 @@ func (sm SandboxMetadata) Fields() []zap.Field { logger.WithTemplateID(sm.TemplateID), logger.WithTeamID(sm.TeamID), - // Fields for Vector - zap.String("instanceID", sm.SandboxID), - zap.String("envID", sm.TemplateID), + // Legacy aliases kept only for the Vector pipeline during the migration + // window; see packages/local-dev/vector.toml and the deprecation note + // in scripts/check-log-field-keys.sh. Do not add new raw ID keys here. + zap.String("instanceID", sm.SandboxID), // log-key-exempt: vector-legacy + zap.String("envID", sm.TemplateID), // log-key-exempt: vector-legacy } } diff --git a/scripts/check-log-field-keys.sh b/scripts/check-log-field-keys.sh new file mode 100755 index 0000000000..91d02895a3 --- /dev/null +++ b/scripts/check-log-field-keys.sh @@ -0,0 +1,61 @@ +#!/bin/bash +set -euo pipefail + +# Guard against raw structured-log identifier keys. +# +# Identifier fields (sandbox/team/template/build/node/cluster/user/execution/ +# service-instance IDs) must be logged through the canonical logger.With* helpers +# in packages/shared/pkg/logger/fields.go, which emit a single agreed ".id" +# key. Logging a raw zap.String("team_id", ...) / zap.String("teamID", ...) etc. +# reintroduces the key-name drift that forces log queries and alerts to match a +# concept under several spellings across api / orchestrator / client-proxy. +# +# golangci-lint's forbidigo cannot express this: it matches the called function +# name, not a string-literal argument, so a grep gate is used instead. +# +# A line may opt out with a trailing "log-key-exempt:" comment (used only for the +# Vector-legacy aliases in logger/sandbox/metadata.go during the migration +# window). Test files are not scanned. + +# Raw keys that each already have a canonical With* helper. Extend alongside +# fields.go. +FORBIDDEN='sandbox_id|sandboxID|sandboxId|team_id|teamID|teamId|template_id|templateID|templateId|build_id|buildID|buildId|node_id|nodeID|nodeId|cluster_id|clusterID|clusterId|instance_id|instanceID|env_id|envID' + +PATTERN="zap\\.String\\(\"($FORBIDDEN)\"" + +root="$(cd "$(dirname "$0")/.." && pwd)" +cd "$root" + +# All non-test Go files, excluding generated code. +mapfile -t files < <(git ls-files 'packages/**/*.go' | grep -vE '_test\.go$|\.gen\.go$|\.pb\.go$') + +hits=0 +for f in "${files[@]}"; do + while IFS= read -r line; do + # line is "lineno:content" + content="${line#*:}" + case "$content" in + *log-key-exempt:*) continue ;; + esac + printf '%s:%s\n' "$f" "$line" + hits=$((hits + 1)) + done < <(grep -nE "$PATTERN" "$f" 2>/dev/null || true) +done + +if [[ "$hits" -gt 0 ]]; then + cat >&2 <<'MSG' + +Raw structured-log identifier key(s) found above. + +Use the canonical helpers from packages/shared/pkg/logger instead, e.g.: + zap.String("team_id", id) -> logger.WithTeamID(id) + zap.String("build_id", id) -> logger.WithBuildID(id) + +This keeps one key per concept (team.id, build.id, ...) across every service. +If a line is a deliberate, documented exception, append a trailing comment +containing "log-key-exempt:". +MSG + exit 1 +fi + +echo "log field keys OK: no raw identifier keys found"