fix(CBP-51066): PHP SDK cache status and distinctId fixes by ankurv02 · Pull Request #54 · rollout/rox-php

ankurv02 · 2026-06-19T09:59:05Z

CBP-51066: [rox-php] AppliedFromLocalStorage never fired and unstable distinctId causing excessive CDN requests

This PR have two major bugs reported by ChargePoint (SECO-5292 and SECO-5553) that were causing broken monitoring metrics and massive CDN traffic spikes.

Bug 1: AppliedFromLocalStorage status never fired
The Problem:
Our docs promise that the ConfigurationFetchedHandler will receive an AppliedFromLocalStorage status whenever the SDK serves a config from the local cache. In reality, it was always sending AppliedFromNetwork.

Why it happened:
The PHP SDK uses a Guzzle caching library to save CDN responses to the disk. This library sits entirely in the background. When it intercepted a request and served it from the local disk, our core SDK code just saw a standard HTTP 200 response. It had no clue the data came from the cache, so it defaulted to saying it came from the network.

The Fix:
The Guzzle caching library actually tags its cache hits with an X-Kevinrob-Cache: HIT header. We now check for this header and bubble that flag up the response chain to Core::fetch(). Now, the SDK can accurately fire AppliedFromLocalStorage on cache hits and AppliedFromNetwork on real network hits.

Bug 2: Bad cache keys causing ~1M CDN requests/hour
The Problem:
ChargePoint noticed that setting setConfigFetchIntervalInSeconds(300) (5 minutes) wasn't doing anything. They were still hammering the CDN on every single execution—totaling around 1 million requests per hour.

Why it happened:
The SDK builds a CDN query URL using a distinctId to identify the server node. If a user didn't explicitly set the ROLLOUT_INSTANCE_ID environment variable, the SDK tried to generate a fallback ID using the PHP script's file inode and last-modified timestamp.

In a Docker/Kubernetes setup, these file properties constantly change across different containers and pods. Because the distinctId was unstable, the CDN URL was different on almost every request. Since Guzzle uses the exact URL as its cache key, the cache missed every single time and forced a real network call.

The Fix:
Swapped out the unstable file-based fallback for a hash of the API key: md5($apiKey). The API key is static, uniquely identifies the app, and stays perfectly stable across containers, pods, and deployments. (Note: If a customer relies on unique instance tracking, ROLLOUT_INSTANCE_ID still takes priority over the fallback).

What changed under the hood
Psr7ResponseWrapper: Now sniffs out the X-Kevinrob-Cache: HIT header.

HttpResponseInterface / AbstractHttpResponse: Added an isFromCache() method (defaults to false so nothing breaks upstream).

ConfigurationFetchResult: Acts as the carrier to pass the cache boolean up to the core engine.

ConfigurationFetcherBase / ConfigurationFetcher: Forwards the cache flag specifically for CDN responses.

Core::fetch(): Evaluates the cache flag to fire the correct FetcherStatus.

ServerProperties: Switched the distinctId fallback logic to use md5($apiKey).

Tests: Added two new unit tests and updated the E2E handler to assert against both cache statuses.

… distinctId causing excessive CDN requests

snyk-prodsec-rw · 2026-06-19T09:59:28Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

markawm · 2026-06-19T17:21:49Z

-                getmyinode(),
-                getlastmod()
-            ]));
+            ?: md5($sdkSettings->getApiKey());


The API key is not the distinct ID: doing this would lead to every client for this SDK key appearing as a single 'user' for our usage counting (assuming we started counting server usage in the future). But it would also affect split functionality (if user has not overridden the stickiness property). I suggest keeping this as it was - is there a reason we need to change the distinctId?

(I know you mentioned in the description something about changing the cache key, which makes sense - this should not include the distinctId. But we can change cache key to use sdk key (& maybe buid? i.e. the cdn path part only) without changing the distinctId value.)

Removed distinctId from CDN GET query params so the Guzzle cache key is stable across containers/pods — fixes CDN cache thrashing without touching the distinctId value

markawm · 2026-06-23T10:37:58Z

-            PropertyType::getDistinctId()->getName() =>
-                (string)$properties[PropertyType::getDistinctId()->getName()],


I don't know php at all, but is this removing distinctId from the query params (?distinctId=...)? Or just from the cache key? We should continue to send the parameter... just not use it as the cache key.

Reverted. you're right, we were removing it from the request entirely which was wrong. Fixed: distinct_id is still sent as a query param to the CDN, we now override getCacheKey in CdnCacheStrategy to strip it from the URI before computing the cache key hash. So the CDN gets the param, but the local Guzzle cache key stays stable across instances.

markawm · 2026-06-23T10:38:27Z

            $this->_flagSetter->setExperiments();

-            $hasChanges = ($this->_lastConfigurations == null || $this->_lastConfigurations->equals($result));
+            $hasChanges = ($this->_lastConfigurations == null || !$this->_lastConfigurations->equals($result));


… request

markawm · 2026-06-23T15:13:19Z

+        // Strip distinct_id from the URI before computing the cache key so that
+        // per-instance identity doesn't cause cache misses across containers/pods.
+        // The param is still sent in the actual HTTP request.
+        $uri = Uri::withoutQueryValue($request->getUri(), PropertyType::getDistinctId()->getName());


Are all the other URL query parameters going to be stable? I think simplest/ideal would just be Uri::path($request->getUri())` - if that's even a thing in php!

simplified to $request->getUri()->withQuery('') which strips all query params. The other params (sdkVersion, platformVersion, etc.) are stable per deployment so no cache correctness concern, but keying on path only is cleaner 👍

markawm

👍 (at least, as much as I understand php!)

AsafRollout · 2026-06-24T13:04:17Z

@ankurv02
i know i'm late again, but did you test the has_changes?
from when i tested it, it wasn't just the not ! problem
each request is new, so there's never a _lastConfigurations value, it always returned true
had to create a hash from the config, and store it on disk to compare...

ankurv02 · 2026-06-24T13:09:51Z

@ankurv02 i know i'm late again, but did you test the has_changes? from when i tested it, it wasn't just the not ! problem each request is new, so there's never a _lastConfigurations value, it always returned true had to create a hash from the config, and store it on disk to compare...

let me cover this here before we push for release - #55

fix(CBP-51066): fix AppliedFromLocalStorage never firing and unstable…

c3862a5

… distinctId causing excessive CDN requests

ankurv02 requested review from a team, AsafRollout, kwhetstone and markawm June 19, 2026 09:59

markawm reviewed Jun 19, 2026

View reviewed changes

remove distinctId from CDN query params and fix hasChanges inversion

60618a4

ankurv02 requested a review from markawm June 22, 2026 14:12

markawm reviewed Jun 23, 2026

View reviewed changes

exclude distinctId from CDN cache key instead of removing it from the…

82e5cc8

… request

ankurv02 requested a review from markawm June 23, 2026 10:54

markawm reviewed Jun 23, 2026

View reviewed changes

refactor:key CDN cache on path only, stripping all query params

997a3d4

ankurv02 requested a review from markawm June 24, 2026 05:31

markawm approved these changes Jun 24, 2026

View reviewed changes

ankurv02 merged commit ab7b8e9 into master Jun 24, 2026
13 checks passed

ankurv02 mentioned this pull request Jun 24, 2026

fix: persist config hash to disk so hasChanges works correctly across PHP-FPM processes #56

Merged

		PropertyType::getDistinctId()->getName() =>
		(string)$properties[PropertyType::getDistinctId()->getName()],

Uh oh!

Conversation

ankurv02 commented Jun 19, 2026 • edited by cloudbees-issuebot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snyk-prodsec-rw commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markawm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AsafRollout commented Jun 24, 2026

Uh oh!

ankurv02 commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ankurv02 commented Jun 19, 2026 •

edited by cloudbees-issuebot Bot

Loading

snyk-prodsec-rw commented Jun 19, 2026 •

edited

Loading