Skip to content

Node default integration adds a full OpenTelemetry NodeSDK for logs and leaves no posthog-node shutdown #1330

Open

Description

@gewenyu99

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.json gained @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 were posthog-node and dotenv.
  • posthog-logs.js builds new NodeSDK({ resource, logRecordProcessors: [new BatchLogRecordProcessor(new OTLPLogExporter(...))] }) and calls sdk.start(). It has no shutdown.
  • index.js has four posthog.capture() calls. Each one is followed by a posthogLogs.info() call with the same facts. index.js and posthog.js never call posthog.shutdown() or posthog.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 ... and let sdk; from posthog-logs.js.
  • Installed @opentelemetry/sdk-node 0.222.0: build/src/sdk.js:28-33 and :146-149 build OTLP metric readers from env when none are passed. build/src/utils.js:83-89 builds an OTLP trace exporter when OTEL_TRACES_EXPORTER is empty. build/src/sdk.js:214-229 registers 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.js added a shutdown() that closes the server and awaits posthog.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-34 queues logs on every default run: "Queue it even when AIO will be skipped."
  • context/agents/integration-v2/logs.md:18-26 says 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-30 builds logs-nodejs from https://posthog.com/docs/logs/installation/nodejs.md. That page uses NodeSDK and 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. In context/skills/posthog-best-practices/description.md, the long-running server rule at :125 does not mention shutdown, and :130 asks for shutdown() only in short-lived processes. The Rust rule at context/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.md has no shutdown requirement.

Suggested fix

  1. 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 console use at request boundaries), or when the user asks. Otherwise, complete it as not needed and put one line in the report about how to add logs later. In logs.md, say that a log line must not repeat a capture() call.
  2. For plain Node, use a LoggerProvider from @opentelemetry/sdk-logs with BatchLogRecordProcessor and OTLPLogExporter, and register it with logs.setGlobalLoggerProvider. Do not use @opentelemetry/sdk-node. Use NodeSDK only when the app already runs one, and then attach to it. Add this as a logs-nodejs commandment or override in context-mill, and fix the posthog.com Node logs page to match.
  3. Always flush on shutdown for long-running Node servers. Change the javascript_node commandment to match the Rust wording: register SIGINT and SIGTERM handlers that close the server and await posthog.shutdown(), even when the app has no shutdown path. When logs are set up, the same handler also awaits the logger provider's shutdown(). Make init.md own this. In review.md, say that shutdown and flush wiring is never an unused helper: the fix is to wire it, not to delete it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions