Repository navigation
feat(node): add logs API (posthog.logger, captureLog) - #5599
DanielVisca wants to merge 8 commits into
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 04d5d2c: 5 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
Coherence📄 0 spec file errors · ⛔ 0 broken rules · 🚪 0 unchecked inputs · 🐛 0 bugs that came back What is Coherence?Coherence compares the code with its spec files ( This check read 44 parts of the code and 7 rules. Tip Nothing to fix. |
Node SDK acceptance v2Commit: fc687b3 Capture v0✅ 83 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ 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✅ 148 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ 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. |
SDK performance: posthog-node1 change worth reviewing: 1 observed regression. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate.
View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
- schedule the serverless waitUntil flush when a log is captured - skip throttled logs on automatic event flushes, like spans - retire a pending background log flush on shutdown - fall back to the default batch size for non-integer or non-positive values - treat a non-positive rate cap or window as disabled, per the Logs spec Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
Size Change: +20.1 kB (+0.08%) Total Size: 26.6 MB 📦 View Changed
ℹ️ View Unchanged
|
…concurrently on shutdown - expose PostHogLogs.flushInBackground(), a non-rejecting flush that re-arms the timer with backoff when records remain, and use it from the node unified flush instead of a bare flush() - start the logs drain at the top of shutdown and await it next to the core flush, so a stalled logs endpoint can't spend the traces and events budget Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
PostHog Review · Standard · reviewed ce82407: 1 Should fix.
This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
|
Prerequisites for this PR are live in production:
The check results are in each PR's comments. |
…flush wrapper - PostHogLogs discards its queue when consent is withdrawn, before a flush and between batches, so a pending timer or an in-progress drain cannot export records that carry the distinct id and session id - flushInBackground() returns one shared promise per in-flight flush, so overlapping callers log and count a failed POST once Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 194ee18: 1 Should fix.
This is review round 4 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
…ed names - check consent at the top of each loop iteration, so a 413 shrink-and-retry after an opt-out does not export the queue - add the two new private names to terser-mangled-names.json Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
posthog-js Compliance ReportDate: 2026-11-09 18:57:31 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
…own still flushes The browser host's optedOut also reads true while the client shuts down, so dropping the queue on it discarded the final flush. PostHogLogs now takes an isConsentWithdrawn callback, which only the Node client supplies. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Stacked on #5594 (uses the new
span.traceId/span.spanId); the diff against main includes that PR until it merges.Adds a logs API to
posthog-node:posthog.logger.{trace,debug,info,warn,error,fatal}(body, attributes?)andposthog.captureLog({ body, level, attributes }), configured with alogsoption likemetrics/traces. It reuses@posthog/core'sPostHogLogs(the node client already satisfies itsLogsHost), so batching, retry, rate cap,beforeSendand OTLP encoding are shared with the browser and React Native SDKs. Each record gets the active span'strace_id/span_id/ flags and thewithContextdistinctId and sessionId; explicit ids win.serviceNamefalls back to the traces / metrics one. Logs flush withflush()and onshutdown().Server defaults (in
packages/node/src/logs-config.ts): 3s flush, 100-record buffer, 100 records per POST, 10000 records per 10s rate cap. Say if you want different numbers.Why: feedback item SDK-001 in https://github.com/PostHog/dash/blob/main/docs/POSTHOG_SDK_FEEDBACK.md
🤖 Generated with Claude Code