Repository navigation
feat(node): metrics autocapture for HTTP, database and runtime metrics - #5149
Draft
DanielVisca wants to merge 6 commits into
Draft
DanielVisca wants to merge 6 commits into
DanielVisca wants to merge 6 commits into
Conversation
Contributor
Contributor
|
[High risk] Adds OpenTelemetry instrumentation and metrics export to the Node SDK. The PR is not ready to merge because a trailing-slash host can misroute exports and signal termination loses the final metrics window. Reviews (1) · Last reviewed commit: "feat(node): add metrics autocapture for ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Node SDK compliance v2Commit: f063dce Capture v0 — acceptance✅ 5 passed / ❌ 0 non-passing / Drill-down: run artifacts → Harness exit: 0; report validation exit: 0. All selected cases passed and report validation succeeded. Capture v1 — acceptance✅ 5 passed / ❌ 0 non-passing / Drill-down: run artifacts → Harness exit: 0; report validation exit: 0. All selected cases passed and report validation succeeded. Capture v0 — migration✅ 55 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1 — migration✅ 120 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
Contributor
|
Size Change: +35.9 kB (+0.15%) Total Size: 24.1 MB 📦 View Changed
ℹ️ View Unchanged
|
…trics
`metrics: { autocapture: true }` starts the official OpenTelemetry instrumentations with private meter and tracer providers and exports to `/i/v1/metrics`. `node --import posthog-node/metrics/register` starts it before the app loads any module. The OpenTelemetry packages are optional peer dependencies.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Generated-By: PostHog Desktop
Task-Id: ef9b57c2-47ff-47e6-a509-8873ffe1fab4
DanielVisca
force-pushed
the
posthog/node-metrics-autocapture
branch
from
October 6, 2026 14:41
443c3d9 to
39704b3
Compare
Generated-By: PostHog Desktop Task-Id: c0218209-575c-46f0-a201-d0c831e38096
Generated-By: PostHog Desktop Task-Id: 6921bfc1-7e35-4637-83d4-353c11b3af05
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Comment on lines
+69
to
+71
| "@opentelemetry/sdk-metrics": "^2.0.0", | ||
| "@opentelemetry/sdk-trace-base": "^2.0.0", | ||
| "@opentelemetry/resources": "^2.0.0", |
Contributor
There was a problem hiding this comment.
P1 · Preserve installation compatibility with OpenTelemetry 1.x
Applications with direct OpenTelemetry 1.x dependencies now encounter npm peer-resolution conflicts when upgrading posthog-node, even with autocapture disabled. Marking these peers optional permits their absence but still enforces the ^2.0.0 ranges when installed. This makes the minor release incompatible with existing setups; isolate the version requirements to the opt-in integration.
Comment on lines
+52
to
+55
| distroVersion: version, | ||
| logger: this._logger, | ||
| isEnabled: () => !this.isDisabled && !this.optedOut, | ||
| exportIntervalMs: metrics?.flushIntervalMs, |
Contributor
There was a problem hiding this comment.
P1 · Honor the configured metric filter for autocaptured data
With metrics: { autocapture: true, beforeSend: () => null }, autocaptured metrics still upload. startMetricsAutocapture never receives or applies metrics.beforeSend, so filters that drop metrics or redact database namespaces and server addresses protect only manual samples. Apply the filter to autocapture, or explicitly reject the unsupported combination.
The previous commit reset pnpm-lock.yaml to the base branch, so it no longer matched packages/node/package.json and every `pnpm install --frozen-lockfile` in CI failed with ERR_PNPM_OUTDATED_LOCKFILE. Regenerated with `pnpm install --lockfile-only --config.link-workspace-packages=true` so workspace specifiers keep resolving to the local packages instead of re-resolving freshly bumped PostHog packages from the registry. Co-Authored-By: Claude Opus 5.5 <[email protected]> Generated-By: PostHog Desktop Task-Id: 6921bfc1-7e35-4637-83d4-353c11b3af05
…s/register `beforeExit` does not run when the process is stopped by a signal, so a container stop lost the last metrics window. The register entrypoint now also shuts the client down on SIGTERM and SIGINT (capped at 5s). When the app has no handler of its own, the signal is raised again after the flush, so the process still exits the default way; when it has one, the app keeps owning the exit. Adds a `test:built` check that launches the built entrypoint with `node --import posthog-node/metrics/register` against a fake PostHog host (with a trailing slash), sends SIGTERM, and asserts the export reaches /i/v1/metrics with HTTP server and client metrics in it. Co-Authored-By: Claude Opus 5.5 <[email protected]> Generated-By: PostHog Desktop Task-Id: 6921bfc1-7e35-4637-83d4-353c11b3af05
Contributor
Coherence2 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 5 bullets (5 invariants, 0 requirements), 2 problems Spec problems (spec --check)
Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
… OTel types out of main declarations - A view on db.client.operation.duration keeps only the leading word of db.operation.name. pg derives it from the text before the first space, so a statement like `SELECT\n'[email protected]'` would otherwise export the literal. - optIn() (and enable()) first collects the current window while the exporter still sees the client as opted out, so metrics recorded while opted out are dropped instead of sent after opt-in. - The Node entrypoint types its startMetricsAutocapture override with Required<MetricsAutocaptureConfig>, so the main declarations no longer import the module that references the optional OpenTelemetry packages. Co-Authored-By: Claude Opus 5.5 <[email protected]> Generated-By: PostHog Desktop Task-Id: 6921bfc1-7e35-4637-83d4-353c11b3af05
Comment on lines
+289
to
+292
| new exporter.OTLPMetricExporter({ | ||
| url: `${options.host}/i/v1/metrics`, | ||
| headers: { Authorization: `Bearer ${options.apiKey}` }, | ||
| temporalityPreference: sdkMetrics.AggregationTemporality.DELTA, |
Contributor
There was a problem hiding this comment.
P1 · Exclude unrelated OTLP credentials from PostHog exports
With OTEL_EXPORTER_OTLP_HEADERS=x-api-key=collector-secret configured for another collector, enabling autocapture sends that secret to PostHog. OTLPMetricExporter merges environment headers with these explicit headers even though this code overrides the destination URL. Isolate this exporter from environment credentials intended for other endpoints.
Comment on lines
+372
to
+375
| delete globals[ACTIVE] | ||
| // Export first: disabling removes the runtime gauges' callbacks, so they | ||
| // would be missing from the last window. | ||
| await allSettled([meterProvider.forceFlush()]) |
Contributor
There was a problem hiding this comment.
P2 · Retain singleton ownership until instrumentation teardown finishes
When another client starts while shutdown() awaits its final flush, deleting globals[ACTIVE] lets it reuse the cached instrumentations. The first shutdown then disables those same instances, silently stopping the new client's autocapture. This also happens after a short shutdown timeout; retain singleton ownership until teardown finishes.
| distroVersion: version, | ||
| logger: this._logger, | ||
| isEnabled: () => !this.isDisabled && !this.optedOut, | ||
| exportIntervalMs: metrics?.flushIntervalMs, |
Contributor
There was a problem hiding this comment.
P2 · Apply the configured cardinality guardrail to autocaptured metrics
With metrics: { autocapture: true, maxSeriesPerFlush: 10 }, requests to distinct servers still produce more than ten series. This call never passes the cap to autocapture, which uses OpenTelemetry's default limit per instrument instead of the configured limit per window. The advertised guardrail therefore fails; enforce it across autocaptured series.
Comment on lines
+319
to
+322
| isPostHogRequest( | ||
| request.hostname ?? request.host?.replace(/:\d+$/, ''), | ||
| Number(request.port) || defaultPort(request.protocol ?? undefined) | ||
| ), |
Contributor
There was a problem hiding this comment.
P2 · Resolve the HTTPS transport's default port before excluding requests
For https.request({ hostname: 'us.i.posthog.com', path: '/batch/' }), the hook receives options without protocol or port. defaultPort(undefined) supplies 80, so the comparison against the configured HTTPS host's port 443 fails and records PostHog traffic. Use the request transport's actual default port when applying the exclusion.
| resultCallback({ code: 0 }) | ||
| return | ||
| } | ||
| exporter.export(metrics, resultCallback) |
Contributor
There was a problem hiding this comment.
P2 · Report export failures through the PostHog logger
When the endpoint rejects uploads or the network fails, gateExporter forwards failures only to the metric reader. The reader catches them and logs through OpenTelemetry's diag, which has no logger by default. Autocapture therefore silently loses metrics even with a PostHog logger configured. Log failed export results through options.logger.
Comment on lines
+289
to
+293
| new exporter.OTLPMetricExporter({ | ||
| url: `${options.host}/i/v1/metrics`, | ||
| headers: { Authorization: `Bearer ${options.apiKey}` }, | ||
| temporalityPreference: sdkMetrics.AggregationTemporality.DELTA, | ||
| }), |
Contributor
There was a problem hiding this comment.
P2 · Route autocapture exports through the configured transport
When an application supplies PostHogOptions.fetch to use a proxy or attach gateway authentication, autocapture bypasses it and sends through OpenTelemetry's HTTP transport. Manual metrics use the configured fetch, so enabling autocapture breaks delivery in those deployments. Adapt the exporter to the client's configured transport.
Comment on lines
+933
to
935
| await this._metricsAutocapture.forceFlush().catch(() => {}) | ||
| } | ||
| return super.optIn() |
Contributor
There was a problem hiding this comment.
P1 · Preserve a later opt-out while an opt-in flush is pending
With autocapture enabled and the client opted out, calling optIn() followed by optOut() leaves capture enabled once the flush finishes. The new await forceFlush() lets the later opt-out complete before super.optIn() overwrites its decision, so subsequent events and metrics upload against consent. Serialize consent transitions or invalidate superseded opt-ins.
Comment on lines
+312
to
+316
| this._metricsAutocapture = this.startMetricsAutocapture({ | ||
| http: areas.http ?? true, | ||
| db: areas.db ?? true, | ||
| runtime: areas.runtime ?? true, | ||
| }) |
Contributor
There was a problem hiding this comment.
P2 · Include autocaptured data in the public metrics flush
With metrics.autocapture enabled, await posthog.metrics.flush() only drains PostHogMetrics; it never calls this handle's forceFlush(). A serverless invocation that explicitly flushes metrics leaves its HTTP/DB data buffered until the export timer, losing it when the runtime freezes or exits. Include autocapture in the public metrics flush.
Comment on lines
+2959
to
+2962
| await raceWithTimeout( | ||
| this._metricsAutocapture.shutdown().catch(() => {}), | ||
| Math.max(0, shutdownDeadlineMs - Date.now()) | ||
| ) |
Contributor
There was a problem hiding this comment.
P2 · Drain events concurrently with autocapture shutdown
When the OTLP endpoint stalls longer than shutdownTimeoutMs, this awaited autocapture shutdown consumes the entire shared deadline before super._shutdown begins draining events. The event drain receives zero milliseconds, so exiting after await posthog.shutdown() discards queued events even when event ingestion is healthy. Drain events concurrently with autocapture teardown.
Comment on lines
+404
to
+408
| await allSettled([meterProvider.forceFlush()]) | ||
| for (const instrumentation of instrumentations) { | ||
| instrumentation.disable() | ||
| } | ||
| await allSettled([meterProvider.shutdown(), tracerProvider.shutdown()]) |
Contributor
There was a problem hiding this comment.
P2 · Cancel autocapture exports when the shutdown deadline expires
When the metrics endpoint stalls, posthog.shutdown(500) returns after its timeout but this final forceFlush() continues with the exporter's default 10-second timeout. Instrumentations remain enabled and the referenced HTTP socket keeps a draining process alive beyond its shutdown budget. Pass the remaining deadline into teardown and abort outstanding exports when it expires.
Comment on lines
+405
to
+406
| for (const instrumentation of instrumentations) { | ||
| instrumentation.disable() |
Contributor
There was a problem hiding this comment.
P2 · Preserve application-owned instrumentation during shutdown
When an app enables its own HTTP instrumentation after autocapture but before loading http, its wrappers replace PostHog's. Calling posthog.shutdown() then invokes disable(), whose OpenTelemetry unpatch callback removes those application-owned wrappers too. Subsequent requests lose the app's tracing; check wrapper ownership before unpatching.
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Metrics needs hand-written
posthog.metrics.*calls today. We want metrics to be the easiest setup: install the SDK, set one flag, and get HTTP, database and runtime metrics.Why: posthog-python#967 was closed because a hand-rolled HTTP wrapper duplicates OpenTelemetry instrumentation, and PostHog accepts OTLP directly. So this change starts the official OpenTelemetry instrumentations for you instead of writing new ones.
Changes
metrics: { autocapture: true }(or{ http, db, runtime }toggles). Off by default.node --import posthog-node/metrics/register app.js(readsPOSTHOG_PROJECT_TOKEN,POSTHOG_HOST,OTEL_SERVICE_NAME). It starts before the app loads modules, and registers the ESM loader hook, so ESM apps are measured too.db.client.operation.duration. pg and oracledb record it natively and are not counted twice. Only bounded attributes; never the statement.telemetry.distro.name=posthog-nodeso we can measure adoption.Known gap:
http.server.request.durationhas nohttp.routeyet (that needs a global context manager, which could clash with an app's own OpenTelemetry setup).How I tested it
src/__tests__/metrics-autocapture.spec.ts(16 tests). Fullposthog-nodesuite: 1117 passed.check-types,lint,formatclean.hogliwith themetricsintent): an ESM Express + pg + ioredis app started with--import posthog-node/metrics/register. In ClickHouse I sawhttp.server.request.duration,http.client.request.duration(fetch),db.client.operation.duration(pg native + ioredis derived),db.client.connection.*and 15 runtime metrics (nodejs.eventloop.*,v8js.*). No series for the PostHog host.requireParentSpan: false, the runtime gauges were lost on shutdown). Each now has a test.Release info Sub-libraries affected
Libraries affected
Checklist
metrics.autocapturewarns there)If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Created with PostHog Desktop
🤖 Generated with Claude Code