mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 09:37:19 +01:00
66e8fab9447aa21350f87ddec79fcd027f3e23ad
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicky LeachandPaperclip
|
1d571de5f9 |
Add shared native turn attribution boundaries (#18213)
## Thinking Path > - Paperclip manages work performed by AI agents. > - Native execution crosses the plugin worker, Runner, sidecar, and durable event boundaries. > - Existing outer spans leave repeated preparation and admission work unattributed. > - Optimizations need a common baseline with accurate ownership and event meanings. > - This change extends the existing opt-in tracing contract with bounded timing receipts. > - It preserves execution policy and keeps provider internals opaque where the API provides no timing proof. ## Linked Issues or Issue Description **What existing behavior does this improve?** Attribution of native task preparation, retained/cold Runner handoff, and completion-to-text work. **Current behavior** Worker spans use a flat provider contract. Workspace calls lack a consistent dispatch envelope. Submitted-to-started timing is named as a provider queue even though it includes other work. The transport does not expose a shared breakdown of its waits. **Proposed improvement** Add invocation-scoped nesting, bounded numeric remote receipts, current-run transport timing, durable receipt intervals, and distinct completion/text milestones. Reuse the existing exporter. No latency optimization is included. Related public work: #17842 consolidates existing startup tracing tests; it does not add this runtime coverage. ## What Changed - Extend the optional SDK tracer with nested async scopes and completed timing receipts. Keep the host responsible for parent identity, capability checks, attributes, clocks, depth, and count limits. - Add dispatch and bounded host-read timings for environment and workspace providers. - Resolve transport parents on each call so retained sessions do not retain an earlier run's trace. - Split cold/resume/attach/prepare/registration/command/started-event waits. Add opt-in numeric ACPX Runner/sidecar status and prompt-start receipts. - Distinguish first completion request, validated tool receipt, final result commit, first nonempty text, and event persistence. Rename the submitted-to-started interval to match its meaning. - Link completed earlier remote lifecycles to their host-owned observer rather than placing them under a later polling request. Preserve sampling, timestamp limits and budgets. - Document the versioned coverage map, causal clock limits, and baseline acceptance requirements. ## Verification - Current head: GitHub CI is fully green. Standard Greptile and Apex are both 5/5, with no unresolved review threads. Full-suite validation was performed in CI; follow-up local validation was limited to affected packages and tests. - Review follow-up: early rejection, missing provider capability and startup-parent regressions were reproduced and fixed. The affected worker/SDK/timing set passes 128 tests; 28 timing/context tests, SDK typecheck and direct server TypeScript checking pass. The server typecheck wrapper reached an unrelated Runner rebuild and stopped because Cargo was absent from PATH; full Runner/image validation remains in CI. - CI failure accounting: the earlier Turbo cache/force conflict and Linux OpenCode shell assertion were reproduced and diagnosed. A direct proc probe ruled out permissions; a portable shell comparison identified Bash skipping BASH_ENV with stripped SHLVL and socket stdin. Upstream #18230 now supplies the CI and ignored-stdin fixture fixes; this PR adopts them without duplicate CI changes. The strict Linux OOM assertion passed before this source-equivalent merge. - Retry follow-up: the three insertion/replay/failure timing cases and server typecheck pass. The existing database regression now asserts a single marker across identical result replay; the current-head CI integration gate passes. - Upstream CI alignment: all three real-Turbo 2.11.10 integration cases pass. The repository policy gate rejected committing a generated lockfile; the final PR contains no lockfile edit and uses the maintained inline stale-lockfile recovery. - Linked-lifecycle follow-up: 44 focused host/export tests pass (three optional SDK-dependent cases skip); SDK and server typechecks pass. The API contract test verifies an explicit root context, host-owned observer link and unsampled suppression. - SDK typecheck and focused worker/invocation tests: 30 passed, including clock changes, bounded legacy IPC, and manual-span creation-scope parentage. - Focused reviewed host/native/transaction-boundary checks: 44 passed. The 86-test worker-manager suite passes after updating its exact assertions for the host-owned timing budget found by CI. Earlier preparation/first-text checks also passed. - Affected server and Runner TypeScript checks pass after building local workspace declarations. - Merge follow-up: the profile/candidate startup subset passes 94 tests, the four Grok source-identity checks pass, and Runner TypeScript checking passes. A subsequent execution-factory merge passes 21 focused timing/trace tests and server TypeScript checking. - Pinned Rust build and the ACPX Rust subset pass. The hostile receipt projection test passes. Source profile compatibility checks: 127 passed, one existing skip. Grok identity checks: four passed. - Broad transport/ACPX suite: 410 passed after building all fake provider binaries. Earlier attempts failed on missing Runner/fake-provider outputs; building the required fixtures resolved those failures. - The first CI run correctly rejected stale source attestations. After integrating current master, Grok advances 13→14 and Copilot 84→85; master’s historical fixtures and Pi 35 stay byte-identical. Provider dependencies, models and permission defaults are unchanged. Identity generation/checks now pass. - The added real-Postgres result lock regression could not initialize its local embedded cluster. The insertion/replay/failure unit regressions pass. The current-head CI integration gate passes. - Comprehensive repository checks and image builds are delegated to CI, as requested for this instrumentation pass. No deployment, model run, collector-ingestion proof, or latency improvement is claimed. ## Risks The new SDK methods are optional. Legacy span calls and endpoint-off behavior remain supported. The host accepts only closed timing names and bounded attributes. Remote durations are anchored at local receipt; transport/scheduling uncertainty remains explicit. Trace trees have count/depth limits. The first rollout must inspect actual collector exports before treating the baseline as accepted. Fresh live qualification for the new source identities remains a rollout requirement. Revert these commits to remove the new measurements; no schema migration is required. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and local code/tool execution. The exact runtime model ID and context-window size are not exposed in this session. ## 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 described the issue in-PR following the enhancement template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal issue 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] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Standard Greptile and Apex are both 5/5, with no unresolved findings - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Dotta
|
6df9fc2221 |
test(server): consolidate duplicated startup-tracing unit suites (#17842)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/48/changes/paperclip/48 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server can export manual OpenTelemetry startup spans through `server/src/instrumentation.ts` (`resolveProtocol` plus the `getStartupTracer` / `getStartupTraceContext` accessors). > - Two suites covered that one module in every server Vitest run: `server/src/instrumentation.test.ts` (unit cases added with the manual-span seam in #10565) and `server/src/__tests__/instrumentation.test.ts` (the bootstrap machinery from #3735/#12249). > - `resolveProtocol` and the startup tracer accessors were registered twice per run with overlapping but unequal tables, so the same contracts were executed by two files while each held a few unique rows the other lacked. > - This pull request moves the unique assertions into the `__tests__` owner and retires the colocated file, so one suite exercises the full contract once. > - The benefit is one fewer suite registration per server run and one less duplicated unit surface to maintain, with no lost assertion. ## Linked Issues or Issue Description **What happened?** The server Vitest project ran two test files against `server/src/instrumentation.ts`. Both registered `resolveProtocol` and startup-tracer accessor cases. The colocated file held unique coverage (protocol input trimming, warn-once, the `getStartupTraceContext` accessor, the `setAttribute`/`setStatus` span surface) while the `__tests__` file held the bootstrap, peer-version, shutdown, and span-recording coverage. The split duplicated executions and made the contract's owner ambiguous. **Expected behavior** One suite (`server/src/__tests__/instrumentation.test.ts`) owns every `server/src/instrumentation.ts` unit contract. Every assertion from the retired file still runs, inside the owner. No production code changes. ## What Changed - Retired `server/src/instrumentation.test.ts` after migrating its unique assertions into `server/src/__tests__/instrumentation.test.ts`. - Added the `OTEL_EXPORTER_OTLP_PROTOCOL=" HTTP/PROTOBUF "` row, proving surrounding space is trimmed (previously only the letter-case row existed). - Strengthened the unknown-protocol case to assert the warning fires exactly once. - Extended the no-op tracer case to exercise `setAttribute` and `setStatus` before `end`. - Added the `getStartupTraceContext` accessor case (tracer surface plus the `contextWithSpan` helper), previously covered only by the retired file. Six production callers use this seam (heartbeat runtime execution, native run trace, duplex observability recorder, and others). ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/instrumentation.test.ts` — 21 passed, 2 skipped (SDK-gated span-recording cases skip without `@opentelemetry/*`, as before). The two new/extended cases pass inside the owner. - Deliberate failure controls, both restored byte-exact afterward and re-run green: - removing `.trim()` from `resolveProtocol` made exactly the migrated `" HTTP/PROTOBUF "` row fail; - making the no-op `contextWithSpan` throw made exactly the migrated accessor case fail. - The retired file's six remaining `resolveProtocol` rows and one `getStartupTracer` case were exact-value duplicates of owner rows (same inputs, same expected protocol/package pairs, same no-op tracer shape). - Full repository typecheck/test/build were not run locally (no Rust toolchain for the Runner build in this environment; server-wide `tsc --noEmit` exceeds the session window). CI is the authoritative gate for those; this diff touches only test code under `server/src`. ## Risks Low. Test-only change; no production behavior, routing, or CI configuration touched. Known stale mentions of the retired path are non-functional: `.devin/wiki.json` lists it as a non-exhaustive wiki anchor (its own instructions say to relocate removed anchors at generation time) and `scripts/general-server-shard-durations.json` keeps a historical duration key that no longer matches any selected file. ## Model Used - Provider: Z.ai via OpenRouter (agent: Paperclip Test Gardener) - Model ID: `openrouter/z-ai/glm-5.3` - Reasoning: standard chain-of-thought mode, tool use (shell, file edit, GitHub CLI) - Context window: 200k tokens (provider default for this model) ## 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 (none required: test-only change) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge |
||
|
Nicky LeachandPaperclip
|
06cd21ed0f |
fix(observability): declare the optional OpenTelemetry peer dependencies (#12249)
## Thinking Path > - Paperclip manages AI agents for work. > - Paperclip includes an observability path that operators can enable for tracing. > - The server loads several OpenTelemetry packages only when tracing is enabled. > - The documentation calls these packages optional peer dependencies, but the server manifest does not declare them. > - This gap hides supported versions and stops Dependabot from maintaining the packages. > - This pull request aligns package metadata, runtime checks, and documentation with the opt-in tracing design. > - The change gives operators clear installation behavior and keeps the no-op default. ## Linked Issues or Issue Description This pull request fixes a package metadata and installation defect. Related observability work appears in [#8476](https://github.com/paperclipai/paperclip/pull/8476) and [#9672](https://github.com/paperclipai/paperclip/pull/9672). The server documentation described optional OpenTelemetry peer dependencies, but `server/package.json` did not declare them. Package managers and Dependabot could not see the supported version ranges. The UI and Claude local adapter also relied on automatic peer installation for `yjs` and `@anthropic-ai/sdk`. The package manifests now declare the optional runtime packages. A default install does not install optional tracing peers. The server keeps its no-op behavior when tracing is disabled or a peer is absent. ## What Changed - Add seven optional OpenTelemetry packages to `server/package.json` and mark each package as optional. - Keep `@opentelemetry/api` as a normal dependency for the no-op interface. - Disable automatic peer installation in `.npmrc`. - Declare `yjs` for the UI package and `@anthropic-ai/sdk` for the Claude local adapter. - Check declared peer versions before the server loads a dynamic OpenTelemetry import. - Keep the endpoint gate, dynamic imports, and fail-open behavior unchanged. - Update the observability and README documentation. - Tell Dependabot that its npm parser does not read `peerDependencies`. ## Verification - Targeted server tests pass: 34 passed and 2 skipped. - The skipped tests require the real OpenTelemetry SDK and remain pre-existing. - The pull request workflow regenerates the lockfile because manifest files and `.npmrc` changed. - The policy job confirms that the pull request does not include `pnpm-lock.yaml`. - GitHub checks pass except `security/snyk (cryppadotta)`, which remains pending after its authorized wait cap. - Greptile Review reports 5/5 with no open findings. - Server typecheck passes. ## Risks - Optional peers can produce a diagnostic when the installed version does not match the declared range. - A missing optional peer does not stop the server. - Disabling automatic peer installation can expose undeclared package use in other workspaces. - This pull request declares the affected packages and adds tests for the changed behavior. - This pull request makes no database or API changes. ## Model Used OpenAI Codex, GPT-5, with repository inspection and pull request preparation. ## 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 - [x] My branch name describes the change and contains no internal Paperclip ticket id - [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] I have considered and documented any risks above - [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> |
||
|
Nicky LeachandPaperclip
|
9ace548fd2 |
feat(observability): rename sandbox provider spans and add run-time wrapper spans (#10999)
## 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> |
||
|
Nicky LeachandPaperclip
|
d114c4925e |
feat(observability): give sandbox sync spans true wall-clock width (#10864)
## Thinking Path > - Paperclip keeps company work visible and governed. > - Sandbox agents run serial sync work across worker and host boundaries. > - The current span path hid real wall-clock time for that sync work. > - The host needs safe timestamps if it wants true span width. > - This pull request carries worker timestamps, validates them, and records the real duration. > - The benefit is clearer operator visibility for sandbox sync work. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting. This touches `packages/plugins`, `server`, and the Daytona plugin test surface. **Problem or motivation** Sandbox sync spans opened and closed in one host call. The native width stayed near zero, so the real time spent in serial round trips was hard to see. **Proposed solution** Carry worker start and end times across the span record protocol. Validate the pair at the host boundary. Record the host span with the true duration when the pair is safe. **Alternatives considered** Keep the numeric duration only. That keeps the data, but it does not widen the span and it does not show the real wall-clock time. **Roadmap alignment** This fits the `Cloud / Sandbox agents` and `Artifacts & Work Products` areas in `ROADMAP.md`. I found no other roadmap item that covers this span-width gap. **Additional context** The host allowlist stays narrow. Unknown names still map to `sandbox.provider.other`. Invalid timestamp pairs still fall back to the synchronous path. Related public PRs: none found. ## What Changed - Added optional `startTimeMs` and `endTimeMs` fields to the `span.record` protocol. - Captured start and end times in the worker tracer and sent them to the host. - Validated host timestamps with finite, ordered, bounded checks before span reconstruction. - Extended the host allowlist to the sandbox sync command names. - Wrapped each inbound sync round trip in its own named span. - Added tests for the worker path, host boundary, host recorder, and Daytona sync flow. ## Verification - `pnpm --filter @paperclipai/plugins-sdk test` - `pnpm --filter @paperclipai/server test` - `pnpm --filter @paperclipai/daytona-plugin test` - `pnpm --filter @paperclipai/server tsc --noEmit` still shows pre-existing `drizzle-orm` duplicate-declaration errors in this sandbox. The changed files do not touch those lines. - GitHub checks are green. - Greptile review is 5/5. - No open review threads remain. ## Risks - A bad timestamp pair can fall back to the synchronous path. - The host clock gate can reject spans if the pair is stale, reversed, or too large. - The new worker fields change the wire protocol, but the public plugin tracer contract stays the same. ## Model Used OpenAI GPT-5, tool-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 linked existing issues with `Fixes: #` / `Closes #` / `Refs #` or described the issue in-PR following the relevant issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change 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] I have considered and documented any risks above - [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> |
||
|
Nicky LeachandPaperclip
|
740554acc6 |
feat: add a no-op OpenTelemetry span seam for the sandbox startup path (#10522)
## Thinking Path > - Paperclip helps people run and govern AI agent work > - Sandbox startup needs a safe place to add telemetry spans without forcing OpenTelemetry on every run > - This change adds a no-op span seam, so the startup path can accept a tracer later and still stay inert now > - The server gets a lazy tracer accessor, and the adapter timing helper gets an injected tracer hook > - The change keeps the default path free of OpenTelemetry and keeps the existing startup event path unchanged > - The benefit is a future-safe seam with no runtime change today ## Linked Issues or Issue Description This PR addresses a feature gap in the sandbox startup path. ### Problem Sandbox startup has no safe span seam. A direct OpenTelemetry import would load telemetry packages on every run. ### Proposed Solution Add a lazy tracer accessor in the server. Add an injected no-op tracer seam in startup timing. ### Alternatives Import OpenTelemetry directly in the startup path. Reject that path because the default startup flow must stay inert. ## What Changed - Added a lazy startup tracer accessor in `server/src/instrumentation.ts`. - Added an injected startup tracer seam in `packages/adapter-utils/src/acpx-engine/startup-timing.ts`. - Kept the startup event path unchanged. - Kept `adapter-utils` free of OpenTelemetry imports. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/startup-timing.test.ts` - `pnpm exec vitest run server/src/__tests__/instrumentation.test.ts` - `tsc --noEmit` for `@paperclipai/adapter-utils` and `@paperclipai/server` ## Risks Low risk. The default tracer is a no-op, so the runtime path stays inert until a later change injects a real tracer. ## Model Used OpenAI GPT-5. Tool use and code execution. ## 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 found none - [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 or instance-local Paperclip issues or links - [x] My branch name describes the change 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 - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Jannes StubbemannandClaude Opus 4.7
|
362c30ccdc |
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> |