From b3dc49de1850e5a9b131b11832ecf9b9305c8706 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:00:17 +0800 Subject: [PATCH 1/2] obs(orchestrator): add teamID to valuable sandbox failure logs With 100K+ teams and growing, orchestrator failure logs that only carry the sandboxID cannot be filtered or aggregated by team at scale. This adds logger.WithTeamID to the error/warn logs where the team is already in scope (no new DB lookup, no signature change), following the same shape as #3674: - routing/publisher.go: failed to publish / delete sandbox routing record - sandbox/hoststats_collector.go: push baseline / collect sample / final sample - sandbox/oom.go: failed to seed the OOM kill watermark - sandbox/reboot.go: ignoring invalid reboot CPU template override - server/prefetch_harvest.go: resume prefetch harvest failed; stop/close throwaway - server/checkpoint_detached.go: left the live map before the kill teamID is taken from the runtime/metadata already held at each site (sbx.Runtime.TeamID / runtime.TeamID / h.metadata.TeamID). Logs whose scope has no team handle (fc process teardown, network slot release, cgroup FD release, template build) are intentionally left unchanged. Revives the intent of #3674 (closed) against current main, extended beyond the two create-path logs to the failure logs across the sandbox lifecycle. --- packages/orchestrator/pkg/routing/publisher.go | 2 ++ packages/orchestrator/pkg/sandbox/hoststats_collector.go | 3 +++ packages/orchestrator/pkg/sandbox/oom.go | 3 ++- packages/orchestrator/pkg/sandbox/reboot.go | 3 ++- packages/orchestrator/pkg/server/checkpoint_detached.go | 2 +- packages/orchestrator/pkg/server/prefetch_harvest.go | 5 +++-- 6 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/orchestrator/pkg/routing/publisher.go b/packages/orchestrator/pkg/routing/publisher.go index 4e0d986b18..387f0c2958 100644 --- a/packages/orchestrator/pkg/routing/publisher.go +++ b/packages/orchestrator/pkg/routing/publisher.go @@ -139,6 +139,7 @@ func (p *Publisher) OnInsert(ctx context.Context, sbx *sandbox.Sandbox) { p.publishCounter.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "error"))) logger.L().Error(ctx, "failed to publish sandbox routing record", logger.WithSandboxID(sbx.Runtime.SandboxID), + logger.WithTeamID(sbx.Runtime.TeamID), logger.WithLifecycleID(sbx.LifecycleID), zap.Error(err), ) @@ -184,6 +185,7 @@ func (p *Publisher) OnStopping(ctx context.Context, sbx *sandbox.Sandbox) { p.deleteCounter.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "error"))) logger.L().Error(ctx, "failed to delete sandbox routing record; it expires via TTL", logger.WithSandboxID(sbx.Runtime.SandboxID), + logger.WithTeamID(sbx.Runtime.TeamID), logger.WithLifecycleID(sbx.LifecycleID), zap.Error(err), ) diff --git a/packages/orchestrator/pkg/sandbox/hoststats_collector.go b/packages/orchestrator/pkg/sandbox/hoststats_collector.go index 0ec8082ff6..24eec581b7 100644 --- a/packages/orchestrator/pkg/sandbox/hoststats_collector.go +++ b/packages/orchestrator/pkg/sandbox/hoststats_collector.go @@ -139,6 +139,7 @@ func (h *HostStatsCollector) Start(ctx context.Context) { if err := h.delivery.Push(initial); err != nil { logger.L().Error(ctx, "failed to push initial host stats baseline", logger.WithSandboxID(h.metadata.SandboxID), + logger.WithTeamID(h.metadata.TeamID.String()), zap.Error(err)) } @@ -152,6 +153,7 @@ func (h *HostStatsCollector) Start(ctx context.Context) { // Log error but continue sampling - don't kill the sandbox logger.L().Error(ctx, "failed to collect host stats sample", logger.WithSandboxID(h.metadata.SandboxID), + logger.WithTeamID(h.metadata.TeamID.String()), zap.Error(err)) } case <-h.stopCh: @@ -173,6 +175,7 @@ func (h *HostStatsCollector) Stop(ctx context.Context) { // Log but don't fail the shutdown logger.L().Error(ctx, "failed to collect final host stats sample", logger.WithSandboxID(h.metadata.SandboxID), + logger.WithTeamID(h.metadata.TeamID.String()), zap.Error(err)) } }) diff --git a/packages/orchestrator/pkg/sandbox/oom.go b/packages/orchestrator/pkg/sandbox/oom.go index 818eb82130..c1b5d7d5dc 100644 --- a/packages/orchestrator/pkg/sandbox/oom.go +++ b/packages/orchestrator/pkg/sandbox/oom.go @@ -114,7 +114,8 @@ func (c *Checks) seedOOMWatermark(ctx context.Context) { logger.WithSandboxID(c.sandbox.Runtime.SandboxID)) case err != nil: logger.L().Warn(ctx, "Failed to seed the OOM kill watermark, kills until the first metrics poll will not be logged", - zap.Error(err), logger.WithSandboxID(c.sandbox.Runtime.SandboxID)) + zap.Error(err), logger.WithSandboxID(c.sandbox.Runtime.SandboxID), + logger.WithTeamID(c.sandbox.Runtime.TeamID)) } } diff --git a/packages/orchestrator/pkg/sandbox/reboot.go b/packages/orchestrator/pkg/sandbox/reboot.go index 4e51832289..ea008c7bcf 100644 --- a/packages/orchestrator/pkg/sandbox/reboot.go +++ b/packages/orchestrator/pkg/sandbox/reboot.go @@ -184,7 +184,8 @@ func (f *Factory) RebootSandbox( if err != nil { logger.L().Error(ctx, "ignoring invalid reboot CPU template override", zap.String("flag", featureflags.RebootCPUTemplateOverride.Key()), zap.Error(err), - logger.WithSandboxID(runtime.SandboxID)) + logger.WithSandboxID(runtime.SandboxID), + logger.WithTeamID(runtime.TeamID)) } if overridden { logger.L().Info(ctx, "reboot CPU template overridden", diff --git a/packages/orchestrator/pkg/server/checkpoint_detached.go b/packages/orchestrator/pkg/server/checkpoint_detached.go index 8b024b9816..1f6309e029 100644 --- a/packages/orchestrator/pkg/server/checkpoint_detached.go +++ b/packages/orchestrator/pkg/server/checkpoint_detached.go @@ -291,7 +291,7 @@ func (s *Server) killForAcknowledgedDelete(ctx context.Context, op *detachedChec } if !s.sandboxFactory.Sandboxes.MarkStopping(ctx, live.Runtime.SandboxID, live.LifecycleID) { logger.L().Warn(ctx, "sandbox a delete acknowledged during its checkpoint left the live map under another operation before the kill", - logger.WithSandboxID(sandboxID), logger.WithLifecycleID(live.LifecycleID), zap.String("kill_reason", killReason)) + logger.WithSandboxID(sandboxID), logger.WithTeamID(live.Runtime.TeamID), logger.WithLifecycleID(live.LifecycleID), zap.String("kill_reason", killReason)) return } diff --git a/packages/orchestrator/pkg/server/prefetch_harvest.go b/packages/orchestrator/pkg/server/prefetch_harvest.go index 1d8d5e8d34..c0655e6627 100644 --- a/packages/orchestrator/pkg/server/prefetch_harvest.go +++ b/packages/orchestrator/pkg/server/prefetch_harvest.go @@ -324,6 +324,7 @@ func (s *Server) harvestResumePrefetchAsync( span.SetStatus(codes.Error, err.Error()) logger.L().Warn(hCtx, "resume prefetch harvest failed", logger.WithSandboxID(sbx.Runtime.SandboxID), + logger.WithTeamID(sbx.Runtime.TeamID), logger.WithBuildID(buildID), zap.String("path", string(source)), zap.Error(err), @@ -521,10 +522,10 @@ func (h *prefetchHarvester) resumeMapping( reapCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), harvestReapTimeout) defer cancel() if stopErr := resumedSbx.Stop(reapCtx); stopErr != nil { - logger.L().Warn(reapCtx, "harvest: failed to stop throwaway", logger.WithSandboxID(runtime.SandboxID), zap.Error(stopErr)) + logger.L().Warn(reapCtx, "harvest: failed to stop throwaway", logger.WithSandboxID(runtime.SandboxID), logger.WithTeamID(runtime.TeamID), zap.Error(stopErr)) } if closeErr := resumedSbx.Close(reapCtx); closeErr != nil { - logger.L().Warn(reapCtx, "harvest: failed to close throwaway", logger.WithSandboxID(runtime.SandboxID), zap.Error(closeErr)) + logger.L().Warn(reapCtx, "harvest: failed to close throwaway", logger.WithSandboxID(runtime.SandboxID), logger.WithTeamID(runtime.TeamID), zap.Error(closeErr)) } }() From a08aaffee37e525bce21e6fb7b53f0f2c0bb7b86 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:07:03 +0800 Subject: [PATCH 2/2] obs(orchestrator): thread teamID into cgroup/fc-teardown/slot failure logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on within the same PR: the three failure logs that previously could not carry teamID without a struct/signature change now do. - sandbox.go releaseCgroupFD: add a teamID parameter (both call sites already hold runtime.TeamID) so the cgroup FD release warning is team-labelled. - fc/process.go teardown warnings (remove metrics FIFO, SIGTERM, SIGKILL): use the teamID the Process already holds (NewProcess receives it for metrics), so no new plumbing is needed. - network/slot.go: Slot gains a teamID field set by AssignSandbox(sandboxID, teamID) and cleared on release; getNetworkSlot threads runtime.TeamID through so the non-DNS-UDP slot log is team-labelled. Test call sites updated for the new signatures. Note on envd: the two envd-init error logs from the original #3674 are NOT touched here — on current main they log through s.log() (= Runtime.Logger()), whose LogFields() already emits WithTeamID on every line, so adding it explicitly would duplicate the field. The create-path log #3674 targeted is likewise already covered by the sites in the previous commit. Intentionally left out: fc vcpu telemetry spans (telemetry.ReportError, trace-level, already correlatable) and the sandboxtools command path (bare sandboxID threaded through ~10 call sites for low-value Debug/span lines). --- packages/orchestrator/pkg/sandbox/fc/process.go | 6 +++--- .../pkg/sandbox/network/firewall_udp_test.go | 4 ++-- packages/orchestrator/pkg/sandbox/network/slot.go | 8 +++++++- .../pkg/sandbox/network_slot_wait_test.go | 1 + packages/orchestrator/pkg/sandbox/sandbox.go | 12 +++++++----- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/orchestrator/pkg/sandbox/fc/process.go b/packages/orchestrator/pkg/sandbox/fc/process.go index 1547bc164b..ee22ac78f2 100644 --- a/packages/orchestrator/pkg/sandbox/fc/process.go +++ b/packages/orchestrator/pkg/sandbox/fc/process.go @@ -953,7 +953,7 @@ func (p *Process) Stop(ctx context.Context) error { // Always remove the metrics FIFO, even if the process already exited, // to avoid leaving orphaned files behind. if removeErr := os.Remove(p.metricsPath); removeErr != nil && !os.IsNotExist(removeErr) { - logger.L().Warn(ctx, "failed to remove fc metrics FIFO", zap.Error(removeErr), logger.WithSandboxID(p.files.SandboxID)) + logger.L().Warn(ctx, "failed to remove fc metrics FIFO", zap.Error(removeErr), logger.WithSandboxID(p.files.SandboxID), logger.WithTeamID(p.teamID)) } pid := p.cmd.Process.Pid @@ -980,7 +980,7 @@ func (p *Process) Stop(ctx context.Context) error { return nil } - logger.L().Warn(ctx, "failed to send SIGTERM to fc process", zap.Error(err), logger.WithSandboxID(p.files.SandboxID)) + logger.L().Warn(ctx, "failed to send SIGTERM to fc process", zap.Error(err), logger.WithSandboxID(p.files.SandboxID), logger.WithTeamID(p.teamID)) } termDeadline := time.NewTimer(10 * time.Second) @@ -1002,7 +1002,7 @@ func (p *Process) Stop(ctx context.Context) error { return nil } if killErr != nil && !errors.Is(killErr, os.ErrProcessDone) { - logger.L().Warn(ctx, "failed to send SIGKILL to fc process", zap.Error(killErr), logger.WithSandboxID(p.files.SandboxID)) + logger.L().Warn(ctx, "failed to send SIGKILL to fc process", zap.Error(killErr), logger.WithSandboxID(p.files.SandboxID), logger.WithTeamID(p.teamID)) } killDeadline := time.NewTimer(time.Second) diff --git a/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go b/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go index 699262e212..e71cd0a5c9 100644 --- a/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go +++ b/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go @@ -263,7 +263,7 @@ func TestFirewall_NonDNSUDP_ResetRestoresMode(t *testing.T) { //nolint:parallelt require.Equal(t, dropChain, filterChainShapes(t, slot.NamespaceID()), "a tenant's rules build with the current mode") reader := swapNonDNSUDPCounter(t) - slot.AssignSandbox("sandbox-reset") + slot.AssignSandbox("sandbox-reset", "team-reset") mode.Store("count") require.NoError(t, slot.ResetInternet(t.Context())) @@ -392,7 +392,7 @@ func TestSlot_WithoutFirewallRefusesEveryOperation(t *testing.T) { slot, err := NewSlot("fw-no-firewall", 1, Config{}, NewNoopEgressProxy()) require.NoError(t, err) - slot.AssignSandbox("sandbox-no-firewall") + slot.AssignSandbox("sandbox-no-firewall", "team-no-firewall") withRules := &orchestrator.SandboxNetworkConfig{ Egress: &orchestrator.SandboxNetworkEgressConfig{DeniedCidrs: []string{"10.0.0.0/8"}}, diff --git a/packages/orchestrator/pkg/sandbox/network/slot.go b/packages/orchestrator/pkg/sandbox/network/slot.go index 401c5552df..2e323c8c4f 100644 --- a/packages/orchestrator/pkg/sandbox/network/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/slot.go @@ -111,6 +111,9 @@ type Slot struct { // synchronization point: the owning sandbox sets it and the pool's // return path, which runs after, reads and clears it. sandboxID string + // teamID labels the tenant's team on the slot's log lines; set and + // cleared alongside sandboxID. + teamID string // handedOut is set by the pool from the take to the return, so a mode // re-application leaves a slot on its way back to its release. Written @@ -272,8 +275,9 @@ func (s *Slot) UseNonDNSUDPMode(provider NonDNSUDPModeProvider) { } // AssignSandbox names the tenant the slot was handed to. -func (s *Slot) AssignSandbox(sandboxID string) { +func (s *Slot) AssignSandbox(sandboxID, teamID string) { s.sandboxID = sandboxID + s.teamID = teamID } // exportNonDNSUDP reports every non-empty tally the firewall harvested for @@ -288,6 +292,7 @@ func (s *Slot) exportNonDNSUDP(ctx context.Context, fw *Firewall) { recordNonDNSUDP(ctx, tally.Packets, chain, s.config.NetworkVersion) logger.L().Info(ctx, "slot firewall saw non-DNS UDP from the guest", logger.WithSandboxID(s.sandboxID), + logger.WithTeamID(s.teamID), zap.Int("slot_index", s.Idx), zap.String("mode", string(chain.Mode)), zap.String("posture", chain.Posture), @@ -664,6 +669,7 @@ func (s *Slot) release(ctx context.Context, fw *Firewall) { s.exportNonDNSUDP(ctx, fw) s.firewallCustomRules.Store(false) s.sandboxID = "" + s.teamID = "" s.tenantMode = nil s.tenantEgress = nil } diff --git a/packages/orchestrator/pkg/sandbox/network_slot_wait_test.go b/packages/orchestrator/pkg/sandbox/network_slot_wait_test.go index 54f2b3757d..a532ef42fc 100644 --- a/packages/orchestrator/pkg/sandbox/network_slot_wait_test.go +++ b/packages/orchestrator/pkg/sandbox/network_slot_wait_test.go @@ -125,6 +125,7 @@ func TestGetNetworkSlotUsesReturnedSlotBeforeProducerPublishes(t *testing.T) { / sandboxes.NetworkRelease("pool-wait-replacement", "replacement-lifecycle"), sandboxtypes.EgressClassSandbox, "pool-wait-replacement", + "pool-wait-team", ) synctest.Wait() select { diff --git a/packages/orchestrator/pkg/sandbox/sandbox.go b/packages/orchestrator/pkg/sandbox/sandbox.go index 02e351b627..30a635059d 100644 --- a/packages/orchestrator/pkg/sandbox/sandbox.go +++ b/packages/orchestrator/pkg/sandbox/sandbox.go @@ -1158,7 +1158,7 @@ func (f *Factory) CreateSandbox( if err != nil { return nil, err } - defer releaseCgroupFD(ctx, cgroupHandle, runtime.SandboxID) + defer releaseCgroupFD(ctx, cgroupHandle, runtime.SandboxID, runtime.TeamID) cgroupRemovalRegistered := false defer cleanupIfNotRegistered(ctx, &e, &cgroupRemovalRegistered, cgroupHandle.Remove) @@ -1588,7 +1588,7 @@ func (f *Factory) ResumeSandbox( if cgroupErr != nil { return nil, cgroupErr } - defer releaseCgroupFD(ctx, cgroupHandle, runtime.SandboxID) + defer releaseCgroupFD(ctx, cgroupHandle, runtime.SandboxID, runtime.TeamID) cgroupRemovalRegistered := false defer cleanupIfNotRegistered(ctx, &e, &cgroupRemovalRegistered, cgroupHandle.Remove) @@ -4515,7 +4515,7 @@ func (f *Factory) getNetworkSlot( runtime sandboxtypes.RuntimeMetadata, lifecycleID string, ) *utils.Promise[*network.Slot] { - return getNetworkSlot(ctx, f.networkPool, cleanup, config.Network, f.Sandboxes.NetworkRelease(runtime.SandboxID, lifecycleID), runtime.SandboxType.EgressClass(), runtime.SandboxID) + return getNetworkSlot(ctx, f.networkPool, cleanup, config.Network, f.Sandboxes.NetworkRelease(runtime.SandboxID, lifecycleID), runtime.SandboxType.EgressClass(), runtime.SandboxID, runtime.TeamID) } func getNetworkSlot( @@ -4526,6 +4526,7 @@ func getNetworkSlot( networkReleased network.ReleaseNotify, egressClass sandboxtypes.EgressClass, sandboxID string, + teamID string, ) *utils.Promise[*network.Slot] { return utils.NewPromise(func() (*network.Slot, error) { ctx, span := tracer.Start(ctx, "get network-slot") @@ -4535,7 +4536,7 @@ func getNetworkSlot( if err != nil { return nil, fmt.Errorf("failed to get network slot: %w", err) } - slot.AssignSandbox(sandboxID) + slot.AssignSandbox(sandboxID, teamID) cleanup.Add(ctx, func(ctx context.Context) error { // Async so sandbox cleanup doesn't block on the return delay or @@ -4710,10 +4711,11 @@ func (s *Sandbox) envdInitLoop() envdInitLoop { return envdInitLoopLegacy } -func releaseCgroupFD(ctx context.Context, cgroupHandle *cgroup.CgroupHandle, sandboxID string) { +func releaseCgroupFD(ctx context.Context, cgroupHandle *cgroup.CgroupHandle, sandboxID, teamID string) { if releaseErr := cgroupHandle.ReleaseCgroupFD(); releaseErr != nil { logger.L().Warn(ctx, "failed to release cgroup directory FD", logger.WithSandboxID(sandboxID), + logger.WithTeamID(teamID), zap.Error(releaseErr)) } }