mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## 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 <noreply@paperclip.ing>
231 lines
9.2 KiB
TypeScript
231 lines
9.2 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { createRequire } from "node:module";
|
|
|
|
/**
|
|
* Tests for the opt-in OpenTelemetry bootstrap. The @opentelemetry/* packages
|
|
* are optional peer dependencies and are NOT installed in CI, which is itself
|
|
* part of the contract under test: with the endpoint set but packages absent,
|
|
* the module must warn and settle instead of crashing the server.
|
|
*
|
|
* The module reads OTEL_* env vars at import time, so each test resets the
|
|
* module registry and imports a fresh copy.
|
|
*/
|
|
|
|
const ENDPOINT_ENV = "OTEL_EXPORTER_OTLP_ENDPOINT";
|
|
const PROTOCOL_ENV = "OTEL_EXPORTER_OTLP_PROTOCOL";
|
|
|
|
const originalEndpoint = process.env[ENDPOINT_ENV];
|
|
const originalProtocol = process.env[PROTOCOL_ENV];
|
|
|
|
async function importFreshInstrumentation() {
|
|
vi.resetModules();
|
|
return await import("../instrumentation.js");
|
|
}
|
|
|
|
beforeEach(() => {
|
|
delete process.env[ENDPOINT_ENV];
|
|
delete process.env[PROTOCOL_ENV];
|
|
});
|
|
|
|
afterEach(() => {
|
|
if (originalEndpoint === undefined) delete process.env[ENDPOINT_ENV];
|
|
else process.env[ENDPOINT_ENV] = originalEndpoint;
|
|
if (originalProtocol === undefined) delete process.env[PROTOCOL_ENV];
|
|
else process.env[PROTOCOL_ENV] = originalProtocol;
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
describe("resolveProtocol", () => {
|
|
it.each([
|
|
[undefined, "grpc", "@opentelemetry/exporter-trace-otlp-grpc"],
|
|
["", "grpc", "@opentelemetry/exporter-trace-otlp-grpc"],
|
|
["grpc", "grpc", "@opentelemetry/exporter-trace-otlp-grpc"],
|
|
["http/protobuf", "http/protobuf", "@opentelemetry/exporter-trace-otlp-proto"],
|
|
["http/json", "http/json", "@opentelemetry/exporter-trace-otlp-http"],
|
|
["HTTP/JSON", "http/json", "@opentelemetry/exporter-trace-otlp-http"],
|
|
])("maps OTEL_EXPORTER_OTLP_PROTOCOL=%s to %s", async (raw, protocol, packageName) => {
|
|
if (raw === undefined) delete process.env[PROTOCOL_ENV];
|
|
else process.env[PROTOCOL_ENV] = raw;
|
|
|
|
const { resolveProtocol } = await importFreshInstrumentation();
|
|
|
|
expect(resolveProtocol()).toEqual({ protocol, packageName });
|
|
});
|
|
|
|
it("warns and falls back to grpc on an unrecognized protocol", async () => {
|
|
process.env[PROTOCOL_ENV] = "carrier-pigeon";
|
|
// Spy before the import so the assertion holds even if a future change
|
|
// makes the warning fire at module load time instead of on the call.
|
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
|
|
|
const { resolveProtocol } = await importFreshInstrumentation();
|
|
|
|
expect(resolveProtocol().protocol).toBe("grpc");
|
|
expect(warn).toHaveBeenCalledWith(expect.stringContaining("carrier-pigeon"));
|
|
});
|
|
});
|
|
|
|
describe("instrumentationReady", () => {
|
|
it("resolves immediately when OTEL_EXPORTER_OTLP_ENDPOINT is unset", async () => {
|
|
const { instrumentationReady } = await importFreshInstrumentation();
|
|
|
|
await expect(instrumentationReady).resolves.toBeUndefined();
|
|
});
|
|
|
|
it("settles with a diagnostic instead of throwing when the endpoint is set but packages are missing", async () => {
|
|
process.env[ENDPOINT_ENV] = "http://collector:4318";
|
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
|
|
|
const { instrumentationReady } = await importFreshInstrumentation();
|
|
|
|
// Bootstrap must absorb the failed dynamic imports — the server keeps
|
|
// booting without tracing rather than crashing on an opt-in feature.
|
|
await expect(instrumentationReady).resolves.toBeUndefined();
|
|
expect(warn).toHaveBeenCalledWith(
|
|
expect.stringContaining("@opentelemetry/* packages are not installed"),
|
|
expect.anything(),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("getStartupTracer", () => {
|
|
it("returns a usable tracer-shaped object when OTEL_EXPORTER_OTLP_ENDPOINT is unset", async () => {
|
|
const { getStartupTracer } = await importFreshInstrumentation();
|
|
|
|
const tracer = getStartupTracer();
|
|
|
|
// The accessor never returns null. The result exposes the span surface the
|
|
// startup seam calls, so the caller needs no null check.
|
|
expect(tracer).not.toBeNull();
|
|
expect(typeof tracer.startSpan).toBe("function");
|
|
// A no-op tracer must open and end a span without throwing.
|
|
expect(() => tracer.startSpan("workspace.resolve").end()).not.toThrow();
|
|
});
|
|
|
|
it("loads no OTel SDK package when the endpoint is unset", async () => {
|
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
|
|
|
const { getStartupTracer, instrumentationReady } = await importFreshInstrumentation();
|
|
|
|
// The endpoint is unset, so the bootstrap never runs and no SDK package
|
|
// import is attempted; readiness resolves at once.
|
|
await expect(instrumentationReady).resolves.toBeUndefined();
|
|
|
|
// The accessor still returns a usable tracer even though no @opentelemetry
|
|
// SDK package is installed. That proves it never imported the SDK: a hard
|
|
// dependency on the SDK would throw here instead.
|
|
const tracer = getStartupTracer();
|
|
expect(typeof tracer.startSpan).toBe("function");
|
|
expect(() => tracer.startSpan("stage.sync").end()).not.toThrow();
|
|
|
|
// The bootstrap "packages are not installed" diagnostic must not fire on
|
|
// this path. That message comes only from the endpoint-set bootstrap.
|
|
for (const call of warn.mock.calls) {
|
|
expect(String(call[0])).not.toContain("@opentelemetry/* packages are not installed");
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("shutdownInstrumentation", () => {
|
|
it("is a no-op when tracing is off and idempotent across calls", async () => {
|
|
const { shutdownInstrumentation } = await importFreshInstrumentation();
|
|
|
|
const first = shutdownInstrumentation();
|
|
const second = shutdownInstrumentation();
|
|
|
|
// Memoized: concurrent callers share one shutdown promise.
|
|
expect(first).toBe(second);
|
|
await expect(first).resolves.toBeUndefined();
|
|
});
|
|
|
|
it("resolves after a failed bootstrap instead of hanging", async () => {
|
|
process.env[ENDPOINT_ENV] = "http://collector:4318";
|
|
vi.spyOn(console, "warn").mockImplementation(() => {});
|
|
|
|
const { shutdownInstrumentation } = await importFreshInstrumentation();
|
|
|
|
await expect(shutdownInstrumentation()).resolves.toBeUndefined();
|
|
});
|
|
});
|
|
|
|
// The `@opentelemetry/*` packages are optional. When they are absent,
|
|
// `recordProviderPluginSpan` is a no-op by contract, so a native-duration test
|
|
// cannot run. Resolve the SDK first and skip the test when it is not installed.
|
|
const otelSdk = (() => {
|
|
try {
|
|
const require = createRequire(import.meta.url);
|
|
return {
|
|
api: require("@opentelemetry/api") as typeof import("@opentelemetry/api"),
|
|
sdk: require("@opentelemetry/sdk-trace-base") as typeof import("@opentelemetry/sdk-trace-base"),
|
|
};
|
|
} catch {
|
|
return null;
|
|
}
|
|
})();
|
|
|
|
const hrTimeToMs = (time: [number, number]): number => time[0] * 1000 + time[1] / 1e6;
|
|
|
|
describe.skipIf(!otelSdk)("recordProviderPluginSpan native duration", () => {
|
|
it("opens the span at the true start time and ends it at the true end time", async () => {
|
|
const { api, sdk } = otelSdk!;
|
|
const exporter = new sdk.InMemorySpanExporter();
|
|
const provider = new sdk.BasicTracerProvider({
|
|
spanProcessors: [new sdk.SimpleSpanProcessor(exporter)],
|
|
});
|
|
api.trace.setGlobalTracerProvider(provider);
|
|
try {
|
|
const { recordProviderPluginSpan } = await import("../instrumentation.js");
|
|
const startTimeMs = Date.now() - 4500;
|
|
const endTimeMs = startTimeMs + 4500;
|
|
recordProviderPluginSpan({
|
|
name: "sandbox.daytona.ensureDirectory",
|
|
parent: {
|
|
traceId: "0af7651916cd43dd8448eb211c80319c",
|
|
spanId: "b7ad6b7169203331",
|
|
traceFlags: 1,
|
|
},
|
|
attributes: { provider: "daytona" },
|
|
startTimeMs,
|
|
endTimeMs,
|
|
});
|
|
const finished = exporter.getFinishedSpans();
|
|
expect(finished).toHaveLength(1);
|
|
const span = finished[0]!;
|
|
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.
|
|
expect(Math.round(hrTimeToMs(span.duration as [number, number]))).toBe(4500);
|
|
} finally {
|
|
api.trace.disable();
|
|
}
|
|
});
|
|
|
|
it("opens and ends the span now when the timestamp pair is absent", async () => {
|
|
const { api, sdk } = otelSdk!;
|
|
const exporter = new sdk.InMemorySpanExporter();
|
|
const provider = new sdk.BasicTracerProvider({
|
|
spanProcessors: [new sdk.SimpleSpanProcessor(exporter)],
|
|
});
|
|
api.trace.setGlobalTracerProvider(provider);
|
|
try {
|
|
const { recordProviderPluginSpan } = await import("../instrumentation.js");
|
|
recordProviderPluginSpan({
|
|
name: "sandbox.daytona.pack",
|
|
parent: {
|
|
traceId: "0af7651916cd43dd8448eb211c80319c",
|
|
spanId: "b7ad6b7169203331",
|
|
traceFlags: 1,
|
|
},
|
|
attributes: { provider: "daytona" },
|
|
});
|
|
const finished = exporter.getFinishedSpans();
|
|
expect(finished).toHaveLength(1);
|
|
// The synchronous open-and-end path yields a near-zero native width.
|
|
expect(hrTimeToMs(finished[0]!.duration as [number, number])).toBeLessThan(1000);
|
|
} finally {
|
|
api.trace.disable();
|
|
}
|
|
});
|
|
});
|