Skip to content

feat(groups): add disableGeoip option to groupIdentify - #263

Merged
marandaneto merged 3 commits into
mainfrom
xljones/group-identify-disable-geoip
Oct 16, 2026
Merged

marandaneto merged 3 commits into
mainfrom
xljones/group-identify-disable-geoip

Conversation

@xljones

@xljones xljones commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

  • PHP users cannot turn off GeoIP enrichment on $groupidentify events, so backend group identify calls pick up the server's location.
  • groupIdentify() has no option to set $geoip_disable. capture(), identify(), and alias() accept it in properties.
  • A customer reported this through support.

Changes:

  • Add a client-wide disableGeoip option and per-call overrides for capture(), identify(), alias(), and groupIdentify(). When enabled, the SDK sets properties.$geoip_disable: true, alongside $group_set rather than inside it.
  • Remote feature flag evaluation inherits the client setting unless explicitly overridden. Snapshot exposure events retain that override, including filtered snapshots and minimized exposure events.
  • Explicit false overrides a client setting of true; omitted or null options inherit the client setting. Setting the option to false does not remove manually supplied $geoip_disable properties. before_send can still modify the enriched event.
  • before_send can already set the flag, but the group-identify spec defines a per-call disableGeoip, and the Node, Python, and Go SDKs have one.
  • Deliberate default difference from Node: disableGeoip defaults to false for compatibility with current PHP releases. Ideally it will become true in the next major release; the initialization code documents this decision. Set disableGeoip => true to opt into Node's default behavior now.
  • The option follows the published SDK contracts; existing defaults are preserved. See UPGRADING.md for configuration and compatibility notes.

💚 How did you test it?

  • GeoipTest covers client defaults, per-call precedence, omitted/null inheritance, all four event APIs, remote flag requests, filtered snapshot exposure events, minimized exposure events, exception capture, before_send, and manually supplied properties. It supersedes the original group-only test in RequestContextTest.
  • Regression-first validation: the new tests failed before implementation; the compatibility-default cases also failed with a true default and passed after restoring false.
  • Full PHPUnit suite: 759 tests and 5,561 assertions pass (existing deprecations reported).
  • Adapter tests: 16 pass using local HTTP receivers. composer api:check and git diff --check pass. Changed-file PHP_CodeSniffer reports no errors and two pre-existing line-length warnings.
  • Not run: a send to a real PostHog project.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change; a minor-release change intent is included.

If releasing new changes

  • Ran pnpm change to generate the minor-release change intent.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Claude Code with Claude Opus 5.5 (claude-opus-5-5) wrote the change. Skills used: writing-pr-descriptions.
  • Follow-up commit 0fb3ea0 was implemented with the Pi coding assistant at the maintainer's request. It adds shared GeoIP configuration while explicitly retaining false as the compatibility default.
  • The change intent was regenerated with non-interactive pnpm change as a minor release.
  • Docs: added UPGRADING.md, a README link, and public API documentation. The PHP SDK page lives in the posthog.com repo and was not changed.

🤖 Generated with Claude Code

groupIdentify() puts every caller property under $group_set, so PHP
users had no way to set a top-level $geoip_disable on $groupidentify
events. The new disableGeoip option sets it, matching the Node,
Python, and Go SDKs and the group-identify spec.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@xljones xljones self-assigned this Oct 16, 2026
@xljones
xljones requested a review from a team as a code owner October 16, 2026 14:18
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in Greptile

[Medium impact] The PR appears safe to merge.

Reviews (2) · Last reviewed commit: "chore(groups): trim group_set detail fro..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-10-16T18:53:48.991543+00:00
Duration: 112324ms

⚠️ Some Tests Failed

36/47 tests passed, 11 failed


Capture Tests

⚠️ 19/30 tests passed, 11 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 35ms
Format Validation.Event Has Uuid ✅ 529ms
Format Validation.Event Has Lib Properties ✅ 529ms
Format Validation.Distinct Id Is String ✅ 530ms
Format Validation.Token Is Present ✅ 531ms
Format Validation.Custom Properties Preserved ✅ 530ms
Format Validation.Event Has Timestamp ✅ 530ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 530ms
Retry Behavior.Retries On 503 ❌ 5532ms
Retry Behavior.Does Not Retry On 400 ✅ 2535ms
Retry Behavior.Does Not Retry On 401 ✅ 2533ms
Retry Behavior.Respects Retry After Header ❌ 5535ms
Retry Behavior.Implements Backoff ❌ 15540ms
Retry Behavior.Retries On 500 ❌ 5042ms
Retry Behavior.Retries On 502 ❌ 5533ms
Retry Behavior.Retries On 504 ❌ 5538ms
Retry Behavior.Max Retries Respected ❌ 15546ms
Deduplication.Generates Unique Uuids ✅ 538ms
Deduplication.Preserves Uuid On Retry ❌ 5533ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 10543ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 5538ms
Deduplication.No Duplicate Events In Batch ✅ 536ms
Deduplication.Different Events Have Different Uuids ✅ 532ms
Compression.Sends Gzip When Enabled ✅ 532ms
Batch Format.Uses Proper Batch Structure ✅ 530ms
Batch Format.Flush With No Events Sends Nothing ✅ 517ms
Batch Format.Multiple Events Batched Together ✅ 520ms
Error Handling.Does Not Retry On 403 ✅ 2532ms
Error Handling.Does Not Retry On 413 ✅ 2532ms
Error Handling.Retries On 408 ❌ 5536ms

