Skip to content

feat(logs): honor configured session id keys in session detection - #70736

Merged
jonmcwest merged 10 commits into
masterfrom
posthog-code/logs-session-id-detection-wiring
Jul 15, 2026
Merged

feat(logs): honor configured session id keys in session detection#70736
jonmcwest merged 10 commits into
masterfrom
posthog-code/logs-session-id-detection-wiring

Conversation

@jonmcwest

Copy link
Copy Markdown
Contributor

Problem

The logs_session_id_attribute_keys team setting shipped recently (#70414), but nothing consumes it yet: logs session-ID detection still only knows the hardcoded convention list in products/logs/frontend/utils.tsx. A team whose pipeline emits the session ID under a custom attribute key gets no replay links, no related-errors matching, and no "view session logs" pivot, even after configuring their keys.

Second PR in the logs-in-error-tracking series - the error tracking "view logs" surface will build on the same configured keys.

Changes

Detection now checks the configured keys first, then falls back to the built-in list, so teams that never touch the setting see no change:

  • isSessionIdKey, getSessionIdWithKey, getSessionIdFromLogAttributes accept an optional configuredKeys list. Semantics, now locked in by tests: configured keys are checked in list order, exact match only (no dot-suffix matching, unlike the built-ins), attributes before resource_attributes per key, first value found wins.
  • All five consumers pass the setting through from logsConfigLogic:
    • row-hover "view recording" button (LogRowFAB)
    • log drawer replay button and related-errors tab (logDetailsModalLogic now connects logsConfigLogic, so the sessionId selector recomputes when config loads)
    • attribute cell and attributes-table replay affordances (AttributeCell, LogAttributes)
    • "view session logs" context pivot (logContextUtils + LogContextSelector) - the session filter it builds uses the matched configured key, so the pivot works for custom keys too

While config is loading the functions fall back to the built-in list, which contains the default posthogSessionId - no flash of missing buttons for default configs.

How did you test this code?

  • Extended products/logs/frontend/utils.test.ts with a configured-keys block (92 tests pass across the two affected suites). Each case pins a contract no existing test covered: configured key beats a built-in key present on the same log (the priority rule), list order wins between two configured keys (a Set/object-iteration refactor would silently break ordering), exact-match-only for configured keys (merging them into the suffix-matching helper would silently widen matching), and fallback to built-ins when configured keys are absent (an "exclusive mode" refactor would break every default team).
  • No component/logic tests: the consumer changes are argument pass-through with no branching.
  • Lint, format, and typecheck clean on all changed files.
  • Not manually verified in a running app - the visible behavior for default configs is intentionally unchanged; a custom-key end-to-end check needs logs ingested with a custom session attribute.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

The setting itself is behind the LOGS_SETTINGS flag and its docs will land with the user-facing error-tracking surface later in the series.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with PostHog Code (Claude). Skills invoked: /writing-tests.

Decisions:

  • Optional trailing parameter on the pure functions rather than reading kea state inside utils - keeps them pure/testable, and the kea logic consumer (logDetailsModalLogic) gets proper reactivity by connecting logsConfigLogic into its selector dependencies instead.
  • Exact match for configured keys, suffix matching reserved for the built-in list: a user configuring custom.key shouldn't silently match anything.custom.key; if they want the suffixed form they can add it as another key.
  • Per-key "attributes then resource_attributes" ordering (key order dominates), matching the setting's documented "first key with a value wins" contract.

Created with PostHog Code

Session-ID detection in logs (replay links, related errors, session
context pivots) now checks the team's logs_session_id_attribute_keys
setting first — in list order, exact match, attributes before
resource_attributes — and falls back to the built-in convention list,
so teams that never touch the setting see no change.

Generated-By: PostHog Code
Task-Id: 964398f1-fa06-46f5-8605-2236fa2a3c32
@jonmcwest jonmcwest self-assigned this Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +3.5 KiB (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 64.76 MiB · 🔺 +3.5 KiB (+0.0%)

File Size Δ vs base
posthog-app/src/scenes/AuthenticatedShell.js 222.4 KiB 🔺 +1.3 KiB (+0.6%)

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.22 MiB · 22 files 🔺 +316 B (+0.0%) ███░░░░░░░ 28.4% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.14 MiB · 2,978 files 🔺 +2.6 KiB (+0.0%) █████████░ 88.0% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
280.3 KiB ../node_modules/.pnpm/posthog-js@1.400.1/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
235.5 KiB src/taxonomy/core-filter-definitions-by-group.json
222.7 KiB ../node_modules/.pnpm/posthog-js@1.400.1/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
154.3 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB ../packages/quill/packages/quill/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

Dist folder size — 🟢 -304.2 KiB (-0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1307.77 MiB · 🟢 -304.2 KiB (-0.0%)

@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Polish pass on the detection wiring: configuredSessionIdKeys selector on
logsConfigLogic replaces repeated inline reads, unified param naming,
plain-object connect, and a test pinning that the session pivot filter
uses the matched configured key.

Generated-By: PostHog Code
Task-Id: 964398f1-fa06-46f5-8605-2236fa2a3c32
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 446e3c0 · box box-0f77fc3c5686 · ready in 707s (push → usable) · build log · rebuilds on every push, torn down on close

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 14, 2026 14:41
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "refactor(logs): centralize configured se..." | Re-trigger Greptile

Comment thread products/logs/frontend/components/LogsViewer/LogAttributes.tsx Outdated
Match AttributeCell's `&& value` guard in the LogAttributes value cell so the
ViewRecordingButton is only rendered for a non-empty configured session id key,
even if the empty-value early return is ever refactored away.

Generated-By: PostHog Code
Task-Id: f4ae2e8b-13ad-4671-b6df-6f375b4fc516
@github-actions
github-actions Bot requested a deployment to preview-pr-70736 July 14, 2026 14:58 In progress
Generated-By: PostHog Code
Task-Id: f4ae2e8b-13ad-4671-b6df-6f375b4fc516
Generated-By: PostHog Code
Task-Id: ce4fe931-775c-45cc-9a77-3e6a06882673
Generated-By: PostHog Code
Task-Id: 00b6a2ea-643f-4e76-ad7b-44efedc6e023
Generated-By: PostHog Code
Task-Id: 45ceaeac-8adf-4815-8ba6-e76a75db1cb6
@posthog

posthog Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

👋 Visual changes detected for this PR.

Review and approve in PostHog Visual Review

If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix.

Copy link
Copy Markdown
Contributor Author

Status update

The branch is now merged up to date with master (clean two-parent merge, no unrelated files in the PR diff — it still touches only products/logs/frontend/), and every functional CI check is green.

The only red checks are the visual-review trio (PostHog Visual Review / storybook, Complete Visual Review run, Visual regression tests pass). Those are not caused by this PR. All 6 changed snapshots are AI-observability stories:

  • scenes-app-ai-observability-conversation-display--{basic,markdown,tools}--{light,dark}

This logs PR doesn't touch AI observability. The diffs come from 84e0d350 (fix(aio): render activity AI generation input correctly, #66469), already merged to master, which changed ConversationDisplay.tsx/.stories.tsx but did not update the committed frontend/snapshots.yml baseline. So current master renders the new component against a stale baseline, and any branch built on it inherits these 6 phantom changes.

I'm deliberately not finalizing/approving them here: doing so would commit an unrelated AIO baseline into frontend/snapshots.yml under this logs PR, which would pollute the diff and ship a visual change this PR has no business owning. The baseline needs refreshing on master by the AIO owners (or on a run that legitimately owns those stories); once that lands, a re-run here will show 0 visual changes.

Remaining to merge: a human review approval (reviewDecision: REVIEW_REQUIRED) and the visual-review baseline situation above — both need a maintainer.

🦉 via talyn.dev

Generated-By: PostHog Code
Task-Id: 00b6a2ea-643f-4e76-ad7b-44efedc6e023

Copy link
Copy Markdown
Contributor Author

Status: green except one unrelated visual-review gate

Review threads: the one review thread (the record.value guard on the replay affordance) is addressed and resolved.

Merge state: branch is up to date with master (merged in server-side, no conflicts) and mergeable. It still shows BLOCKED because a human review is required (REVIEW_REQUIRED) and because of the visual-review gate below.

CI: every functional check is green — Jest (FOSS + EE), frontend typecheck, backend, Rust, Node, and all 16 Storybook chromium shards pass. The only red is the PostHog Visual Review gate ("6 changed"), and those 6 changed snapshots are not from this PR:

scenes-app-ai-observability-conversation-display--basic--{dark,light}
scenes-app-ai-observability-conversation-display--markdown--{dark,light}
scenes-app-ai-observability-conversation-display--tools--{dark,light}

All six are AI-observability ConversationDisplay stories. This PR only touches products/logs/frontend/, and the Logs scene story is test-skip, so it contributes zero snapshots — no logs snapshot changed. The AIO drift comes from master #68820, which changed ConversationDisplay.tsx and its story but did not regenerate frontend/snapshots.yml, so the committed baseline is stale for those 6 stories. It reproduces on a clean re-capture (not flaky) and is present on the latest master, so it affects any branch built on it — not something this logs PR introduced or can fix.

I deliberately did not resolve it by finalizing the baseline here: that would commit an unrelated AIO snapshots.yml change into a logs-only PR, and the diffs belong to a component I can't visually verify.

To unblock: a reviewer can approve those 6 AIO diffs in the visual review run during review (or the baseline gets regenerated on master), plus the usual review approval.

🦉 via talyn.dev

Copy link
Copy Markdown
Contributor Author

CI status: all green except Visual Review — root cause is a master-side baseline drift, not this PR

Current state of this branch (fe78e37):

  • ✅ All review threads resolved.
  • ✅ Mergeable — no conflicts with master (recently synced).
  • ✅ Every CI check passing except two: Complete Visual Review run and Visual regression tests pass.

The Visual Review failure is not caused by this PR. The run flags exactly 6 changed snapshots, and all 6 are scenes-app-ai-observability-conversation-display--{basic,markdown,tools}--{light,dark} — AI-observability stories. This PR only touches products/logs/frontend/, so it renders none of them.

It's a stale baseline on master: the ConversationDisplay rendering changed in #69090, but the approved baseline in frontend/snapshots.yml was never updated to match. The baseline hashes for those 6 entries are byte-identical on this branch and on the latest master, so merging master in doesn't clear it — every gated PR that syncs with master inherits the same 6 "changed" snapshots.

I can't resolve this from within this PR:

  • The baseline hash is an HMAC-signed token produced only by the Visual Review backend on approval, so frontend/snapshots.yml can't be hand-edited.
  • Approving the run needs the visual_review:write scope / the Approve all action in the Visual Review UI, which this automation doesn't have.

To green the check, one of:

  1. A maintainer opens the Visual Review run and clicks Approve all — the 6 diffs are the expected fix(llma): highlight correct span in sidebar when deep-linking via event #69090 rendering, not regressions; or
  2. The AI-observability baseline for those conversation-display stories is regenerated on master.

Once master's baseline is corrected, this PR needs no further changes to pass. The PR also still needs a human review approval (REVIEW_REQUIRED) before it can merge.

🦉 via talyn.dev

Generated-By: PostHog Code
Task-Id: 7fecffbc-6444-4e4d-b958-937551407cb6
Generated-By: PostHog Code
Task-Id: 96e692ef-17e6-4cb8-b58b-0c0d704659f8

Copy link
Copy Markdown
Contributor Author

CI fully green — only a human review approval remains

Latest commit (446e3c06, master merged in server-side as a clean two-parent merge):

  • All review threads resolved — the one thread (the record.value guard on the replay affordance) is addressed and marked resolved.
  • CI fully green — every check is SUCCESS or SKIPPED (0 failing, 0 pending). The Visual Review gate that earlier status updates flagged now passes ("No visual changes") — the AI-observability conversation-display baseline drift was corrected on master, so it no longer affects branches synced with it. Nothing left to do there.
  • Mergeable — no conflicts, master is an ancestor, branch is not behind, and the PR diff is unchanged (still only products/logs/frontend/ — no base-branch files leaked in from the merge).

The PR still shows BLOCKED solely because of REVIEW_REQUIRED: it needs a maintainer's review approval before it can merge. That's the only outstanding item and it needs a human.

🦉 via talyn.dev

Copy link
Copy Markdown
Contributor Author

This PR is in a clean, mergeable state:

  • CI: all checks green on the latest commit. The earlier failures were both unrelated to this change — the Visual Review flagged 6 ai-observability-conversation-display snapshots (baseline drift on master, cleared once the branch was brought up to date), and semgrep-general was cancelled by an infra timeout (passed on re-run; all other semgrep jobs were already green).
  • Review threads: all resolved.
  • Mergeability: MERGEABLE, no conflicts, up to date with master.

The only thing left is a required human review approval — nothing further to fix on my end.

🦉 via talyn.dev

@jonmcwest jonmcwest added the stamphog Request AI approval (no full review) label Jul 15, 2026

@stamphog stamphog 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.

Contained frontend feature (argument pass-through + fallback logic) by an owning-team author with strong recent familiarity; the one substantive review comment (empty session id button) was fixed and resolved, with new parameterized tests covering the added behavior.

  • Author wrote 96% of the modified lines and has 230 merged PRs in these paths (familiarity STRONG).
  • 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 92L, 8F substantive, 157L/10F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1c-medium (157L, 10F, single-area, feat)
stamphog 2.0.0b3 .stamphog/policy.yml @ 46b0dba · reviewed head 446e3c0

@jonmcwest
jonmcwest merged commit 72841dc into master Jul 15, 2026
281 checks passed
@jonmcwest
jonmcwest deleted the posthog-code/logs-session-id-detection-wiring branch July 15, 2026 13:06
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-15 13:34 UTC Run
prod-us ✅ Deployed 2026-07-15 14:01 UTC Run
prod-eu ✅ Deployed 2026-07-15 14:01 UTC Run

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

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant