Repository navigation
Node default integration adds a full OpenTelemetry NodeSDK for logs and leaves no posthog-node shutdown #1330
Description
Problem
Since context-mill v1.55.0 (#390), a default integration run always queues a Logs task. On a small Express todo API, that task added five @opentelemetry packages and a new posthog-logs.js. It also added a posthogLogs.info() call after every posthog.capture(). The file starts a full NodeSDK from @opentelemetry/sdk-node just to export logs.
NodeSDK does more than logs. With only logRecordProcessors set, it also registers a global tracer provider and a meter provider. Both get OTLP exporters built from OTEL_* environment defaults. They do nothing today. But they pull in about 30 @opentelemetry packages (about 35 MB) plus @grpc. Any span or metric the app adds later goes to the default OTLP endpoint, not to PostHog. This is too much for a todo API that did not ask for logs.
The same run added no graceful shutdown. Nothing calls posthog.shutdown() on SIGINT or SIGTERM. posthog-node and the OTel BatchLogRecordProcessor both buffer, so events and log records queued at stop time are lost. The Logs task did write an exported shutdown helper. The review task then deleted it under its "Minimal" rule, because nothing called it. The posthog-node client never got a shutdown path.
An earlier run of the same program on the same app, on context-mill v1.54.1 before #390, added server.close() and await posthog.shutdown() on SIGINT and SIGTERM. This is one run per version, so it shows the guidance allows both results. It does not prove a regression.
Evidence
Sweep: the posthog-integration program on basic-integration/javascript-node/express-todo, pi harness, orchestrator sequence. Wizard build 2.76.1, at the refactor stack's B2 head d938a43 (#1307). Skills came from context-mill releases/latest, which was v1.55.0. logs-nodejs came from releases/download/v1.55.0/logs-nodejs.zip.
- Plan: 10 tasks. "Configure log capture" completed. "Assess AI observability" was skipped. The seed queued the Logs task with the reason "Log capture follows AI instrumentation to preserve any tracing configuration it adds."
package.jsongained@opentelemetry/api-logs,@opentelemetry/exporter-logs-otlp-http,@opentelemetry/resources,@opentelemetry/sdk-logs, and@opentelemetry/sdk-node(all 0.222.x or 2.11.x). The other new dependencies wereposthog-nodeanddotenv.posthog-logs.jsbuildsnew NodeSDK({ resource, logRecordProcessors: [new BatchLogRecordProcessor(new OTLPLogExporter(...))] })and callssdk.start(). It has no shutdown.index.jshas fourposthog.capture()calls. Each one is followed by aposthogLogs.info()call with the same facts.index.jsandposthog.jsnever callposthog.shutdown()orposthog.flush(), and they register no signal handlers.- Review status line in the wizard log: "The minimality review found one unused exported shutdown helper; removing it". The edit that follows removes
async function ...andlet sdk;fromposthog-logs.js. - Installed
@opentelemetry/sdk-node0.222.0:build/src/sdk.js:28-33and:146-149build OTLP metric readers from env when none are passed.build/src/utils.js:83-89builds an OTLP trace exporter whenOTEL_TRACES_EXPORTERis empty.build/src/sdk.js:214-229registers the global tracer provider. - Comparison run: the same program and app on the Release A top, 200961f (refactor(agent): WIP publish the agent through entry modules #1303), with context-mill v1.54.1. That run had no Logs task, and its
index.jsadded ashutdown()that closes the server and awaitsposthog.shutdown()on SIGINT and SIGTERM.
Where it lives. The wizard gets these prompts at runtime from context-mill releases/latest, so this happens on wizard main too. It does not depend on the refactor stack. On main, the only wizard-side control is the kill switch: src/lib/programs/posthog-integration/index.ts:255-258 (excludedTaskTypes) and src/shared/constants.ts:310 (wizard-default-aio-logs). An explicit 'false' drops the task for everyone. The fix belongs in PostHog/context-mill, and the Node setup code comes from the posthog.com Node logs page. Paths are on context-mill main, dc2898b (v1.55.0):
context/agents/integration-v2/integrate-posthog.md:29-34queueslogson every default run: "Queue it even when AIO will be skipped."context/agents/integration-v2/logs.md:18-26says to use "the documented OTLP exporter" for the platform. It gives no rule for when an app needs logs, and nothing stops a log line from repeating every capture.context/skills/logs/config.yaml:26-30buildslogs-nodejsfromhttps://posthog.com/docs/logs/installation/nodejs.md. That page usesNodeSDKand shows no shutdown.context/agents/integration-v2/review.md:58-60: "Delete helpers nothing calls" removed the only shutdown code in the run.context/commandments.yaml:65(javascript_node): "await flush() or await shutdown() before letting that process exit". A long-running server with no exit path can read this as not applying. Incontext/skills/posthog-best-practices/description.md, the long-running server rule at:125does not mention shutdown, and:130asks forshutdown()only in short-lived processes. The Rust rule atcontext/commandments.yaml:97(chore: papercut for mcp install #354) already closes this gap for Rust: "An app with no shutdown path still needs this".context/agents/integration-v2/init.mdhas no shutdown requirement.
Suggested fix
- Offer logs only when the app needs them. Queue or run the Logs task when the app already has a logger (pino, winston, bunyan, or structured
consoleuse at request boundaries), or when the user asks. Otherwise, complete it asnot neededand put one line in the report about how to add logs later. Inlogs.md, say that a log line must not repeat acapture()call. - For plain Node, use a
LoggerProviderfrom@opentelemetry/sdk-logswithBatchLogRecordProcessorandOTLPLogExporter, and register it withlogs.setGlobalLoggerProvider. Do not use@opentelemetry/sdk-node. UseNodeSDKonly when the app already runs one, and then attach to it. Add this as alogs-nodejscommandment or override in context-mill, and fix the posthog.com Node logs page to match. - Always flush on shutdown for long-running Node servers. Change the
javascript_nodecommandment to match the Rust wording: register SIGINT and SIGTERM handlers that close the server and awaitposthog.shutdown(), even when the app has no shutdown path. When logs are set up, the same handler also awaits the logger provider'sshutdown(). Makeinit.mdown this. Inreview.md, say that shutdown and flush wiring is never an unused helper: the fix is to wire it, not to delete it.
Activity