Failures

retry_behavior.retries_on_503

Expected at least 3 requests, got 1

retry_behavior.respects_retry_after_header

Expected at least 2 requests, got 1

retry_behavior.implements_backoff

Expected at least 3 requests, got 1

retry_behavior.retries_on_500

Expected at least 2 requests, got 1

retry_behavior.retries_on_502

Expected at least 2 requests, got 1

retry_behavior.retries_on_504

Expected at least 2 requests, got 1

retry_behavior.max_retries_respected

Expected 4 requests, got 1

deduplication.preserves_uuid_on_retry

Need at least 2 requests to check retry

deduplication.preserves_uuid_and_timestamp_on_retry

Expected at least 3 requests, got 1

deduplication.preserves_uuid_and_timestamp_on_batch_retry

Expected at least 2 requests, got 1

error_handling.retries_on_408

Expected at least 2 requests, got 1

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 522ms
Request Payload.Flags Request Uses V2 Query Param ✅ 521ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 521ms
Request Payload.Flags Request Omits Authorization Header ✅ 521ms
Request Payload.Token In Flags Body Matches Init ✅ 521ms
Request Payload.Groups Round Trip ✅ 521ms
Request Payload.Groups Default To Empty Object ✅ 522ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 522ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 521ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 521ms
Request Lifecycle.No Flags Request On Init Alone ✅ 516ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 518ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 525ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 523ms
Retry Behavior.Retries Flags On 502 ✅ 625ms
Retry Behavior.Retries Flags On 504 ✅ 624ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 532ms

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-10-16T18:53:45.141132+00:00
Duration: 117714ms

✅ All Tests Passed!

47/47 tests passed


Capture Tests

✅ 30/30 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 18ms
Format Validation.Event Has Uuid ✅ 513ms
Format Validation.Event Has Lib Properties ✅ 515ms
Format Validation.Distinct Id Is String ✅ 514ms
Format Validation.Token Is Present ✅ 515ms
Format Validation.Custom Properties Preserved ✅ 514ms
Format Validation.Event Has Timestamp ✅ 514ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 515ms
Retry Behavior.Retries On 503 ✅ 5820ms
Retry Behavior.Does Not Retry On 400 ✅ 2516ms
Retry Behavior.Does Not Retry On 401 ✅ 2518ms
Retry Behavior.Respects Retry After Header ✅ 8523ms
Retry Behavior.Implements Backoff ✅ 16230ms
Retry Behavior.Retries On 500 ✅ 5124ms
Retry Behavior.Retries On 502 ✅ 5622ms
Retry Behavior.Retries On 504 ✅ 5622ms
Retry Behavior.Max Retries Respected ✅ 17034ms
Deduplication.Generates Unique Uuids ✅ 220ms
Deduplication.Preserves Uuid On Retry ✅ 5620ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10828ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5625ms
Deduplication.No Duplicate Events In Batch ✅ 519ms
Deduplication.Different Events Have Different Uuids ✅ 516ms
Compression.Sends Gzip When Enabled ✅ 514ms
Batch Format.Uses Proper Batch Structure ✅ 514ms
Batch Format.Flush With No Events Sends Nothing ✅ 511ms
Batch Format.Multiple Events Batched Together ✅ 507ms
Error Handling.Does Not Retry On 403 ✅ 2515ms
Error Handling.Does Not Retry On 413 ✅ 2516ms
Error Handling.Retries On 408 ✅ 5621ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 514ms
Request Payload.Flags Request Uses V2 Query Param ✅ 513ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 513ms
Request Payload.Flags Request Omits Authorization Header ✅ 512ms
Request Payload.Token In Flags Body Matches Init ✅ 513ms
Request Payload.Groups Round Trip ✅ 512ms
Request Payload.Groups Default To Empty Object ✅ 513ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 513ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 512ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 513ms
Request Lifecycle.No Flags Request On Init Alone ✅ 509ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 506ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 515ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 513ms
Retry Behavior.Retries Flags On 502 ✅ 614ms
Retry Behavior.Retries Flags On 504 ✅ 614ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 514ms

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-10-16T18:54:09.789406+00:00
Duration: 149746ms

⚠️ Some Tests Failed

36/47 tests passed, 11 failed


Capture Tests

