feat(browser): add os.name and os.version to logs, and share one OTLP resource builder - #4710
Merged
Merged
Conversation
23 tasks
Contributor
posthog-node Compliance ReportDate: 2026-08-31 21:48:42 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
Contributor
posthog-js Compliance ReportDate: 2026-08-31 21:45:30 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
Contributor
|
Size Change: +7.71 kB (+0.04%) Total Size: 20.8 MB 📦 View Changed
ℹ️ View Unchanged
|
turnipdabeets
changed the title
turnipdabeets
marked this pull request as ready for review
August 31, 2026 21:33
Contributor
Prompt To Fix All With AI### Issue 1
packages/core/src/utils/otlp-resource.ts:65
**Inconsistent absence sentinel**
`normalizeOsName` and `osResourceAttributes` use `undefined` to represent an intentionally unavailable OS value, contrary to the repository convention requiring `null`; this establishes an inconsistent API contract and forces callers and tests to handle the wrong sentinel.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(core): guard normalizeOsName against..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… resource builder Logs from posthog-js now carry the visitor's OS, normalized to the names the native SDKs send. buildResourceAttributes and buildMetricsResourceAttributes delegate to one shared builder.
turnipdabeets
force-pushed
the
feat/otlp-shared-resource-builder
branch
from
August 31, 2026 21:41
22de641 to
4519225
Compare
marandaneto
approved these changes
Sep 1, 2026
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
2 participants
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
Logs from the web SDK don't record what OS the visitor was on. You can't answer "show me errors on Windows only" — the OS is simply absent from every logs payload
posthog-jsproduces.Logs are sent as OTLP, and every batch carries a set of resource attributes: key/value pairs describing where the logs came from (service name, SDK version, and so on). The logs spec lists
os.nameandos.versionamong the ones the envelope SHALL carry.posthog-react-native,posthog-iosandposthog-androidall send them;posthog-jsnever has.Separately, the code that builds those resource attributes existed twice.
buildResourceAttributes(logs) andbuildMetricsResourceAttributes(metrics) had byte-identical bodies; only the return-type spelling differed.Changes
One builder.
packages/core/src/utils/otlp-resource.tsholdsbuildOtlpResourceAttributes, generic over each signal's attribute value type. Both existing functions keep their exported names, parameter lists and return types and now delegate to it —buildResourceAttributesis a public@posthog/coreexport with a shipped call site inpackages/browser, so nothing about its surface moves.The OS attributes on
posthog-jslogs, fromdetectOS(navigator.userAgent), merged before userresourceAttributesso a user-supplied value still wins. Either key is omitted rather than emitted empty when it can't be determined, and the path cannot throw out of client construction.os.nameis normalized to one cross-SDK vocabulary. This is the part worth a look. The raw detected spellings disagree across SDKs for the same OS:detectOSMac OS XmacOSmacosiOSiOSiosAndroidAndroidandroidWindowswindowsdetectOSalready agrees with the native SDKs on iOS, Android and watchOS — macOS was the only real disagreement, sonormalizeOsNamemapsMac OS X→macOS. OpenTelemetry definesos.nameas the human-readable name, which is what iOS and Android already send; the lowercase identifiers belong toos.type, a different attribute.The table also covers
node:osplatform()identifiers (darwin,win32, …) soposthog-nodecan reuseosResourceAttributeswithout inventing a third spelling. That consumer is #4679 —feat(node): emit os.name and os.version on spans— which is the PR this one was split out of, and it is why thenode:oskeys are in the table now rather than bolted on later. Node reaches them throughosResourceAttributesalone, which is why that is the only symbol exported. React Native is deliberately untouched — its lowercase values are already shipped, so realigning them is a data-migration conversation and shouldn't block this.Reviewer notes
null,undefined, booleans and numbers) x 6 values for each ofserviceName/environment/serviceVersionx 2 SDK name/version pairs — assertingtoEqual,Object.keysorder andJSON.stringifyequality. Identical every time. Key order matters becausetoOtlpKeyValueListiterates the object.@posthog/coreexport, tagged@internal.osResourceAttributesis added to core's entry point becausepackages/browsercan only reach core through its published entry points. It appears indist/index.d.tsand carries@internal Exposed for cross-package use within this SDK; not part of the stable public API.— the same idiomcreateNamedErroralready uses.buildOtlpResourceAttributesandnormalizeOsNamestay module-internal and are absent fromdist/index.d.ts. Noposthog-jspublic API changes.Verification
packages/core: 56 suites, 1111 pass.packages/browser:posthog-logs157 pass (was 156 — one added here assertsos.*reaches the emitted payload),logs-defaults17 pass.eslintclean on all changed files.tsc --noEmiton core reports 37 errors both with and without this change — all pre-existing in unrelated__tests__files, none naming a file touched here.Release info Sub-libraries affected
Libraries affected
@posthog/coreis also bumped (patch); it has no checkbox above.Checklist
Two new resource attributes appear on logs batches from the web SDK — new behavior on an already-shipped payload, hence the minor bump.
os.nameis emitted whenever the user agent can be placed;os.versionis absent where the user agent does not carry one (Android and desktop Linux). Both keys are overridable and both are omitted when undetectable. Nothing is removed or renamed.If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, directed by @turnipdabeets.
Split out of #4679 during a pre-PR review pass: that PR sat three deep in a stack behind the traces MVP, but only ~10 of its lines actually needed traces. The
os.namevocabulary divergence was found by comparing the emitted values againstposthog-ios,posthog-androidandposthog-react-nativesource — the original change would have shipped a fourth spelling for macOS.A later review pass added the two follow-up commits:
normalizeOsNamewas un-exported after a rebuild showed it reaching the publisheddist/index.d.tswith no cross-package caller, and the changeset was corrected — it had claimed oneos.namefilter matches every SDK, whichposthog-react-native's already-shipped lowercaseios/androidfalsifies. That parity gap is left for a separate spec proposal, since realigning it changes a shipped attribute value.