feat(server): opt-in OpenTelemetry auto-instrumentation (#3735)
## 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) <noreply@anthropic.com>
This commit is contained in:
Jannes Stubbemann
committed by
GitHub
GitHub
parent
937fe62d10
commit
362c30ccdc
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,9 @@
|
||||
/// <reference path="./types/express.d.ts" />
|
||||
// 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<StartedServer> {
|
||||
// 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<StartedServer> {
|
||||
}
|
||||
}
|
||||
|
||||
// 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);
|
||||
};
|
||||
|
||||
|
||||
@@ -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<void>) | null = null;
|
||||
let shutdownPromise: Promise<void> | 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<void> = 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<void> {
|
||||
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<string, unknown>) => 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<void> {
|
||||
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<void>((_, 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,
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user