Skip to content

feat(node): link captured exceptions to active OpenTelemetry spans - #5289

Merged
pauldambra merged 5 commits into
mainfrom
posthog/node-exception-trace-context
Oct 14, 2026
Merged

pauldambra merged 5 commits into
mainfrom
posthog/node-exception-trace-context

Conversation

@pauldambra

@pauldambra pauldambra commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

[Robot / PostHog Slack app] This draft PR was prepared by an automated agent.

Problem

An exception captured by posthog-node has no trace ID, even when the application has an active OpenTelemetry span. This makes it hard to move from the error to its trace and related logs. The contract is being defined in sdk-specs#123.

Changes

  • Read the optional OpenTelemetry API when Node prepares an exception event, after merging request context and event properties. Add valid trace and span IDs without changing caller supplied IDs or adding a required runtime dependency.
  • Cover sampled and unsampled spans, invalid IDs, absent context, caller values, and a broken tracing API with tests.

This covers all posthog-node $exception events, including the deprecated Sentry integration. It reads ambient OpenTelemetry spans, not spans made by posthog-node's own tracing API.

The optional OpenTelemetry import promise is cached for the life of the process. A local Node microbenchmark measured about 18 ms for the first lookup and 0.2 microseconds for a cached lookup; this is not a production latency measurement. The feature flag and Sentry exception tests now await flush() because the new lookup adds an async step, so fixed counts of promise turns no longer prove capture completed.

Release info Sub-libraries affected

Libraries affected

  • posthog-node

Checklist

  • Tests for new code
  • Accounted for the impact across Node and edge entrypoints
  • Kept existing caller property behavior
  • Kept OpenTelemetry optional

If releasing new changes

  • Added a posthog-node patch changeset

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

PostHog Slack app used the existing event preparation path and the pending SDK contract. The implementation caches the optional API lookup and reads the active span after caller and request properties are merged. Local unit, edge, type, lint, format, build, and frozen lockfile checks passed. A real SDK acceptance adapter has not run the new spec scenarios.

Refs PostHog/sdk-specs#122


Created with PostHog from a Slack thread

Generated-By: PostHog Desktop
Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
@pauldambra pauldambra self-assigned this Oct 13, 2026

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 5f4812e: 1 Should fix.

@github-actions

Copy link
Copy Markdown
Contributor

Coherence

2 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)

  • Posthog-js.practice.md:20 practice add an invariant cites 187a5c5, which is no commit in this repository
  • Posthog-js.practice.md:20 practice add an invariant cites 8922ab2, which is no commit in this repository

Chokepoints: none declared.

Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline

  • packages/web/Web.spec.md entrance public api (trust host-app): packages/web
  • packages/webpack-plugin/Webpack-plugin.spec.md entrance webpack plugin (trust build): packages/webpack-plugin

Close one with guard: <chokepoint>, an invariant that names it on its entrances: line, or control: none — <reason>; scaffold control proposes it.

Generated-By: PostHog Desktop
Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed c55518d: 1 Must fix.

Generated-By: PostHog Desktop
Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Node SDK compliance v2

Commit: 6d25844
Workflow run and artifacts

Capture v0 — acceptance

✅ 5 passed / ❌ 0 non-passing / ⚠️ 812 not selected

Drill-down: run artifacts → node-v2-v0 → acceptance/report.json.diagnostics.json (match case_id below).

Harness exit: 0; report validation exit: 0.

All selected cases passed and report validation succeeded.

Capture v1 — acceptance

✅ 5 passed / ❌ 0 non-passing / ⚠️ 812 not selected

Drill-down: run artifacts → node-v2-v1 → acceptance/report.json.diagnostics.json (match case_id below).

Harness exit: 0; report validation exit: 0.

All selected cases passed and report validation succeeded.

Capture v0 — migration

✅ 55 passed / ❌ 1 non-passing / ⚠️ 101 not selected

Drill-down: run artifacts → node-v2-v0 → reports/report.json.diagnostics.json (match case_id below).

❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertion

Flags field differs

Code: flag_request_field

Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134

the first flags request field "geoip_disable" should equal JSON false

Operation: /flags/

Field: geoip_disable

Expected:

false

Actual value:

true

Harness exit: 1; report validation exit: 1.

Capture v1 — migration

✅ 120 passed / ❌ 1 non-passing / ⚠️ 36 not selected

Drill-down: run artifacts → node-v2-v1 → reports/report.json.diagnostics.json (match case_id below).

❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertion

Flags field differs

Code: flag_request_field

Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134

the first flags request field "geoip_disable" should equal JSON false

Operation: /flags/

Field: geoip_disable

Expected:

false

Actual value:

true

Harness exit: 1; report validation exit: 1.

@pauldambra
pauldambra marked this pull request as ready for review October 13, 2026 16:24
@pauldambra
pauldambra requested a review from a team as a code owner October 13, 2026 16:24
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T16:28:46.591713Z a3b0368 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

pauldambra commented Oct 13, 2026 •

Copy link
Copy Markdown
Member Author

Note

🤖 Automated comment by QA Swarm — not written by a human

Review: router pass and focused re-review of the repair.

Verdict: ✅ APPROVE (round 2 @ cd366fa)

The Webpack issue found after round 1 is fixed. A peer-free Webpack build failed before the repair and passed with the emitted helper after it. The focused trace test, package build, and edge suite pass.

Key findings

No open actionable issue.

Convergence

The bot review and a peer-free Webpack build found the same optional import problem; cd366fa fixes it.

Reviewer summaries

Reviewer Assessment
Router Re-reviewed cd366fa; no new defect found.
Previous rounds (1)

round 1 @ a3b0368 — approve on code review; a later bot review found the optional import bundle problem, now fixed.


Automated by QA Swarm — not a human review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3b0368c71

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Generated-By: PostHog Desktop
Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
@pauldambra

Copy link
Copy Markdown
Member Author

@PostHog review --deep

@posthog

posthog Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@pauldambra PostHog Review is reviewing this pull request.

@posthog

posthog Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

PostHog Review · Deep · reviewed cd366fa

Posted 1 finding: 1 Must fix.

Step Status Result
Prepare the diff done 4 files, 1 chunk
Pick perspectives done 5 perspectives
Run review passes done 6 issues (+1 blind-spot)
Merge overlapping findings done 7 → 4
Validate findings done 1 kept, 3 dismissed
Publish done 1 posted (view review)
Resolve comments done 1 left for you

Comment thread packages/node/package.json Resolved
Generated-By: PostHog Desktop
Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
@pauldambra
pauldambra added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit e6598be Oct 14, 2026
72 checks passed
@pauldambra
pauldambra deleted the posthog/node-exception-trace-context branch October 14, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants