Repository navigation
docs(capture): specify the client-level GeoIP option and per-call override - #131
Merged
Merged
Conversation
turnipdabeets
force-pushed
the
posthog/client-level-geoip-option
branch
from
October 20, 2026 18:01
dcb74f9 to
1713595
Compare
turnipdabeets
marked this pull request as ready for review
October 20, 2026 19:47
…rride The specs listed a per-call disable_geoip argument on capture, identify, alias, group_identify, and the flag APIs, but never said what an omitted value means or that server SDKs carry a client-level setting behind it. SDKs diverged as a result. Adds a capture requirement covering the client-level option and its default, per-call inheritance (explicit boolean wins, null inherits), $geoip_disable marker placement, and the geoip_disable field on remote flag-evaluation requests. Co-Authored-By: Claude Opus 5 <[email protected]> Generated-By: PostHog Desktop Task-Id: 840fad87-610e-4e40-8719-c99b2c5616d2
posthog-php and posthog-rs ship with GeoIP enrichment enabled by default, so requiring a disabled default would force a breaking change. The default is now recommended rather than required; inheritance of an unset per-call value, explicit per-call overrides, and keeping $geoip_disable out of $set/$set_once/$group_set stay SHALL. posthog-go's per-message DisableGeoIP is a plain bool, so it conforms by keeping the client setting authoritative until a major release can tell unset from false. The explicit-override scenario is tagged @geoip_per_call_unset_capable. The proposal and design now list Rust and drop the "majority" framing. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…st and .NET Ruby and Elixir only send geoip_disable on flags requests; events never carry $geoip_disable. Java has no GeoIP option. Rust and .NET keep a caller-supplied $geoip_disable over the client setting, which the requirement forbids. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
turnipdabeets
force-pushed
the
posthog/client-level-geoip-option
branch
from
October 20, 2026 21:20
1713595 to
b35ba85
Compare
turnipdabeets
changed the base branch from
docs/acceptance-step-catalogue
to
main
October 20, 2026 21:20
marandaneto
approved these changes
Oct 21, 2026
marandaneto
left a comment
marandaneto
left a comment
Member
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
# Conflicts: # openspec/specs/capture/spec.md
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.
Why
The specs describe GeoIP control only as a per-call argument —
capture,identify,alias,group-identifyand the flag specs all listdisable_geoip/disableGeoip, and no requirement says what an omitted value means or that server SDKs carry a client-level setting behind it. SDKs resolved that gap differently.posthog-php#263 (merged 2026-10-16) added a client-level
disableGeoipoption with per-call overrides, and had to derive its inheritance rules and default by reading Node, Python and Go, because the contract does not state them. Its description notes the spec only defines a per-call option.Changes
Adds one
capturerequirement, "Client-level GeoIP control with per-call override":false.$geoip_disable: truesits among the event's ordinary properties, never inside$set/$set_once/$group_set, is stamped beforebefore_send, and is absent when enrichment stays on. A caller-supplied$geoip_disableis left alone.geoip_disablefield.Proposed, applied and archived on this branch as
2026-10-18-add-client-geoip-configuration.Verified SDK behavior
disableGeoip?? client setting)disable_geoipNoneinherits)Config.DisableGeoIPEnqueueoverwrites the per-message field from configdisableGeoip(new)Uncertain — worth a human call
falsedeliberately for release compatibility, and .NET's event and flag paths default differently. Confirm whether PHP's default is a sanctioned temporary divergence with a target major, and which of .NET's two defaults is intended.How did you test this code?
openspec validate --specs --strictreports the same 33 passed / 31 failed totals as the base branch (all pre-existing "requirement text is very long" warnings; the new requirement raises the same warning as its neighbours).Created with PostHog Desktop
🤖 Generated with Claude Code