⚠️ 19/30 tests passed, 11 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 27ms
Format Validation.Event Has Uuid ✅ 522ms
Format Validation.Event Has Lib Properties ✅ 524ms
Format Validation.Distinct Id Is String ✅ 523ms
Format Validation.Token Is Present ✅ 523ms
Format Validation.Custom Properties Preserved ✅ 523ms
Format Validation.Event Has Timestamp ✅ 523ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 523ms
Retry Behavior.Retries On 503 ❌ 9233ms
Retry Behavior.Does Not Retry On 400 ✅ 2527ms
Retry Behavior.Does Not Retry On 401 ✅ 2526ms
Retry Behavior.Respects Retry After Header ❌ 9234ms
Retry Behavior.Implements Backoff ❌ 19245ms
Retry Behavior.Retries On 500 ❌ 9237ms
Retry Behavior.Retries On 502 ❌ 9236ms
Retry Behavior.Retries On 504 ❌ 9235ms
Retry Behavior.Max Retries Respected ❌ 19244ms
Deduplication.Generates Unique Uuids ✅ 531ms
Deduplication.Preserves Uuid On Retry ❌ 9233ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 14241ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 9239ms
Deduplication.No Duplicate Events In Batch ✅ 531ms
Deduplication.Different Events Have Different Uuids ✅ 524ms
Compression.Sends Gzip When Enabled ✅ 523ms
Batch Format.Uses Proper Batch Structure ✅ 523ms
Batch Format.Flush With No Events Sends Nothing ✅ 519ms
Batch Format.Multiple Events Batched Together ✅ 513ms
Error Handling.Does Not Retry On 403 ✅ 2526ms
Error Handling.Does Not Retry On 413 ✅ 2526ms
Error Handling.Retries On 408 ❌ 5529ms

Failures

retry_behavior.retries_on_503

Expected at least 3 requests, got 1

retry_behavior.respects_retry_after_header

Expected at least 2 requests, got 1

retry_behavior.implements_backoff

Expected at least 3 requests, got 1

retry_behavior.retries_on_500

Expected at least 2 requests, got 1

retry_behavior.retries_on_502

Expected at least 2 requests, got 1

retry_behavior.retries_on_504

Expected at least 2 requests, got 1

retry_behavior.max_retries_respected

Expected 4 requests, got 1

deduplication.preserves_uuid_on_retry

Need at least 2 requests to check retry

deduplication.preserves_uuid_and_timestamp_on_retry

Expected at least 3 requests, got 1

deduplication.preserves_uuid_and_timestamp_on_batch_retry

Expected at least 2 requests, got 1

error_handling.retries_on_408

Expected at least 2 requests, got 1

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 522ms
Request Payload.Flags Request Uses V2 Query Param ✅ 523ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 521ms
Request Payload.Flags Request Omits Authorization Header ✅ 522ms
Request Payload.Token In Flags Body Matches Init ✅ 522ms
Request Payload.Groups Round Trip ✅ 522ms
Request Payload.Groups Default To Empty Object ✅ 522ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 522ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 522ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 521ms
Request Lifecycle.No Flags Request On Init Alone ✅ 517ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 509ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 526ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 523ms
Retry Behavior.Retries Flags On 502 ✅ 623ms
Retry Behavior.Retries Flags On 504 ✅ 623ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 525ms

@xljones
xljones marked this pull request as draft October 16, 2026 14:37
@xljones
xljones marked this pull request as ready for review October 16, 2026 14:47

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code looks good; no qualifying findings. Verdict: correct.

@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 0fb3ea0: 1 Must fix.

Comment thread lib/Client.php
array $groupProperties = [],
bool $onlyEvaluateLocally = false,
bool $disableGeoip = false,
?bool $disableGeoip = null,

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.

P1 · Preserve compatibility with existing Client method overrides

Existing subclasses overriding Client::evaluateFlags() with bool $disableGeoip = false now fail at class loading: PHP rejects the narrower parameter because the parent accepts null. The same break affects overrides of Client::flags(). This makes the minor SDK update fatal for these applications; preserve the inherited signatures or reserve this change for a major release.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We acknowledge this breaks compatibility for existing subclasses overriding these methods. We’re accepting that break in this minor release as an explicit tradeoff: nullable parameters let omitted arguments inherit the client-wide GeoIP setting while preserving an explicit false override. Keeping bool $disableGeoip = false would lose that distinction. Given that subclassing these SDK methods is an uncommon usage pattern, we prefer consistent configuration behavior here. Affected subclasses will need to update their overrides to ?bool $disableGeoip = null.

@marandaneto

Copy link
Copy Markdown
Member

thanks @xljones
i pushed some changes to avoid breaking changes and adding a global settings similar to other SDKs

@marandaneto
marandaneto merged commit 9dd3981 into main Oct 16, 2026
28 checks passed
@marandaneto
marandaneto deleted the xljones/group-identify-disable-geoip branch October 16, 2026 19:22
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.

3 participants