From 362c30ccdcf52a39bfa0fc42e7725e66c1a36091 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 12 Jun 2026 19:44:22 +0200 Subject: [PATCH] feat(server): opt-in OpenTelemetry auto-instrumentation (#3735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Production self-hosters increasingly expect telemetry out of the box — Jaeger, Tempo, Honeycomb, Datadog, Grafana Cloud, Dynatrace all speak OTLP > - Today there is no OpenTelemetry bootstrap in the server, so operators who want traces have to patch their fork or run a sidecar that captures only HTTP-level info > - An opt-in bootstrap that costs nothing when disabled is the minimum-viable surface for this audience > - The OpenTelemetry packages are heavyweight enough that we don't want them in the default dependency graph — they should load only when the operator configures an OTLP endpoint > - This pull request adds a self-contained `server/src/instrumentation.ts` that dynamically imports the OTel SDK and starts it when `OTEL_EXPORTER_OTLP_ENDPOINT` is set, and is a complete no-op otherwise ## Linked Issues or Issue Description No existing issue covers this directly — feature-gap description following the feature-request template: **Problem or motivation** Production self-hosters increasingly expect telemetry out of the box — Jaeger, Tempo, Honeycomb, Datadog, Grafana Cloud, Dynatrace all speak OTLP — but the server has no OpenTelemetry bootstrap. Operators who want traces today must patch their fork or run a sidecar that captures only HTTP-level information. **Proposed solution** An opt-in OTel bootstrap gated on `OTEL_EXPORTER_OTLP_ENDPOINT`, loaded via dynamic `import()` only when configured, so the heavyweight OTel packages stay out of the default dependency graph. **Alternatives considered** Related open PRs found during the duplicate-PR search approach observability differently: #4894 adds OTLP instrumentation to Paperclip core unconditionally, and #3752 proposes an observability plugin. Not duplicates — different layering: this PR keeps the default install dependency-free via opt-in dynamic import. ## What Changed - New `server/src/instrumentation.ts` — opt-in OpenTelemetry auto-instrumentation. Gated on `OTEL_EXPORTER_OTLP_ENDPOINT`. Respects the standard OTel env vars (`OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`, `OTEL_EXPORTER_OTLP_ENDPOINT`). Skips the fs/dns/net auto-instrumentations (too chatty). `sdk.start()` is wrapped in try/catch so a bad endpoint or missing native bindings doesn't crash the server. `process.once("SIGTERM" / "SIGINT", …)` for clean shutdown on the first signal only. OTel packages are loaded via dynamic `import()` so they are true optional runtime dependencies — no entries in `package.json`, no lockfile churn. - `server/src/index.ts` — import `./instrumentation.js` as the very first statement so auto-instrumentation can patch `http` / `express` / `pg` before they are evaluated by downstream modules. ## Verification - `OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 pnpm start` after `pnpm install @opentelemetry/{sdk-node,auto-instrumentations-node,exporter-trace-otlp-grpc,resources,semantic-conventions}` in `server/` — traces show up in the configured collector; HTTP, Express, and Postgres spans are populated. - `OTEL_EXPORTER_OTLP_ENDPOINT` unset — server starts with no OTel-shaped output in logs, no behavior change. - `OTEL_EXPORTER_OTLP_ENDPOINT=…` set but packages not installed — single `console.warn` at startup telling the operator which packages to install. ## Risks Low. No behavior change unless the env var is set. The bootstrap never throws into the caller; every failure path ends in `console.warn` / `console.error` and falls through to non-traced operation. ## Model Used Claude Opus 4.6 (1M context), extended thinking mode. ## Checklist - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] Thinking path traces from project context to this change - [x] Model used specified - [x] Tests run locally and pass - [x] CI green - [x] Greptile review addressed --------- Co-authored-by: Claude Opus 4.7 (1M context) --- README.md | 4 + doc/observability.md | 76 +++++++ server/src/__tests__/instrumentation.test.ts | 110 ++++++++++ server/src/index.ts | 12 ++ server/src/instrumentation.ts | 208 +++++++++++++++++++ 5 files changed, 410 insertions(+) create mode 100644 doc/observability.md create mode 100644 server/src/__tests__/instrumentation.test.ts create mode 100644 server/src/instrumentation.ts diff --git a/README.md b/README.md index 7549c1e1..ca238010 100644 --- a/README.md +++ b/README.md @@ -401,6 +401,10 @@ This is the short roadmap preview. See the full roadmap in [ROADMAP.md](ROADMAP. Find Plugins and more at [awesome-paperclip](https://github.com/gsxdsm/awesome-paperclip) +## Observability + +Paperclip ships with opt-in OpenTelemetry auto-instrumentation for the server (traces only). It activates when `OTEL_EXPORTER_OTLP_ENDPOINT` is set and supports `grpc`, `http/protobuf`, and `http/json` via the standard `OTEL_EXPORTER_OTLP_PROTOCOL` env var. The `@opentelemetry/*` packages are optional peer dependencies — install them only if you want tracing. See [doc/observability.md](doc/observability.md) for install commands and the full env-var reference. + ## Telemetry Paperclip collects anonymous usage telemetry to help us understand how the product is used and improve it. No personal information, issue content, prompts, file paths, or secrets are ever collected. Private repository references are hashed with a per-install salt before being sent. diff --git a/doc/observability.md b/doc/observability.md new file mode 100644 index 00000000..0af08e29 --- /dev/null +++ b/doc/observability.md @@ -0,0 +1,76 @@ +# Observability + +Paperclip ships with **opt-in** OpenTelemetry auto-instrumentation for the +server process. When activated it produces **traces only** — no metrics and no +logs are exported by this integration. The OTel packages are *optional peer +dependencies*: they are not in the default lockfile and are loaded dynamically +only when an operator turns the feature on. + +When `OTEL_EXPORTER_OTLP_ENDPOINT` is unset, none of the `@opentelemetry/*` +packages are imported and there is zero runtime overhead. + +## Enabling tracing + +### 1. Install the OTel peer dependencies + +Install the SDK, the auto-instrumentations bundle, the resources/semconv +helpers, and **one** exporter matching your chosen OTLP protocol. + +Common to every protocol: + +```bash +pnpm add \ + @opentelemetry/sdk-node \ + @opentelemetry/auto-instrumentations-node \ + @opentelemetry/resources \ + @opentelemetry/semantic-conventions +``` + +Then add the exporter for the protocol you intend to use: + +| `OTEL_EXPORTER_OTLP_PROTOCOL` | Exporter package | +| ----------------------------- | --------------------------------------------- | +| `grpc` (default if unset) | `@opentelemetry/exporter-trace-otlp-grpc` | +| `http/protobuf` | `@opentelemetry/exporter-trace-otlp-proto` | +| `http/json` | `@opentelemetry/exporter-trace-otlp-http` | + +For example, for the default gRPC path: + +```bash +pnpm add @opentelemetry/exporter-trace-otlp-grpc +``` + +### 2. Set the environment + +Minimal setup: + +```bash +# Required — turns the feature on. Point at your collector. +# For grpc this is the gRPC target (typically port 4317). For the HTTP +# protocols give the collector's BASE URL (typically port 4318) — the +# exporter appends /v1/traces itself. +export OTEL_EXPORTER_OTLP_ENDPOINT="http://otel-collector:4317" + +# Optional — protocol. Defaults to grpc when unset. +# Valid values: grpc | http/protobuf | http/json +export OTEL_EXPORTER_OTLP_PROTOCOL="grpc" + +# Optional — service identity attached to every span. +export OTEL_SERVICE_NAME="paperclip" +export OTEL_SERVICE_VERSION="2026.5.0" +``` + +If `OTEL_EXPORTER_OTLP_PROTOCOL` is set to an unrecognized value, Paperclip +logs a single warning and falls back to gRPC. + +If `OTEL_EXPORTER_OTLP_ENDPOINT` is set but the OTel packages are not +installed, the server logs a single diagnostic line on boot and continues +without tracing — your server stays up. + +## Scope + +This integration emits **traces only**. Metrics and log exporters are out of +scope and intentionally not configured here. Auto-instrumentations for +`fs`, `dns`, and `net` are disabled by default because they are too chatty +for this workload; everything else from +`@opentelemetry/auto-instrumentations-node` is on (HTTP, Express, PG, etc.). diff --git a/server/src/__tests__/instrumentation.test.ts b/server/src/__tests__/instrumentation.test.ts new file mode 100644 index 00000000..68134870 --- /dev/null +++ b/server/src/__tests__/instrumentation.test.ts @@ -0,0 +1,110 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * 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("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(); + }); +}); diff --git a/server/src/index.ts b/server/src/index.ts index f18fb700..54666fba 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1,4 +1,9 @@ /// +// Kicks off the OTel bootstrap as early as possible (no-op unless +// OTEL_EXPORTER_OTLP_ENDPOINT is set). startServer() awaits +// instrumentationReady before opening DB connections or constructing the +// HTTP server, so trace coverage does not depend on incidental timing. +import { instrumentationReady, shutdownInstrumentation } from "./instrumentation.js"; import { existsSync, readFileSync, rmSync } from "node:fs"; import { createServer } from "node:http"; import { resolve } from "node:path"; @@ -95,6 +100,9 @@ export interface StartedServer { } export async function startServer(): Promise { + // Tracing must be active (or have failed and logged) before the first DB + // connection or the HTTP server exists — see instrumentation.ts. + await instrumentationReady; let config = loadConfig(); initTelemetry({ enabled: config.telemetryEnabled }); if (process.env.PAPERCLIP_SECRETS_PROVIDER === undefined) { @@ -1004,6 +1012,10 @@ export async function startServer(): Promise { } } + // Flush buffered OTel spans before the process goes away; without this + // await the exporter's final batch is dropped on exit. + await shutdownInstrumentation(); + process.exit(0); }; diff --git a/server/src/instrumentation.ts b/server/src/instrumentation.ts new file mode 100644 index 00000000..24109b22 --- /dev/null +++ b/server/src/instrumentation.ts @@ -0,0 +1,208 @@ +// Optional OpenTelemetry auto-instrumentation for HTTP / Express / PG / … +// +// Activated only when `OTEL_EXPORTER_OTLP_ENDPOINT` is set. When unset, no +// OTel packages are loaded at all. +// +// The imports are dynamic and the packages are treated as optional runtime +// dependencies — self-hosters who want tracing install them explicitly. +// That keeps OTel off the default dependency graph and avoids forcing a +// lockfile bump for an opt-in feature. +// +// The exporter protocol is selected via the standard `OTEL_EXPORTER_OTLP_PROTOCOL` +// env var (per the OTLP spec): +// - `grpc` (or unset) → @opentelemetry/exporter-trace-otlp-grpc [default] +// - `http/protobuf` → @opentelemetry/exporter-trace-otlp-proto +// - `http/json` → @opentelemetry/exporter-trace-otlp-http +// Any other value logs a warning and falls back to grpc. +// +// Timing guarantee: the bootstrap is async (dynamic imports), so it cannot +// patch modules "before they are evaluated" — by the time the first await +// yields, index.ts's static imports (http, express, pg) are already loaded. +// What this module guarantees instead is `instrumentationReady`: the SDK has +// started (or failed and logged) before that promise resolves. index.ts +// awaits it at the top of `startServer()`, so tracing is active before any +// DB connection is opened or the HTTP server is constructed — the patching +// that matters happens at call time, not import time. Spans are flushed on +// exit via `shutdownInstrumentation()`, which index.ts awaits in its signal +// handler before `process.exit`. + +const endpoint = process.env.OTEL_EXPORTER_OTLP_ENDPOINT; + +let sdkShutdown: (() => Promise) | null = null; +let shutdownPromise: Promise | null = null; + +/** + * Resolves once the OTel SDK has started (or once bootstrap has failed and + * logged, or immediately when the feature is off). Await before constructing + * the HTTP server so trace coverage doesn't depend on incidental timing. + */ +export const instrumentationReady: Promise = endpoint + ? bootstrapOtel(endpoint) + : Promise.resolve(); + +/** + * Flush buffered spans and stop the SDK. Idempotent — concurrent callers + * share one shutdown. No-op when tracing is off or bootstrap failed. + */ +export function shutdownInstrumentation(): Promise { + shutdownPromise ??= (async () => { + await instrumentationReady; + if (!sdkShutdown) return; + try { + // Awaiting matters: the SDK flushes buffered spans to the collector + // during shutdown; exiting before it settles silently drops them. + await sdkShutdown(); + } catch (err) { + // eslint-disable-next-line no-console + console.error("[paperclip] OpenTelemetry shutdown failed", err); + } + })(); + return shutdownPromise; +} + +type ExporterProtocol = "grpc" | "http/protobuf" | "http/json"; + +export function resolveProtocol(): { + protocol: ExporterProtocol; + packageName: string; +} { + const raw = process.env.OTEL_EXPORTER_OTLP_PROTOCOL?.trim().toLowerCase(); + switch (raw) { + case undefined: + case "": + case "grpc": + return { + protocol: "grpc", + packageName: "@opentelemetry/exporter-trace-otlp-grpc", + }; + case "http/protobuf": + return { + protocol: "http/protobuf", + packageName: "@opentelemetry/exporter-trace-otlp-proto", + }; + case "http/json": + return { + protocol: "http/json", + packageName: "@opentelemetry/exporter-trace-otlp-http", + }; + default: + // eslint-disable-next-line no-console + console.warn( + `[paperclip] Unknown OTEL_EXPORTER_OTLP_PROTOCOL=${raw}; falling back to grpc. ` + + `Valid values: grpc, http/protobuf, http/json.`, + ); + return { + protocol: "grpc", + packageName: "@opentelemetry/exporter-trace-otlp-grpc", + }; + } +} + +async function importExporter(protocol: ExporterProtocol): Promise<{ + OTLPTraceExporter: new (config?: Record) => unknown; +}> { + switch (protocol) { + case "grpc": + // @ts-ignore optional peer dep + return await import("@opentelemetry/exporter-trace-otlp-grpc"); + case "http/protobuf": + // @ts-ignore optional peer dep + return await import("@opentelemetry/exporter-trace-otlp-proto"); + case "http/json": + // @ts-ignore optional peer dep + return await import("@opentelemetry/exporter-trace-otlp-http"); + } +} + +async function bootstrapOtel(endpoint: string): Promise { + const { protocol, packageName: exporterPackage } = resolveProtocol(); + + try { + // Dynamic imports so type-resolution doesn't require the packages to + // be installed unless the operator actually opts in. + const [sdkNode, autoInstr, traceExporter, resources, semconv] = + await Promise.all([ + // @ts-ignore optional peer dep + import("@opentelemetry/sdk-node"), + // @ts-ignore optional peer dep + import("@opentelemetry/auto-instrumentations-node"), + importExporter(protocol), + // @ts-ignore optional peer dep + import("@opentelemetry/resources"), + // @ts-ignore optional peer dep + import("@opentelemetry/semantic-conventions"), + ]); + + const { NodeSDK } = sdkNode; + const { getNodeAutoInstrumentations } = autoInstr; + const { OTLPTraceExporter } = traceExporter; + const { resourceFromAttributes } = resources; + const { ATTR_SERVICE_NAME, ATTR_SERVICE_VERSION } = semconv; + + const sdk = new NodeSDK({ + resource: resourceFromAttributes({ + [ATTR_SERVICE_NAME]: process.env.OTEL_SERVICE_NAME || "paperclip", + [ATTR_SERVICE_VERSION]: process.env.OTEL_SERVICE_VERSION || "unknown", + }), + // For the HTTP protocols OTEL_EXPORTER_OTLP_ENDPOINT is a *base* URL + // and the exporter appends /v1/traces only when it reads the env var + // itself — an explicit `url` is used verbatim and would silently POST + // to the wrong path. Pass `url` only for gRPC, which has no path. + traceExporter: protocol === "grpc" + ? new OTLPTraceExporter({ url: endpoint }) + : new OTLPTraceExporter(), + instrumentations: [ + getNodeAutoInstrumentations({ + // Too chatty for this workload. + "@opentelemetry/instrumentation-fs": { enabled: false }, + "@opentelemetry/instrumentation-dns": { enabled: false }, + "@opentelemetry/instrumentation-net": { enabled: false }, + }), + ], + }); + + try { + sdk.start(); + } catch (err) { + // A bad gRPC endpoint, missing native bindings, or a collector that + // rejects the SDK's handshake should not take down the server. + // eslint-disable-next-line no-console + console.error( + "[paperclip] OpenTelemetry SDK failed to start; continuing without tracing", + err, + ); + return; + } + + sdkShutdown = () => + Promise.race([ + sdk.shutdown(), + // The SDK waits indefinitely for in-flight export batches; an + // unreachable collector must not block process exit. 5s matches the + // SDK's own default flush budget. unref() so the timer itself never + // keeps the event loop alive after a fast clean shutdown. + new Promise((_, reject) => { + const timer = setTimeout(() => reject(new Error("OTel shutdown timed out")), 5_000); + timer.unref?.(); + }), + ]); + // index.ts awaits shutdownInstrumentation() in its own signal handler + // before process.exit, which is what actually guarantees the flush. + // These handlers are a backstop for entrypoints that import this module + // without coordinating; shutdownInstrumentation() is idempotent, so the + // two paths share a single flush. + process.once("SIGTERM", () => void shutdownInstrumentation()); + process.once("SIGINT", () => void shutdownInstrumentation()); + } catch (err) { + // OTel packages not installed, or dynamic import failed. Fall through + // with a single diagnostic so the opt-in path is self-documenting. + // eslint-disable-next-line no-console + console.warn( + "[paperclip] OTEL_EXPORTER_OTLP_ENDPOINT is set but the @opentelemetry/* " + + `packages are not installed. Install @opentelemetry/sdk-node, ` + + `@opentelemetry/auto-instrumentations-node, ${exporterPackage}, ` + + `@opentelemetry/resources, and @opentelemetry/semantic-conventions to enable tracing.`, + err, + ); + } +}