Skip to content

feat(metrics): label pause/resume telemetry by fs_only #3425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bchalios merged 1 commit into from
Jul 28, 2026
+38 −6
Merged

feat(metrics): label pause/resume telemetry by fs_only #3425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/orchestrator/pkg/server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type Server struct {
uploadedBuilds *ttlcache.Cache[string, struct{}]
uploads *sandbox.Uploads
sandboxCreateDuration metric.Int64Histogram
sandboxPauseDuration metric.Int64Histogram
sandboxKilledCounter metric.Int64Counter
uploadFailedCounter metric.Int64Counter

Expand Down Expand Up @@ -146,6 +147,12 @@ func New(ctx context.Context, cfg ServiceConfig) (*Server, error) {
}
server.sandboxCreateDuration = sandboxCreateDuration

sandboxPauseDuration, err := telemetry.GetHistogram(meter, telemetry.PauseDurationHistogramName)
if err != nil {
return nil, fmt.Errorf("failed to register sandbox pause duration histogram: %w", err)
}
server.sandboxPauseDuration = sandboxPauseDuration

sandboxKilledCounter, err := telemetry.GetCounter(meter, telemetry.OrchestratorSandboxKilledCounterName)
if err != nil {
return nil, fmt.Errorf("failed to register sandbox kills counter: %w", err)
Expand Down
34 changes: 28 additions & 6 deletions packages/orchestrator/pkg/server/sandboxes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,19 @@ func (s *Server) Create(ctx context.Context, req *orchestrator.SandboxCreateRequ
defer childSpan.End()

isResume := req.GetSandbox().GetSnapshot()
// fsOnly is set at the resume fork below when this takes the filesystem-only
// reboot path (vs a memory restore), so create/resume e2e latency can be
// split reboot vs memory — mirroring the fs_only pause label. Combined with
// sandbox.resume: resume=false → fresh create; resume=true,fs_only=false →
// memory resume; resume=true,fs_only=true → filesystem-only reboot.
var fsOnly bool
createStart := time.Now()
defer func() {
if createErr != nil {
return
}

s.sandboxCreateDuration.Record(ctx, time.Since(createStart).Milliseconds(),
metric.WithAttributes(
attribute.Bool("sandbox.resume", isResume),
attribute.Bool("fs_only", fsOnly),
attribute.Bool("success", createErr == nil),
),
)
}()
Expand Down Expand Up @@ -228,6 +232,7 @@ func (s *Server) Create(ctx context.Context, req *orchestrator.SandboxCreateRequ

var sbx *sandbox.Sandbox
if meta.IsFilesystemOnly() {
fsOnly = true
sbx, err = s.sandboxFactory.RebootSandbox(
ctx,
template,
Expand Down Expand Up @@ -645,10 +650,23 @@ func recordSandboxKill(ctx context.Context, counter metric.Int64Counter, killRea
counter.Add(ctx, 1, metric.WithAttributes(attribute.String("kill_reason", killReason)))
}

func (s *Server) Pause(ctx context.Context, in *orchestrator.SandboxPauseRequest) (*orchestrator.SandboxPauseResponse, error) {
func (s *Server) Pause(ctx context.Context, in *orchestrator.SandboxPauseRequest) (resp *orchestrator.SandboxPauseResponse, err error) {
ctx, childSpan := tracer.Start(ctx, "sandbox-pause")
defer childSpan.End()

// Record pause duration split by fs_only vs memory (the gRPC RPC metric
// can't distinguish them) and success, so dashboards can scope pause
// call-count / error-rate / latency to filesystem-only pauses.
pauseStart := time.Now()
defer func() {
s.sandboxPauseDuration.Record(ctx, time.Since(pauseStart).Milliseconds(),
metric.WithAttributes(
attribute.Bool("fs_only", in.GetFilesystemOnly()),
attribute.Bool("success", err == nil),

@kalyazin kalyazin Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly a part of this, but Claude is telling me that we don't differentiate success/failure for Create like we do for Pause. Seems like we can make it uniform via:

        defer func() { 
                s.sandboxCreateDuration.Record(ctx, time.Since(createStart).Milliseconds(),
                        metric.WithAttributes(
                                attribute.Bool("sandbox.resume", isResume),
                                attribute.Bool("fs_only", fsOnly),
                                attribute.Bool("success", createErr == nil),
                        ),
                )
        }()

@bchalios bchalios Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — done in 35ff850. create.duration now records on every exit with a success attribute (matching pause.duration), so create/resume error rate is queryable too, and pause/create are uniform.

One behavioral note: it now also records failed creates (previously success-only), so any existing create-latency panel that wants the old semantics should add success="true".

@bchalios bchalios Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied your snippet exactly in 35ff85035 — create.duration now records on every exit with sandbox.resume + fs_only + success, uniform with pause.duration, so create/resume error-rate is queryable too.

One behavioral change to flag: it now records failed creates as well (previously success-only via an early return), so any existing create-latency panel that wants the old semantics should filter success="true".

Reopened the thread so it's yours to close — lmk if the semantics look right (and whether you'd like the success="true" filter added to the current create.duration dashboard panels as part of the follow-up).

),
)
}()

childSpan.SetAttributes(
telemetry.WithSandboxID(in.GetSandboxId()),
telemetry.WithTemplateID(in.GetTemplateId()),
Expand Down Expand Up @@ -948,6 +966,9 @@ type snapshotResult struct {
// with. The prefetch harvest reuses it verbatim when re-uploading the
// metadata object, so the two can never drift.
objectMetadata storage.ObjectMetadata
// filesystemOnly records whether this was a filesystem-only (memoryless)
// pause, so the async upload can label its failure counter with fs_only.
filesystemOnly bool
}

// snapshotAndCacheSandbox creates a snapshot of a sandbox and adds it to the
Expand Down Expand Up @@ -1050,6 +1071,7 @@ func (s *Server) snapshotAndCacheSandbox(
upload: upload,
completeUpload: completeUpload,
objectMetadata: objectMetadata,
filesystemOnly: filesystemOnly,
}, nil
}

Expand Down Expand Up @@ -1084,7 +1106,7 @@ func (s *Server) uploadSnapshotAsync(ctx context.Context, sbx *sandbox.Sandbox,
)
if err != nil {
sbxlogger.I(sbx).Error(spanCtx, "snapshot upload did not durably land", zap.Error(err))
s.uploadFailedCounter.Add(spanCtx, 1)
s.uploadFailedCounter.Add(spanCtx, 1, metric.WithAttributes(attribute.Bool("fs_only", res.filesystemOnly)))
} else {
sbxlogger.I(sbx).Info(spanCtx, "snapshot finished uploading successfully")
}
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/pkg/telemetry/meters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ const (
OrchestratorSandboxCreateDurationName HistogramType = "orchestrator.sandbox.create.duration"
WaitForEnvdDurationHistogramName HistogramType = "orchestrator.sandbox.envd.init.duration"
GuestSyncDurationHistogramName HistogramType = "orchestrator.sandbox.guest_sync.duration"
PauseDurationHistogramName HistogramType = "orchestrator.sandbox.pause.duration"

// Pre-pause envd heap collapse round-trip duration (the pause-path cost of
// POST /collapse: network plus envd's madvise work), recorded once per pause
Expand Down Expand Up @@ -451,6 +452,7 @@ var histogramDesc = map[HistogramType]string{
WaitForEnvdDurationHistogramName: "Time taken for Envd to initialize successfully",
EnvdCollapseDurationHistogramName: "Time taken for the pre-pause envd heap collapse round-trip",
GuestSyncDurationHistogramName: "Time taken for the mandatory pre-pause guest sync (filesystem-only pause)",
PauseDurationHistogramName: "Time taken to pause a sandbox, labeled by fs_only (filesystem-only vs memory) and success",

PauseResumePrefetchHarvestDurationName: "Time taken for a pause-resume prefetch harvest run (slot-hold cost)",
PauseResumePrefetchHarvestPagesName: "Harvested resume-prefetch trace size in 2 MiB blocks, per successful harvest",
Expand Down Expand Up @@ -501,6 +503,7 @@ var histogramUnits = map[HistogramType]string{
WaitForEnvdDurationHistogramName: "ms",
EnvdCollapseDurationHistogramName: "ms",
GuestSyncDurationHistogramName: "ms",
PauseDurationHistogramName: "ms",
PauseResumePrefetchHarvestDurationName: "ms",
PauseResumePrefetchHarvestPagesName: "{page}",
UffdStartupPagesHistogramName: "{page}",
Expand Down
Loading