Skip to content

chore: extend the default otelgrpc metric with resume#2293

Merged
jakubno merged 3 commits intomainfrom
fix/is-resume-metric
Apr 2, 2026
Merged

chore: extend the default otelgrpc metric with resume#2293
jakubno merged 3 commits intomainfrom
fix/is-resume-metric

Conversation

@jakubno
Copy link
Copy Markdown
Member

@jakubno jakubno commented Apr 2, 2026

No description provided.

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 2, 2026

PR Summary

Low Risk
Low risk: this only plumbs a boolean via gRPC metadata and opt-in server configuration to enrich OpenTelemetry metrics, without changing request handling or persistence behavior.

Overview
Extends gRPC telemetry so orchestrator RPC metrics can be tagged with whether a sandbox create is a resume (sandbox.resume). The API now sends the snapshot/resume flag as outgoing gRPC metadata (x-sandbox-resume), and the shared gRPC server can be configured (enabled for the orchestrator) to read that metadata during TagRPC and attach it as an otelgrpc metric attribute.

Written by Cursor Bugbot for commit c2f8b50. This will update automatically on new commits. Configure here.

@@ -42,6 +45,7 @@ func NewGRPCServer(tel *telemetry.Client) *grpc.Server {
otelgrpc.NewServerHandler(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NewGRPCServer is used by both the orchestrator and the API server (packages/api/main.go:420). Adding extractIsResume here means it also runs on the API gRPC server, which serves external/proxy clients. Those clients can send arbitrary x-sandbox-resume metadata, causing the API otelgrpc metrics to reflect attacker-controlled sandbox.resume label values (metrics poisoning). Since this attribute is only meaningful on the orchestrator side, consider accepting metric attribute functions as a variadic option in NewGRPCServer, so the orchestrator can pass extractIsResume without it being applied to all gRPC servers.

@jakubno jakubno assigned dobrac and unassigned dobrac and bchalios Apr 2, 2026
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — clean, focused observability addition.

Extended reasoning...

Overview

This PR adds a sandbox.resume metric attribute to the default otelgrpc instrumentation. It touches three files: metadata.go (API client side, sets the metadata key), filter.go (shared grpc package, defines the constant), and server.go (shared grpc package, reads the metadata and returns the attribute via WithMetricAttributesFn).

Security Risks

No security risks. The change only reads an already-present boolean value from incoming gRPC metadata and attaches it as a telemetry attribute. There is no new auth surface, no user-controlled data reaching sensitive paths, and metadata is already trusted internal infrastructure traffic.

Level of Scrutiny

Low scrutiny needed. This is purely observability plumbing — a new constant, a simple boolean read, and a single otelgrpc option. The pattern of using WithMetricAttributesFn is the documented otelgrpc extension point. The fallback cases (!ok, len==0) return nil safely.

Other Factors

No bugs found by the automated system. The change is self-contained, follows the existing grpc package conventions, and has minimal blast radius — worst case a metric attribute is missing or incorrect, which does not affect runtime behavior.

@jakubno jakubno enabled auto-merge (squash) April 2, 2026 14:20
@jakubno jakubno merged commit 2b29315 into main Apr 2, 2026
65 of 66 checks passed
@jakubno jakubno deleted the fix/is-resume-metric branch April 2, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants