From 9ace548fd21536f83ca5dd8181bf978bc2a723ea Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Thu, 6 Aug 2026 22:31:15 -0700 Subject: [PATCH] feat(observability): rename sandbox provider spans and add run-time wrapper spans (#10999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip uses adapter and sandbox code to start agents and run sandbox work > - The current sandbox spans use mixed names and do not group related run-time work > - Mixed names make traces harder to read and compare across providers > - This pull request renames provider spans, adds run-time wrapper spans, and keeps the host allowlist closed > - The benefit is clearer traces with the same sandbox behavior and trust boundary ## Linked Issues or Issue Description **What existing behavior does this improve?** This improves OpenTelemetry span names and grouping for sandbox startup, execution, callback relay, and agent session work. **Subsystem affected** Cross-cutting (multiple of the above): adapter utilities, sandbox providers, shared telemetry documentation, and server instrumentation. **Current behavior** Sandbox provider spans use mixed names. Related run-time operations expose inner `sandbox.exec` spans without a named wrapper span. The host mapper uses a closed allowlist for provider span names. **Proposed behavior** Use descriptive provider-scoped span names. Add wrapper spans for agent session input, agent session output polling, and callback relay. Keep the host mapper allowlist closed and map unknown names to `other`. **Reason and benefit** Clear names make traces easier to read and reduce ambiguity during sandbox operation analysis. Wrapper spans show the full operation while preserving the inner execution spans. **Breaking changes** None. This change updates telemetry span names and grouping only. It does not change sandbox behavior, endpoint behavior, or the host trust boundary. **Additional context** Related prior work: [#10758](https://github.com/paperclipai/paperclip/pull/10758). ## What Changed - Rename Daytona provider sync and session spans with descriptive provider-scoped names. - Add three run-time wrapper spans for agent session input, output polling, and callback relay. - Add a shared span runner that preserves no-op behavior without a real tracer. - Keep the host mapper allowlist closed and map unknown names to `other`. - Update telemetry documentation and span-name tests. ## Verification - Focused adapter-utils span tests pass for startup timing, callback relay, and sandbox execution. - Focused Daytona plugin span tests pass for renamed leaf spans and session open or close spans. - Focused server tests pass for host mapping and instrumentation. - The stacked diff contains one commit on top of `feat/daytona-persistent-session-model`. ## Risks - Span names change for existing telemetry consumers. - The wrapper spans add trace structure but do not change sandbox execution. - The host mapper keeps the existing closed allowlist and `other` bucket. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 (Codex agent); exact deployment revision and context window are not exposed in this run; tool use and code execution enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .../adapter-utils/src/acpx-engine/execute.ts | 18 ++- .../src/acpx-engine/startup-timing.test.ts | 61 ++++++++- .../src/acpx-engine/startup-timing.ts | 60 +++++++++ .../src/execution-target-sandbox.test.ts | 125 ++++++++++++++++++ .../adapter-utils/src/execution-target.ts | 82 ++++++++---- .../src/sandbox-callback-bridge.test.ts | 48 +++++++ .../src/sandbox-callback-bridge.ts | 34 +++-- .../daytona/src/file-sync.ts | 32 +++-- .../daytona/src/plugin.test.ts | 56 +++++--- .../sandbox-providers/daytona/src/plugin.ts | 20 +-- packages/shared/src/telemetry/README.md | 44 ++++-- .../src/__tests__/environment-runtime.test.ts | 2 +- server/src/__tests__/instrumentation.test.ts | 6 +- .../plugin-host-services-span.test.ts | 40 +++--- server/src/services/plugin-host-services.ts | 34 ++--- 15 files changed, 539 insertions(+), 123 deletions(-) diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 868df3897f..a43c2543e1 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -84,12 +84,14 @@ import { DEFAULT_ACP_ENGINE_WARM_HANDLE_IDLE_MS, } from "./constants.js"; import { + createRuntimeSpanRunner, emitSkippedStartupStep, measureStartupStep, NOOP_STARTUP_SPAN, NOOP_STARTUP_TRACE_CONTEXT, runWithRuntimeParent, setSandboxRootSpanAttributes, + type RuntimeSpanRunner, type SandboxRootSpanContext, type StartupSpan, type StartupSpanContext, @@ -1367,6 +1369,12 @@ async function buildRuntime(input: { // run closure passes the run-scoped getter here; when it is absent, each // bridge site keeps its earlier unparented run-time behavior. getRuntimeParentContext?: () => StartupSpanContext | undefined; + // Wrap each unit of bridge run-time work in its own named span. + // `buildRuntime` threads it into the two remote bridge factories, so the + // socket handler, the poll loop, and the callback worker each open a wrapper + // span per unit of work. The run closure passes the run-scoped runner here; + // when it is absent, each bridge site opens no wrapper span. + runtimeSpan?: RuntimeSpanRunner; }): Promise { const { runId, agent, config, context, authToken } = input.ctx; // Injectable monotonic clock for per-step startup timing. Hoisted above the @@ -1963,6 +1971,7 @@ async function buildRuntime(input: { hostApiToken: env.PAPERCLIP_API_KEY, onLog: input.ctx.onLog, getRuntimeParentContext: input.getRuntimeParentContext, + runtimeSpan: input.runtimeSpan, }), concurrentBridgeStepMetrics, ); @@ -1994,6 +2003,7 @@ async function buildRuntime(input: { timeoutSec, onLog: input.ctx.onLog, getRuntimeParentContext: input.getRuntimeParentContext, + runtimeSpan: input.runtimeSpan, }), concurrentBridgeStepMetrics, ); @@ -3131,6 +3141,12 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { // reads it through `getRuntimeParentContext` to parent to the live span. let currentRunParentContext: StartupSpanContext | undefined = runRootSpan.parentContext; const getRuntimeParentContext = (): StartupSpanContext | undefined => currentRunParentContext; + // Wrap each unit of bridge run-time work (one outbound ACP message, one poll + // tick, one callback request) in its own named span, parented to the live run + // span. The runner reads the run parent per call through + // `getRuntimeParentContext`, so a wrapper span always parents to the current + // run span. On a no-op trace context the runner opens no real span. + const runRuntimeSpan = createRuntimeSpanRunner(tracing, getRuntimeParentContext); // `runFailed` marks the run root span status at end time. It stays `true` // until the run reaches a clean completed turn, so every failure and every // early exit closes the span with error status. @@ -3190,7 +3206,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { // parents to its step span. On a local or SSH target // `spanParent.parentContext` is a no-op token, so the wrap is inert. prepared = await runWithRuntimeParent(spanParent.parentContext, () => - buildRuntime({ ctx, engine, deps, spanParent, getRuntimeParentContext }), + buildRuntime({ ctx, engine, deps, spanParent, getRuntimeParentContext, runtimeSpan: runRuntimeSpan }), ); } catch (err) { rootSpan.end(true); diff --git a/packages/adapter-utils/src/acpx-engine/startup-timing.test.ts b/packages/adapter-utils/src/acpx-engine/startup-timing.test.ts index 1678282143..14c7e0bfe2 100644 --- a/packages/adapter-utils/src/acpx-engine/startup-timing.test.ts +++ b/packages/adapter-utils/src/acpx-engine/startup-timing.test.ts @@ -1,12 +1,14 @@ import { describe, expect, it, vi } from "vitest"; import type { AdapterRuntimeEvent } from "../types.js"; -import type { StartupSpan, StartupTracer } from "./startup-timing.js"; +import type { StartupSpan, StartupTraceContext, StartupTracer } from "./startup-timing.js"; import { clampSpanLabel, + createRuntimeSpanRunner, emitSkippedStartupStep, getActiveStepContext, measureStartupStep, NOOP_STARTUP_SPAN, + NOOP_STARTUP_TRACE_CONTEXT, normalizeProviderFamily, runWithoutActiveStep, runWithRuntimeParent, @@ -653,3 +655,60 @@ describe("clampSpanLabel", () => { expect(clampSpanLabel("stdout", "secret output")).toBeUndefined(); }); }); + +describe("createRuntimeSpanRunner", () => { + it("opens a named wrapper span and parents the wrapped work to it", async () => { + const { tracer, spans } = makeMockTracer(); + const runParent = { marker: "run-parent" }; + const traceContext: StartupTraceContext = { + tracer, + contextWithSpan: (span) => ({ span }), + }; + const run = createRuntimeSpanRunner(traceContext, () => runParent); + + let childParent: unknown = "unset"; + const result = await run("sandbox.agentSession.sendInput", async () => { + childParent = getActiveStepContext()?.parentContext; + return "ok"; + }); + + expect(result).toBe("ok"); + expect(spans).toHaveLength(1); + expect(spans[0]!.name).toBe("sandbox.agentSession.sendInput"); + expect(spans[0]!.endCount).toBe(1); + // The wrapped work parents to the wrapper span, not straight to the run + // parent, so the inner exec spans group under the wrapper span. + expect((childParent as { span?: unknown }).span).toBe(spans[0]); + }); + + it("marks the wrapper span failed and ends it when the work throws", async () => { + const { tracer, spans } = makeMockTracer(); + const traceContext: StartupTraceContext = { + tracer, + contextWithSpan: (span) => ({ span }), + }; + const run = createRuntimeSpanRunner(traceContext, () => ({ marker: "run-parent" })); + + await expect( + run("sandbox.callbackBridge.relayRequest", async () => { + throw new Error("boom"); + }), + ).rejects.toThrow(/boom/); + expect(spans).toHaveLength(1); + expect(spans[0]!.status?.code).toBe(2); + expect(spans[0]!.endCount).toBe(1); + }); + + it("runs the work unwrapped under a no-op trace context", async () => { + const run = createRuntimeSpanRunner(NOOP_STARTUP_TRACE_CONTEXT, () => ({ marker: "run-parent" })); + // The no-op `contextWithSpan` yields no child token, so the store empties for + // the work, exactly like the earlier unparented run-time behavior. + let childStep: unknown = "unset"; + const result = await run("sandbox.agentSession.pollOutput", async () => { + childStep = getActiveStepContext(); + return 7; + }); + expect(result).toBe(7); + expect(childStep).toBeNull(); + }); +}); diff --git a/packages/adapter-utils/src/acpx-engine/startup-timing.ts b/packages/adapter-utils/src/acpx-engine/startup-timing.ts index f49f3ccfd6..ce91ca6a5b 100644 --- a/packages/adapter-utils/src/acpx-engine/startup-timing.ts +++ b/packages/adapter-utils/src/acpx-engine/startup-timing.ts @@ -402,6 +402,66 @@ export function runWithRuntimeParent( return activeStepContextStorage.run(activeStep, work); } +/** + * Run one run-time operation inside its own wrapper span. The runner opens a + * wrapper span parented to the current run span, publishes the wrapper span as + * the runtime parent while `work` runs, and ends the span when `work` settles. + * A child `sandbox.exec` span inside `work` parents to the wrapper span, so the + * trace groups the operation's execs under one named span. A throwing `work` + * sets the wrapper span error status before the span ends. + * + * The runner reads the run parent per call, so it always parents to the live + * span (`agent.turn` during the turn, `task.run` otherwise). The default runner + * opens no real span; it only runs `work` under the current run parent, so the + * span path stays a no-op until the server injects a real tracer. + */ +export type RuntimeSpanRunner = (name: string, work: () => Promise) => Promise; + +/** + * Build a {@link RuntimeSpanRunner} from a trace context and the run-parent + * getter. The runner opens the wrapper span through `traceContext.tracer`, and + * it derives the wrapper span's child parent token through + * `traceContext.contextWithSpan`. A no-op trace context yields a runner that + * opens no real span and runs `work` under the current run parent, so the span + * path stays inert until the server injects a real tracer. Every tracer call + * sits inside an error swallow, so a throwing tracer never changes control flow. + */ +export function createRuntimeSpanRunner( + traceContext: StartupTraceContext, + getRuntimeParentContext: () => StartupSpanContext | undefined, +): RuntimeSpanRunner { + return async (name: string, work: () => Promise): Promise => { + const parentContext = getRuntimeParentContext(); + let span: StartupSpan; + try { + span = traceContext.tracer.startSpan(name, undefined, parentContext); + } catch { + // A throwing tracer must not change control flow; run `work` unwrapped. + return runWithRuntimeParent(parentContext, work); + } + let childContext: StartupSpanContext; + try { + childContext = traceContext.contextWithSpan(span); + } catch { + childContext = parentContext; + } + let failed = false; + try { + return await runWithRuntimeParent(childContext, work); + } catch (err) { + failed = true; + throw err; + } finally { + try { + if (failed) span.setStatus({ code: SPAN_STATUS_CODE_ERROR }); + span.end(); + } catch { + // Observability must not change control flow. + } + } + }; +} + /** * Set a numeric span attribute only when the value is a finite number. A reader * that returns `undefined` (the counter is unavailable) yields no attribute, diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index 321e88cb15..d4425a6ddf 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -481,6 +481,131 @@ describe("sandbox adapter execution targets", () => { } }); + it("wraps a stdin write in a sandbox.agentSession.sendInput span", async () => { + // With a span runner injected, the socket handler wraps one outbound ACP + // message to the agent in a `sandbox.agentSession.sendInput` span. This test + // connects a socket, sends one stdin line, and proves the handler opens that + // wrapper span around the write. + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-process-session-sendinput-span-")); + cleanupDirs.push(rootDir); + const childPath = path.join(rootDir, "noop-acp-child.mjs"); + await writeFile(childPath, "process.stdin.on('data', () => {});\n", "utf8"); + + const spanNames: string[] = []; + let resolveSendInput: () => void = () => {}; + const sendInputObserved = new Promise((resolve) => { + resolveSendInput = resolve; + }); + + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "local-test", + remoteCwd: rootDir, + timeoutMs: 30_000, + runner: createLocalSandboxRunner(), + }; + + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-process-session-sendinput-span", + target, + runtimeRootDir: path.posix.join(rootDir, ".paperclip-runtime", "acpx"), + adapterKey: "acpx", + command: process.execPath, + args: [childPath], + cwd: rootDir, + env: {}, + timeoutSec: 5, + onLog: async () => {}, + // Record each wrapper span name, then run the wrapped work. + runtimeSpan: async (name, work) => { + spanNames.push(name); + if (name === "sandbox.agentSession.sendInput") resolveSendInput(); + return work(); + }, + }); + expect(bridge).not.toBeNull(); + + let peer: net.Socket | null = null; + try { + const proxySource = await readFile(bridge!.agentCommand, "utf8"); + const port = Number(/port: (\d+)/.exec(proxySource)?.[1] ?? Number.NaN); + const tokenLiteral = /const token = (".*?");/.exec(proxySource)?.[1]; + const token = JSON.parse(tokenLiteral as string) as string; + + const peerSocket = net.createConnection({ host: "127.0.0.1", port }); + peer = peerSocket; + peerSocket.on("error", () => undefined); + await new Promise((resolve, reject) => { + peerSocket.once("connect", () => resolve()); + peerSocket.once("error", reject); + }); + + // The first token-bearing message authenticates and writes the stdin file. + peerSocket.write( + `${JSON.stringify({ token, type: "stdin", data: Buffer.from("hi").toString("base64") })}\n`, + ); + + await sendInputObserved; + expect(spanNames).toContain("sandbox.agentSession.sendInput"); + } finally { + peer?.destroy(); + await bridge?.stop(); + } + }); + + it("wraps each poll tick in a sandbox.agentSession.pollOutput span", async () => { + // With a span runner injected, the poll timer wraps each 100 ms poll tick in + // a `sandbox.agentSession.pollOutput` span. This test lets the first poll tick + // fire and proves the timer opens that wrapper span. + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-process-session-poll-span-")); + cleanupDirs.push(rootDir); + const childPath = path.join(rootDir, "noop-acp-child.mjs"); + await writeFile(childPath, "process.stdin.on('data', () => {});\n", "utf8"); + + const spanNames: string[] = []; + let resolvePoll: () => void = () => {}; + const pollObserved = new Promise((resolve) => { + resolvePoll = resolve; + }); + + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "local-test", + remoteCwd: rootDir, + timeoutMs: 30_000, + runner: createLocalSandboxRunner(), + }; + + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-process-session-poll-span", + target, + runtimeRootDir: path.posix.join(rootDir, ".paperclip-runtime", "acpx"), + adapterKey: "acpx", + command: process.execPath, + args: [childPath], + cwd: rootDir, + env: {}, + timeoutSec: 5, + onLog: async () => {}, + // Record each wrapper span name, then run the wrapped work. + runtimeSpan: async (name, work) => { + spanNames.push(name); + if (name === "sandbox.agentSession.pollOutput") resolvePoll(); + return work(); + }, + }); + expect(bridge).not.toBeNull(); + + try { + await pollObserved; + expect(spanNames).toContain("sandbox.agentSession.pollOutput"); + } finally { + await bridge?.stop(); + } + }); + it("bridges bidirectional sandbox process sessions through a local ACPX-spawnable proxy", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-process-session-")); cleanupDirs.push(rootDir); diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 0eb776e8cd..5e7aa5719f 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -48,6 +48,7 @@ import { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js"; import { preferredShellForSandbox, shellCommandArgs } from "./sandbox-shell.js"; import { runWithRuntimeParent, + type RuntimeSpanRunner, type StartupSpanContext, } from "./acpx-engine/startup-timing.js"; import type { RuntimeProgressSink, RuntimeStatusSink } from "./runtime-progress.js"; @@ -1354,6 +1355,14 @@ async function waitForLocalServerListen(server: net.Server): Promise { return address.port; } +/** Span name that wraps the socket handler's one `writeTextFile` exec — one + * outbound ACP message to the agent. */ +const AGENT_SESSION_SEND_INPUT_SPAN = "sandbox.agentSession.sendInput"; + +/** Span name that wraps one 100 ms poll tick — `list`, then `read`+`remove` per + * file found (`1 + 2n` execs). */ +const AGENT_SESSION_POLL_OUTPUT_SPAN = "sandbox.agentSession.pollOutput"; + export async function startAdapterExecutionTargetProcessSessionBridge(input: { runId: string; target: AdapterExecutionTarget | null | undefined; @@ -1376,6 +1385,12 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // turn, `task.run` otherwise). When it is absent, the work runs with an empty // store, exactly like the earlier `runWithoutActiveStep` behavior. getRuntimeParentContext?: () => StartupSpanContext | undefined; + // Wrap each unit of run-time work in its own named span. The socket handler + // uses it for `sandbox.agentSession.sendInput` and the poll timer for + // `sandbox.agentSession.pollOutput`, so each unit's inner `sandbox.exec` spans + // group under one wrapper span. When it is absent, the work runs under the run + // parent with no wrapper span, exactly like the earlier behavior. + runtimeSpan?: RuntimeSpanRunner; }): Promise { if (!input.target || input.target.kind !== "remote" || input.target.transport !== "sandbox") { return null; @@ -1384,6 +1399,14 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { const target = input.target; const onLog = input.onLog ?? (async () => {}); const runner = requireSandboxRunner(target); + // Run one unit of run-time work under its named wrapper span when a span + // runner is injected. Without a runner, run the work under the current run + // parent, so the inner `sandbox.exec` spans parent to the live run span, + // exactly like the earlier behavior. + const runRuntimeWork = (name: string, work: () => Promise): Promise => + input.runtimeSpan + ? input.runtimeSpan(name, work) + : runWithRuntimeParent(input.getRuntimeParentContext?.(), work); const shellCommand = preferredSandboxShell(target); const timeoutMs = typeof input.timeoutSec === "number" && Number.isFinite(input.timeoutSec) && input.timeoutSec > 0 @@ -1548,26 +1571,28 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { socket = nextSocket; flushPendingRemoteEvents(); } - // Read the current-run parent now, at send time. The live parent - // switches to `agent.turn` during the turn and back to `task.run` - // after it. With no getter the store stays empty, exactly like the - // earlier unparented behavior. - runWithRuntimeParent(input.getRuntimeParentContext?.(), () => { - void (async () => { - if (message.type === "stdin" && typeof message.data === "string") { - stdinSeq += 1; - const name = `${String(stdinSeq).padStart(12, "0")}.json`; - await client.writeTextFile(path.posix.join(stdinDir, name), jsonLine({ type: "stdin", data: message.data })); - } else if (message.type === "stdinEnd") { - stdinSeq += 1; - const name = `${String(stdinSeq).padStart(12, "0")}.json`; - await client.writeTextFile(path.posix.join(stdinDir, name), jsonLine({ type: "stdinEnd" })); - } - })().catch((error) => { + // Wrap one outbound ACP message to the agent in a + // `sandbox.agentSession.sendInput` span, so its one `writeTextFile` exec + // groups under one named span. The span runner reads the current-run + // parent at send time: the live parent switches to `agent.turn` during + // the turn and back to `task.run` after it. A message that is neither + // `stdin` nor `stdinEnd` writes nothing, so it opens no span. + const stdinPayload = + message.type === "stdin" && typeof message.data === "string" + ? { type: "stdin", data: message.data } + : message.type === "stdinEnd" + ? { type: "stdinEnd" } + : null; + if (stdinPayload) { + stdinSeq += 1; + const name = `${String(stdinSeq).padStart(12, "0")}.json`; + void runRuntimeWork(AGENT_SESSION_SEND_INPUT_SPAN, () => + client.writeTextFile(path.posix.join(stdinDir, name), jsonLine(stdinPayload)), + ).catch((error) => { nextSocket.write(jsonLine({ type: "error", message: error instanceof Error ? error.message : String(error) })); nextSocket.destroy(); }); - }); + } } }); }); @@ -1600,15 +1625,16 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { } }; - // Schedule the long-lived poll timer under the current-run parent context. - // The poll loop reads remote event files with run-time execs, not startup - // work, so a poll `sandbox.exec` span parents to the live run span, not to the - // ended bridge step. Read the getter per tick, because the re-arm timer that - // the poll body schedules reads it again: the live parent switches to - // `agent.turn` during the turn and back to `task.run` after it. With no getter - // the store stays empty, exactly like the earlier unparented behavior. + // Schedule the long-lived poll timer. Wrap each 100 ms poll tick in a + // `sandbox.agentSession.pollOutput` span, so the tick's `list` plus per-file + // `read`/`remove` execs group under one named span. The poll loop reads remote + // event files with run-time execs, not startup work, so the wrapper span and + // its child execs parent to the live run span, not to the ended bridge step. + // The span runner reads the run parent per tick, because the re-arm timer that + // the poll body schedules opens a new tick span: the live parent switches to + // `agent.turn` during the turn and back to `task.run` after it. const schedulePoll = () => { - pollTimer = setTimeout(() => runWithRuntimeParent(input.getRuntimeParentContext?.(), () => void poll()), 100); + pollTimer = setTimeout(() => void runRuntimeWork(AGENT_SESSION_POLL_OUTPUT_SPAN, poll), 100); pollTimer.unref?.(); }; @@ -1764,6 +1790,11 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { // `sandbox.exec` span parents to the live run span. When it is absent, the // request work runs with an empty store, exactly like the earlier behavior. getRuntimeParentContext?: () => StartupSpanContext | undefined; + // Wrap each callback request in a `sandbox.callbackBridge.relayRequest` span. + // The factory threads it into the worker, which uses it per request so each + // request's execs group under one wrapper span. When it is absent, the request + // work runs under the run parent with no wrapper span. + runtimeSpan?: RuntimeSpanRunner; }): Promise { if (!adapterExecutionTargetUsesPaperclipBridge(input.target)) { return null; @@ -1835,6 +1866,7 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { queueDir, maxBodyBytes, getRuntimeParentContext: input.getRuntimeParentContext, + runtimeSpan: input.runtimeSpan, handleRequest: async (request) => { const method = request.method.trim().toUpperCase() || "GET"; if (bridgeDebugEnabled) { diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts index 85a361f431..e69fef32c8 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts @@ -611,6 +611,54 @@ describe("sandbox callback bridge", () => { expect((requestStep as { criticalPath?: boolean }).criticalPath).toBe(false); }); + it("wraps each request in a sandbox.callbackBridge.relayRequest span", async () => { + // With a span runner injected, the worker wraps each request in one + // `sandbox.callbackBridge.relayRequest` span, so the request's read, write, + // and remove execs group under one named span. This test drives the worker + // with a recording runner and proves it opens the wrapper span around the + // request work. + const wrapped: string[] = []; + let served = false; + let resolveServed: () => void = () => {}; + const requestServed = new Promise((resolve) => { + resolveServed = resolve; + }); + + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-relay-span-")); + cleanupDirs.push(rootDir); + const queueDir = path.posix.join(rootDir, "queue"); + + const worker = await startSandboxCallbackBridgeWorker({ + client: { + makeDir: async () => {}, + makeDirs: async () => {}, + listJsonFiles: async () => (served ? [] : ["000000000001.json"]), + readTextFile: async () => + JSON.stringify({ id: "req-1", method: "GET", path: "/", query: "", headers: {}, body: "" }), + writeTextFile: async () => {}, + rename: async () => {}, + remove: async () => {}, + }, + queueDir, + authorizeRequest: async () => null, + handleRequest: async () => { + served = true; + resolveServed(); + return { status: 200, body: "ok" }; + }, + // Record each wrapper span name, then run the wrapped work. + runtimeSpan: async (name, work) => { + wrapped.push(name); + return work(); + }, + }); + + await requestServed; + await worker.stop(); + + expect(wrapped).toContain("sandbox.callbackBridge.relayRequest"); + }); + it("test_paperclip_loop_exec_stays_unparented_without_getter", async () => { // With no `getRuntimeParentContext`, a request runs with an empty active // step store, exactly like the earlier `runWithoutActiveStep` behavior. So a diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.ts b/packages/adapter-utils/src/sandbox-callback-bridge.ts index 48f64211d7..bd5a83ab5f 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.ts @@ -6,6 +6,7 @@ import path from "node:path"; import { runWithoutActiveStep, runWithRuntimeParent, + type RuntimeSpanRunner, type StartupSpanContext, } from "./acpx-engine/startup-timing.js"; import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js"; @@ -23,6 +24,10 @@ const SANDBOX_CALLBACK_BRIDGE_ENTRYPOINT = "paperclip-bridge-server.mjs"; const SANDBOX_EXEC_CHANNEL_ENV = "PAPERCLIP_SANDBOX_EXEC_CHANNEL"; const SANDBOX_EXEC_CHANNEL_BRIDGE = "bridge"; +/** Span name that wraps one Paperclip-API callback request — read the request, + * write the response, and remove the request file. */ +const CALLBACK_BRIDGE_RELAY_REQUEST_SPAN = "sandbox.callbackBridge.relayRequest"; + export const DEFAULT_SANDBOX_CALLBACK_BRIDGE_MAX_BODY_BYTES = DEFAULT_BRIDGE_MAX_BODY_BYTES; export interface SandboxCallbackBridgeRouteRule { @@ -630,6 +635,12 @@ export async function startSandboxCallbackBridgeWorker(input: { // otherwise). When it is absent, the request work runs with an empty store, // exactly like the earlier `runWithoutActiveStep` behavior. getRuntimeParentContext?: () => StartupSpanContext | undefined; + // Wrap each Paperclip-API callback request in a + // `sandbox.callbackBridge.relayRequest` span, so the request's read, write, and + // remove execs group under one named span. When it is absent, the request work + // runs under the run parent with no wrapper span, exactly like the earlier + // behavior. + runtimeSpan?: RuntimeSpanRunner; }): Promise { const pollIntervalMs = normalizeTimeoutMs(input.pollIntervalMs, DEFAULT_BRIDGE_POLL_INTERVAL_MS); const maxBodyBytes = normalizeTimeoutMs(input.maxBodyBytes, DEFAULT_BRIDGE_MAX_BODY_BYTES); @@ -777,15 +788,20 @@ export async function startSandboxCallbackBridgeWorker(input: { if (stopping && Date.now() >= stopDeadline) break; inFlight += 1; try { - // A request is run-time work, not startup work. Read the run parent - // context now and run the request under it, so the request - // `sandbox.exec` span parents to the live run span. Read the getter - // per request: the live parent switches to `agent.turn` during the - // turn and back to `task.run` after it. With no getter the store - // stays empty, exactly like the earlier unparented behavior. - await runWithRuntimeParent(input.getRuntimeParentContext?.(), () => - processRequestFile(fileName), - ); + // A request is run-time work, not startup work. Wrap it in a + // `sandbox.callbackBridge.relayRequest` span, so its read, write, and + // remove execs group under one named span that parents to the live + // run span. The span runner reads the run parent per request: the + // live parent switches to `agent.turn` during the turn and back to + // `task.run` after it. Without a runner, the request runs under the + // run parent with no wrapper span, exactly like the earlier behavior. + await (input.runtimeSpan + ? input.runtimeSpan(CALLBACK_BRIDGE_RELAY_REQUEST_SPAN, () => + processRequestFile(fileName), + ) + : runWithRuntimeParent(input.getRuntimeParentContext?.(), () => + processRequestFile(fileName), + )); } finally { inFlight -= 1; } diff --git a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts index be9e51547d..2e3a192caa 100644 --- a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts +++ b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts @@ -460,8 +460,9 @@ async function syncInFileMappings(input: { // Ensure every target directory exists before the bulk upload writes its temp. const mkdirCommand = [...parentDirs].map((dir) => `mkdir -p ${shellQuote(dir)}`).join(" && "); + // `ensureDirectory` span: `mkdir -p` — ensure a directory exists before a write. await withProviderSpan({ - name: "mkdir", + name: "ensureDirectory", run: () => assertSandboxCommandOk(sandbox, mkdirCommand, timeoutSeconds, "syncIn mkdir"), }); guardRoundTrips += 1; @@ -470,8 +471,10 @@ async function syncInFileMappings(input: { // can replace a target parent with a symlink to `/etc` so the string check // passes but the upload + `mv -f` resolve through it. Canonicalize every parent // dir (now materialized) and fail closed if any escapes, BEFORE any bytes land. + // `checkSymlinkEscape` span: re-check a path resolves inside the workspace root + // before use. await withProviderSpan({ - name: "guard", + name: "checkSymlinkEscape", run: () => assertSandboxPathsConfined({ sandbox, @@ -488,6 +491,7 @@ async function syncInFileMappings(input: { // retry never accumulates stale `.paperclip-upload-*` scratch. try { // One batched bulk upload (single /files/bulk-upload) for all file mappings. + // `transfer` span: the real byte upload — `sandbox.fs.uploadFiles`. await withProviderSpan({ name: "transfer", wallMsAttr: SPAN_ATTR.transferWallMs, @@ -534,8 +538,10 @@ async function syncInFileMappings(input: { `exec 8>&-;`, ); } + // `promote` span: atomically move the staged temp onto its target via a + // pinned dir handle. await withProviderSpan({ - name: "rename", + name: "promote", run: () => assertSandboxCommandOk( sandbox, @@ -564,6 +570,7 @@ async function syncInDirectoryMapping(input: { const archivePath = path.join(tmp, "sync-in.tar"); // The pack step is host-local: it builds the tarball and makes no sandbox // round trip. The `pack` span records its wall time. + // `pack` span: build a tarball on the host — no sandbox round trip. await withProviderSpan({ name: "pack", wallMsAttr: SPAN_ATTR.packWallMs, @@ -585,8 +592,9 @@ async function syncInDirectoryMapping(input: { // components, then confirm it (and any existing parent) canonicalizes inside // the remote dir — `tar -C` would otherwise follow a sandbox-planted symlink // and extract our archive outside the workspace root. + // `ensureDirectory` span: `mkdir -p` — ensure a directory exists before a write. await withProviderSpan({ - name: "mkdir", + name: "ensureDirectory", run: () => assertSandboxCommandOk( sandbox, @@ -596,8 +604,10 @@ async function syncInDirectoryMapping(input: { ), }); guardRoundTrips += 1; + // `checkSymlinkEscape` span: re-check a path resolves inside the workspace + // root before use. await withProviderSpan({ - name: "guard", + name: "checkSymlinkEscape", run: () => assertSandboxPathsConfined({ sandbox, @@ -608,6 +618,7 @@ async function syncInDirectoryMapping(input: { }), }); guardRoundTrips += 1; + // `transfer` span: the real byte upload — `sandbox.fs.uploadFiles`. await withProviderSpan({ name: "transfer", wallMsAttr: SPAN_ATTR.transferWallMs, @@ -641,8 +652,10 @@ async function syncInDirectoryMapping(input: { `exec 9>&-;`, `rm -f ${shellQuote(remoteTar)};`, ].join("\n"); + // `extractTarball` span: one round trip — re-check the path, `tar -xf`, and + // remove the scratch tarball. await withProviderSpan({ - name: "extract", + name: "extractTarball", run: () => assertSandboxCommandOk( sandbox, @@ -687,8 +700,10 @@ async function runPostUploadCommands(input: { let cwd = remoteDir; if (command.cwd != null) { assertConfinedSandboxPath(remoteDir, command.cwd, "post-upload command cwd"); + // `checkSymlinkEscape` span: re-check a path resolves inside the workspace + // root before use. await withProviderSpan({ - name: "guard", + name: "checkSymlinkEscape", run: () => assertSandboxPathsConfined({ sandbox, @@ -704,8 +719,9 @@ async function runPostUploadCommands(input: { // C4: first non-zero exit or timeout throws and aborts the remaining commands. const commandTimeoutSeconds = command.timeoutMs != null ? toTimeoutSeconds(command.timeoutMs) : timeoutSeconds; + // `postUploadCommand` span: run one caller-supplied post-upload command. const result = await withProviderSpan({ - name: "provision", + name: "postUploadCommand", run: () => sandbox.process.executeCommand(command.command, cwd, undefined, commandTimeoutSeconds), }); diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts index 7e3d04d9c9..e37d2b348f 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -1202,7 +1202,7 @@ describe("Daytona sandbox provider plugin", () => { // The provision command runs before the run opens its trace root, so the // host marks it `bypassSession`. The provider must not open the session for - // it, or the `session.setup` span loses its run parent. + // it, or the `session.open` span loses its run parent. await plugin.definition.onEnvironmentExecute?.( sessionExecParams({ bypassSession: true }), ); @@ -1406,7 +1406,7 @@ describe("Daytona sandbox provider plugin", () => { expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); }); - it("emits a session.setup span on create and a session.teardown span on delete", async () => { + it("emits a session.open span on create and a session.close span on delete", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox(); mockGet.mockResolvedValue(sandbox); @@ -1414,7 +1414,7 @@ describe("Daytona sandbox provider plugin", () => { const restore = __setDaytonaPluginContextForTest({ tracer } as unknown as PluginContext); try { await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); - const setup = spans.find((span) => span.name === "session.setup"); + const setup = spans.find((span) => span.name === "session.open"); expect(setup).toBeDefined(); expect(setup!.ended).toBe(true); expect(setup!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); @@ -1426,7 +1426,7 @@ describe("Daytona sandbox provider plugin", () => { providerLeaseId: "sandbox-123", config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, }); - const teardown = spans.find((span) => span.name === "session.teardown"); + const teardown = spans.find((span) => span.name === "session.close"); expect(teardown).toBeDefined(); expect(teardown!.ended).toBe(true); expect(teardown!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); @@ -1435,7 +1435,7 @@ describe("Daytona sandbox provider plugin", () => { } }); - it("marks the session.setup span failed when the session create throws", async () => { + it("marks the session.open span failed when the session create throws", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox(); sandbox.process.createSession.mockRejectedValueOnce(new Error("create boom")); @@ -1446,7 +1446,7 @@ describe("Daytona sandbox provider plugin", () => { await expect( plugin.definition.onEnvironmentExecute?.(sessionExecParams()), ).rejects.toThrow(/create boom/); - const setup = spans.find((span) => span.name === "session.setup"); + const setup = spans.find((span) => span.name === "session.open"); expect(setup).toBeDefined(); expect(setup!.ended).toBe(true); expect(setup!.status?.code).toBe(2); @@ -3028,7 +3028,7 @@ describe("daytona native file-sync hooks", () => { expect(transfer!.attributes["paperclip.sandbox.startup.transfer.guard.count"]).toBe(2); }); - it("opens mkdir, guard, transfer, rename spans in call order for a file-mapping sync", async () => { + it("opens ensureDirectory, checkSymlinkEscape, transfer, promote spans in call order for a file-mapping sync", async () => { const hostDir = await makeHostDir(); const source = path.join(hostDir, "config.txt"); await fs.writeFile(source, "plain"); @@ -3055,21 +3055,26 @@ describe("daytona native file-sync hooks", () => { restore(); } - expect(spans.map((span) => span.name)).toEqual(["mkdir", "guard", "transfer", "rename"]); + expect(spans.map((span) => span.name)).toEqual([ + "ensureDirectory", + "checkSymlinkEscape", + "transfer", + "promote", + ]); for (const span of spans) { expect(span.ended).toBe(true); expect(span.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); // A per-round-trip span carries no `*.wall_ms` attribute; the native span // width carries its time. Only `pack` and `transfer` keep a wall_ms value. if (span.name !== "transfer") { - expect(span.attributes["paperclip.sandbox.startup.mkdir.wall_ms"]).toBeUndefined(); - expect(span.attributes["paperclip.sandbox.startup.rename.wall_ms"]).toBeUndefined(); - expect(span.attributes["paperclip.sandbox.startup.guard.wall_ms"]).toBeUndefined(); + expect(span.attributes["paperclip.sandbox.startup.ensureDirectory.wall_ms"]).toBeUndefined(); + expect(span.attributes["paperclip.sandbox.startup.promote.wall_ms"]).toBeUndefined(); + expect(span.attributes["paperclip.sandbox.startup.checkSymlinkEscape.wall_ms"]).toBeUndefined(); } } }); - it("opens pack, mkdir, guard, transfer, extract spans in call order for a directory-mapping sync", async () => { + it("opens pack, ensureDirectory, checkSymlinkEscape, transfer, extractTarball spans in call order for a directory-mapping sync", async () => { const hostDir = await makeHostDir(); const sourceDir = path.join(hostDir, "assets"); await fs.mkdir(sourceDir, { recursive: true }); @@ -3099,7 +3104,13 @@ describe("daytona native file-sync hooks", () => { restore(); } - expect(spans.map((span) => span.name)).toEqual(["pack", "mkdir", "guard", "transfer", "extract"]); + expect(spans.map((span) => span.name)).toEqual([ + "pack", + "ensureDirectory", + "checkSymlinkEscape", + "transfer", + "extractTarball", + ]); for (const span of spans) { expect(span.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); } @@ -3141,7 +3152,7 @@ describe("daytona native file-sync hooks", () => { expect(pack!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); }); - it("opens a guard span and a provision span in call order for a post-upload command with a working directory", async () => { + it("opens a checkSymlinkEscape span and a postUploadCommand span in call order for a post-upload command with a working directory", async () => { const hostDir = await makeHostDir(); const source = path.join(hostDir, "config.txt"); await fs.writeFile(source, "plain"); @@ -3169,17 +3180,18 @@ describe("daytona native file-sync hooks", () => { restore(); } - // The full order: the file mapping opens mkdir, guard, transfer, rename; the - // post-upload command then opens its own cwd guard and the provision span. + // The full order: the file mapping opens ensureDirectory, checkSymlinkEscape, + // transfer, promote; the post-upload command then opens its own cwd + // checkSymlinkEscape and the postUploadCommand span. expect(spans.map((span) => span.name)).toEqual([ - "mkdir", - "guard", + "ensureDirectory", + "checkSymlinkEscape", "transfer", - "rename", - "guard", - "provision", + "promote", + "checkSymlinkEscape", + "postUploadCommand", ]); - const provision = spans.find((span) => span.name === "provision"); + const provision = spans.find((span) => span.name === "postUploadCommand"); expect(provision!.ended).toBe(true); expect(provision!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); }); diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.ts index e00c0a2ca0..18306045b3 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -1336,11 +1336,13 @@ async function getOrCreateSession(sandbox: Sandbox, scope: SandboxScope): Promis // no second command can slip in between the store read and the create start. return sandboxHandleSessionStore.runSingle(scope, async () => { const sessionId = `paperclip-${randomUUID()}`; - // Wrap the session create in a short `session.setup` provider span. The span + // Wrap the session create in a short `session.open` provider span. The span // carries no session id and no command text, only the provider family. The - // host maps the name to `sandbox.provider.session.setup`. + // host maps the name to `sandbox.daytona.session.open`. + // `session.open` span: create the one persistent Daytona session for a lease, + // on the first in-run command — `sandbox.process.createSession`. await withProviderSpan({ - name: "session.setup", + name: "session.open", run: () => sandbox.process.createSession(sessionId), }); sandboxHandleSessionStore.set(scope, sessionId); @@ -1360,10 +1362,12 @@ async function teardownSession(sandbox: Sandbox, scope: SandboxScope): Promise sandbox.process.deleteSession(sessionId), }); } catch (error) { @@ -2255,9 +2259,9 @@ const plugin = definePlugin({ // on, and it does NOT open the session. The host sets this flag on a // pre-run command (the workspace provision command) that runs before the // run opens its trace root. Opening the session there would emit a - // `session.setup` span with no run parent, and the span backend would drop + // `session.open` span with no run parent, and the span backend would drop // it. With the bypass the session opens on the first in-run command, whose - // setup span parents to the run trace. + // open span parents to the run trace. let result: PluginEnvironmentExecuteResult; if (config.useSessions && !params.bypassSession) { const sessionId = await getOrCreateSession(sandbox, scope); diff --git a/packages/shared/src/telemetry/README.md b/packages/shared/src/telemetry/README.md index b86c7ea09d..79ae3e2581 100644 --- a/packages/shared/src/telemetry/README.md +++ b/packages/shared/src/telemetry/README.md @@ -93,10 +93,17 @@ absent, never a misleading `0`. | `bridge.paperclip` | Paperclip bridge start step. | `sandbox.startup` | | `bridge.process-session` | Process-session bridge start step. | `sandbox.startup` | | `acp.handshake` | ACP session handshake step. | `sandbox.startup` | -| `sandbox.exec` | One host-to-sandbox execution. | the active step span | +| `sandbox.agentSession.sendInput` | One outbound ACP message to the agent — the socket handler's one `writeTextFile` exec. | the active run span | +| `sandbox.agentSession.pollOutput` | One 100 ms poll tick — `list`, then `read`+`remove` per file found (`1 + 2n` execs). | the active run span | +| `sandbox.callbackBridge.relayRequest` | One Paperclip-API callback request — read the request, write the response, remove it. | the active run span | +| `sandbox.exec` | One host-to-sandbox execution. | the active step or wrapper span | A step span name is the step name. The `sandbox.exec` span parents to the step -span that runs the execution, so each execution nests under its step. With no +span that runs the execution, so each execution nests under its step. A run-time +`sandbox.exec` span parents instead to the run-time wrapper span that runs it +(`sandbox.agentSession.sendInput`, `sandbox.agentSession.pollOutput`, or +`sandbox.callbackBridge.relayRequest`). Each run-time wrapper span parents to the +live run span (`agent.turn` during the turn, `task.run` otherwise). With no active trace context the exec span opens unparented. The root span sets the error status when the bring-up fails. Each step span sets @@ -172,7 +179,7 @@ the code first. Keep the attribute low-cardinality and free of user content. ### Provider spans A sandbox provider plugin also opens spans for its own sync steps. These spans -use the `sandbox.provider.` name prefix. They share the +use the `sandbox.daytona.` name prefix. They share the `paperclip.sandbox.startup.` attribute prefix and obey the same opt-in and no-user-content rules as the startup spans above. @@ -183,15 +190,26 @@ before it records the span. | Span | Scope | Parent | | --- | --- | --- | -| `sandbox.provider.pack` | The host-local pack step that builds the upload tarball. It makes no sandbox round trip. | the active startup step span | -| `sandbox.provider.transfer` | The transfer step that uploads the files to the sandbox. | the active startup step span | -| `sandbox.provider.other` | Any span name outside the known set. | the active startup step span | +| `sandbox.daytona.pack` | The host-local pack step that builds the upload tarball. It makes no sandbox round trip. | the active startup step span | +| `sandbox.daytona.transfer` | The transfer step that uploads the files to the sandbox. | the active startup step span | +| `sandbox.daytona.ensureDirectory` | The `mkdir -p` step that ensures a directory exists before a write. | the active startup step span | +| `sandbox.daytona.checkSymlinkEscape` | The re-check step that a path resolves inside the workspace root before use. | the active startup step span | +| `sandbox.daytona.promote` | The atomic move of a staged temp onto its target via a pinned dir handle. | the active startup step span | +| `sandbox.daytona.extractTarball` | The one round trip that re-checks the path, runs `tar -xf`, and removes the scratch tarball. | the active startup step span | +| `sandbox.daytona.postUploadCommand` | One caller-supplied post-upload command. | the active startup step span | +| `sandbox.daytona.session.open` | The create of the one persistent session for a lease, on the first in-run command. | the active run span | +| `sandbox.daytona.session.close` | The delete of that persistent session on lease release. | the active run span | +| `sandbox.daytona.other` | Any span name outside the known set. | the active startup step span | -The host clamps the span name to the closed set `pack` and `transfer`. The host -maps a known name to `sandbox.provider.`. The host maps any other value to -`sandbox.provider.other`, so a span name never carries free-form data. +The host clamps the span name to the closed set of leaf names above (`pack`, +`transfer`, `ensureDirectory`, `checkSymlinkEscape`, `promote`, `extractTarball`, +`postUploadCommand`, `session.open`, and `session.close`). The host maps a known +name to `sandbox.daytona.`. The host maps any other value to +`sandbox.daytona.other`, so a span name never carries free-form data. Only the +daytona provider emits these spans today, so the segment is the literal +`daytona`. -The `sandbox.provider.*` spans use this closed attribute allowlist. The host +The `sandbox.daytona.*` spans use this closed attribute allowlist. The host drops every other key, so a command, an argument, a path, an id, a standard output, or a standard error never rides a provider span. The host records only the attributes that the producer sends for one span. @@ -200,9 +218,9 @@ the attributes that the producer sends for one span. | --- | --- | --- | --- | | `paperclip.sandbox.startup.provider` | string | no | The normalized provider family. | | `paperclip.sandbox.startup.outcome` | string | yes | The step outcome (`ok`, `skipped`, or `failed`). | -| `paperclip.sandbox.startup.pack.wall_ms` | number | yes | The host-local wall time of the pack step. It rides the `sandbox.provider.pack` span. | -| `paperclip.sandbox.startup.transfer.wall_ms` | number | yes | The wall time of the transfer step. It rides the `sandbox.provider.transfer` span. | -| `paperclip.sandbox.startup.transfer.guard.count` | number | yes | The number of serial guard round trips before one transfer. It rides the `sandbox.provider.transfer` span. | +| `paperclip.sandbox.startup.pack.wall_ms` | number | yes | The host-local wall time of the pack step. It rides the `sandbox.daytona.pack` span. | +| `paperclip.sandbox.startup.transfer.wall_ms` | number | yes | The wall time of the transfer step. It rides the `sandbox.daytona.transfer` span. | +| `paperclip.sandbox.startup.transfer.guard.count` | number | yes | The number of serial guard round trips before one transfer. It rides the `sandbox.daytona.transfer` span. | The `span.record` host handler enforces the allowlist. It re-maps `provider` through the provider-family normalizer. It keeps `outcome` only when the value diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 5c13b76f52..7397b99732 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -709,7 +709,7 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { // Build the fake plugin fixture for the run-parent release tests below. A // plugin sandbox provider can open a persistent session on the first command // and delete it on lease release; the delete emits a provider - // `session.teardown` span. The host mints that span's parent from the active + // `session.close` span. The host mints that span's parent from the active // step context at the release RPC. So the release must run under the run // parent, or the span loses its traceparent and the backend drops it. async function seedFakePluginSandbox() { diff --git a/server/src/__tests__/instrumentation.test.ts b/server/src/__tests__/instrumentation.test.ts index b0b94e07a3..e2a3937ceb 100644 --- a/server/src/__tests__/instrumentation.test.ts +++ b/server/src/__tests__/instrumentation.test.ts @@ -178,7 +178,7 @@ describe.skipIf(!otelSdk)("recordProviderPluginSpan native duration", () => { const startTimeMs = Date.now() - 4500; const endTimeMs = startTimeMs + 4500; recordProviderPluginSpan({ - name: "sandbox.provider.mkdir", + name: "sandbox.daytona.ensureDirectory", parent: { traceId: "0af7651916cd43dd8448eb211c80319c", spanId: "b7ad6b7169203331", @@ -191,7 +191,7 @@ describe.skipIf(!otelSdk)("recordProviderPluginSpan native duration", () => { const finished = exporter.getFinishedSpans(); expect(finished).toHaveLength(1); const span = finished[0]!; - expect(span.name).toBe("sandbox.provider.mkdir"); + expect(span.name).toBe("sandbox.daytona.ensureDirectory"); expect(Math.round(hrTimeToMs(span.startTime as [number, number]))).toBe(startTimeMs); expect(Math.round(hrTimeToMs(span.endTime as [number, number]))).toBe(endTimeMs); // The native width equals the true wall-clock difference, not near zero. @@ -211,7 +211,7 @@ describe.skipIf(!otelSdk)("recordProviderPluginSpan native duration", () => { try { const { recordProviderPluginSpan } = await import("../instrumentation.js"); recordProviderPluginSpan({ - name: "sandbox.provider.pack", + name: "sandbox.daytona.pack", parent: { traceId: "0af7651916cd43dd8448eb211c80319c", spanId: "b7ad6b7169203331", diff --git a/server/src/__tests__/plugin-host-services-span.test.ts b/server/src/__tests__/plugin-host-services-span.test.ts index 110d06a0b6..bb2a72bb56 100644 --- a/server/src/__tests__/plugin-host-services-span.test.ts +++ b/server/src/__tests__/plugin-host-services-span.test.ts @@ -63,7 +63,7 @@ describe("plugin provider span host handler", () => { parent: { traceId: string; spanId: string; traceFlags: number }; attributes: Record; }; - expect(call.name).toBe("sandbox.provider.pack"); + expect(call.name).toBe("sandbox.daytona.pack"); expect(call.parent).toEqual({ traceId: "0af7651916cd43dd8448eb211c80319c", spanId: "b7ad6b7169203331", @@ -114,20 +114,26 @@ describe("plugin provider span host handler", () => { expect(call.status).not.toHaveProperty("message"); }); - it("clamps an unknown span name to sandbox.provider.other", async () => { + it("clamps an unknown span name to sandbox.daytona.other", async () => { const services = servicesFor(); await services.tracer.record( { name: "rm -rf / --no-preserve-root" }, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, ); expect((mockRecordSpan.mock.calls[0]![0] as { name: string }).name).toBe( - "sandbox.provider.other", + "sandbox.daytona.other", ); }); - it("admits each per-round-trip span name to sandbox.provider.", async () => { + it("admits each per-round-trip span name to sandbox.daytona.", async () => { const services = servicesFor(); - for (const name of ["mkdir", "guard", "rename", "extract", "provision"]) { + for (const name of [ + "ensureDirectory", + "checkSymlinkEscape", + "promote", + "extractTarball", + "postUploadCommand", + ]) { await services.tracer.record( { name }, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, @@ -135,17 +141,17 @@ describe("plugin provider span host handler", () => { } const recorded = mockRecordSpan.mock.calls.map((c) => (c[0] as { name: string }).name); expect(recorded).toEqual([ - "sandbox.provider.mkdir", - "sandbox.provider.guard", - "sandbox.provider.rename", - "sandbox.provider.extract", - "sandbox.provider.provision", + "sandbox.daytona.ensureDirectory", + "sandbox.daytona.checkSymlinkEscape", + "sandbox.daytona.promote", + "sandbox.daytona.extractTarball", + "sandbox.daytona.postUploadCommand", ]); }); - it("admits the session setup and teardown span names", async () => { + it("admits the session open and close span names", async () => { const services = servicesFor(); - for (const name of ["session.setup", "session.teardown"]) { + for (const name of ["session.open", "session.close"]) { await services.tracer.record( { name }, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, @@ -153,8 +159,8 @@ describe("plugin provider span host handler", () => { } const recorded = mockRecordSpan.mock.calls.map((c) => (c[0] as { name: string }).name); expect(recorded).toEqual([ - "sandbox.provider.session.setup", - "sandbox.provider.session.teardown", + "sandbox.daytona.session.open", + "sandbox.daytona.session.close", ]); }); @@ -163,7 +169,7 @@ describe("plugin provider span host handler", () => { const startTimeMs = Date.now() - 4500; const endTimeMs = startTimeMs + 4500; await services.tracer.record( - { name: "mkdir", startTimeMs, endTimeMs }, + { name: "ensureDirectory", startTimeMs, endTimeMs }, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, ); const call = mockRecordSpan.mock.calls[0]![0] as { @@ -181,7 +187,7 @@ describe("plugin provider span host handler", () => { const startTimeMs = Date.now() + 5000; const endTimeMs = startTimeMs + 1000; await services.tracer.record( - { name: "mkdir", startTimeMs, endTimeMs }, + { name: "ensureDirectory", startTimeMs, endTimeMs }, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, ); const call = mockRecordSpan.mock.calls[0]![0] as { @@ -207,7 +213,7 @@ describe("plugin provider span host handler", () => { for (const pair of invalidPairs) { mockRecordSpan.mockReset(); await services.tracer.record( - { name: "mkdir", startTimeMs: pair.startTimeMs, endTimeMs: pair.endTimeMs } as never, + { name: "ensureDirectory", startTimeMs: pair.startTimeMs, endTimeMs: pair.endTimeMs } as never, { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, ); // The span still records (the synchronous path), but without a timestamp. diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index 1eb3b8a01a..1428f42fd4 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -507,29 +507,33 @@ const SESSION_EVENT_SUBSCRIPTION_TIMEOUT_MS = 30 * 60 * 1_000; // 30 minutes const SPAN_ATTRS = SANDBOX_STARTUP_SPAN_ATTRS; -/** The closed set of provider span names a plugin may emit. `pack` and - * `transfer` are the host-local build and the byte upload. `mkdir`, `guard`, - * `rename`, `extract`, and `provision` are the per-round-trip command spans in - * the inbound sync path. `session.setup` and `session.teardown` are the short - * spans that wrap a persistent-session create and delete. */ +/** The closed set of provider span leaf names a plugin may emit. `pack` and + * `transfer` are the host-local build and the byte upload. `ensureDirectory`, + * `checkSymlinkEscape`, `promote`, `extractTarball`, and `postUploadCommand` + * are the per-round-trip command spans in the inbound sync path. `session.open` + * and `session.close` are the short spans that wrap a persistent-session create + * and delete. */ const KNOWN_PROVIDER_SPAN_NAMES: ReadonlySet = new Set([ "pack", "transfer", - "mkdir", - "guard", - "rename", - "extract", - "provision", - "session.setup", - "session.teardown", + "ensureDirectory", + "checkSymlinkEscape", + "promote", + "extractTarball", + "postUploadCommand", + "session.open", + "session.close", ]); /** Clamp the span name to a closed, namespaced set. A known name maps to - * `sandbox.provider.`; any other value maps to `sandbox.provider.other`, - * so a span name never carries free-form data. */ + * `sandbox.daytona.`; any other value maps to `sandbox.daytona.other`, so + * a span name never carries free-form data. Only the daytona provider emits + * these spans today, so the segment is the literal `daytona`. When a second + * provider emits provider spans, derive the segment from the normalized + * `provider` family attribute on the span instead of this literal. */ function clampProviderSpanName(raw: unknown): string { const name = typeof raw === "string" && KNOWN_PROVIDER_SPAN_NAMES.has(raw) ? raw : "other"; - return `sandbox.provider.${name}`; + return `sandbox.daytona.${name}`; } /** The closed allowlist of attribute keys a provider span may carry. The host