NH-133385 PHP: investigate rate limiting mechanism in sampler

AGENTS.md

@@ -0,0 +1,48 @@
+# AGENTS.md
+
+## Scope and intent
+- `apm_ext` is a PHP extension that caches `solarwinds/apm` sampling data in-process; most work is under `ext/`.
+- Runtime boundary has 3 layers: Zend module/API (`ext/apm_ext.c`) -> C bridge (`ext/cache_c_wrapper.h/.cpp`) -> C++ cache (`ext/cache.h/.cpp`).
+- Public API is `Solarwinds\\Cache\\{get,put,getBucketState,putBucketState}` from `ext/apm_ext.stub.php` (generated header: `ext/apm_ext_arginfo.h`).
+
+## Core behavior you must preserve
+- Two caches exist in module globals (`ext/php_apm_ext.h`): settings and bucket-state, each with separate INI limits.
+- Settings key is intentionally composite in `ext/cache_c_wrapper.cpp`: `collector + hash(token) + serviceName` (token is never stored raw).
+- `Solarwinds::Cache` is LRU-by-access/update (`ext/cache.cpp`): both `Get` and `Put` move entries to MRU; overflow evicts oldest.
+- Fork handling is explicit in `ext/apm_ext.c`: `pthread_atfork(prefork, postfork, postfork)` frees/rebuilds caches across fork.
+
+## Project-specific rules (Do/Don't)
+- Do add/adjust tunables in `PHP_INI_BEGIN` (`ext/apm_ext.c`) and ensure they appear in `php --ri apm_ext` output.
+- Do keep API error contract: invalid/oversize input returns `false`; oversize paths also log to `stderr`.
+- Do keep the C/C++ boundary thin: add cache ops in `ext/cache_c_wrapper.h/.cpp`, then bind from Zend functions.
+- Don't hand-edit `ext/apm_ext_arginfo.h`; update `ext/apm_ext.stub.php` and regenerate stubs.
+- Don't bypass size guards (`COLLECTOR_MAX_LENGTH`, `TOKEN_MAX_LENGTH`, `SERVICE_NAME_MAX_LENGTH`, `PID_MAX_LENGTH`, and INI max lengths).
+
+## Build/test workflow used in this repo
+- Prefer root `Makefile` targets: `make build-image`, `make build`, `make test`, `make all`.
+- Docker services mount `./ext` to `/usr/src/myapp` (`docker-compose.yaml`), so extension build/tests run in that directory context.
+- `make build` executes `ext/build.sh` (`phpize && ./configure && make`).
+- Tests are PHPT (not PHPUnit); canonical suite is `ext/tests/*.phpt`.
+
+## Common commands (zsh)
+```zsh
+make all
+make build-image
+make build
+make test
+```
+
+## Quick task playbook
+- If editing Zend API or INI/config, start in `ext/apm_ext.c`; if editing function signatures, update `ext/apm_ext.stub.php` first.
+- If editing cache semantics/keying, touch `ext/cache.cpp` and/or `ext/cache_c_wrapper.cpp`, then run targeted PHPTs.
+- For API shape changes, do not edit `ext/apm_ext_arginfo.h` directly; regenerate from `ext/apm_ext.stub.php` before running tests.
+- Run the smallest relevant tests first (`004` update, `008` LRU, `012` fork), then run broader `make test`.
+
+## Test files to mirror when changing behavior
+- Cache update semantics: `ext/tests/004.phpt`.
+- LRU eviction/order semantics: `ext/tests/008.phpt`.
+- Fork isolation semantics: `ext/tests/012.phpt`.
+- Extension load/config smoke: `ext/tests/001.phpt` and nearby `00x.phpt` files.
+
+## Existing AI guidance discovery
+- One required glob search found only `README.md` as an existing AI-instructions source in this workspace.

README.md

@@ -5,7 +5,7 @@
 ![GitHub License](https://img.shields.io/github/license/solarwinds/apm-php-ext)
 
 ## Overview
-solarwinds/apm_ext is an add-on PHP extension to [solarwinds/apm](https://packagist.org/packages/solarwinds/apm), used to cache the sampling settings for the APM library.
+solarwinds/apm_ext is an add-on PHP extension to [solarwinds/apm](https://packagist.org/packages/solarwinds/apm), used to cache the sampling settings and the token bucket state for the APM library.
 
 ## Requirements
 - PHP 8+
@@ -25,10 +25,12 @@ pie install solarwinds/apm_ext
 
 This extension can be configured via `.ini` by modifying the following entries:
 
-| Configuration                 | Default | Description                                       |
-|-------------------------------|---------|---------------------------------------------------|
-| apm_ext.cache_max_entries     | 48      | Maximum number of entries in the cache            |
-| apm_ext.settings_max_length   | 2048    | Maximum length of the settings value in the cache |
+| Configuration                          | Default | Description                                                        |
+|----------------------------------------|---------|--------------------------------------------------------------------|
+| apm_ext.cache_max_entries              | 48      | Maximum number of entries in the cache                             |
+| apm_ext.settings_max_length            | 2048    | Maximum length of the settings value in the cache                  |
+| apm_ext.bucket_state_cache_max_entries | 512     | Maximum number of entries in the bucket state cache                |
+| apm_ext.bucket_state_max_length        | 2048    | Maximum length of the bucket state value in the bucket state cache |
 
 ## Verify that the extension is installed and enabled
 

docker/Dockerfile.alpine

@@ -1,4 +1,4 @@
-ARG ALPINE_VERSION=alpine:3.20@sha256:765942a4039992336de8dd5db680586e1a206607dd06170ff0a37267a9e01958
+ARG ALPINE_VERSION=alpine:3.23@sha256:25109184c71bdad752c8312a8623239686a9a2071e8825f20acb8f2198c3f659
 FROM ${ALPINE_VERSION} AS builder
 WORKDIR /usr/src
 

docker/Dockerfile.debian

@@ -1,4 +1,4 @@
-FROM debian:bookworm-slim@sha256:78d2f66e0fec9e5a39fb2c72ea5e052b548df75602b5215ed01a17171529f706
+FROM debian:bookworm-slim@sha256:f06537653ac770703bc45b4b113475bd402f451e85223f0f2837acbf89ab020a
 WORKDIR /usr/src
 
 #clang-format, gdb, valgrind

ext/apm_ext.c

@@ -24,6 +24,8 @@
 #define COLLECTOR_MAX_LENGTH 100
 #define TOKEN_MAX_LENGTH 128
 #define SERVICE_NAME_MAX_LENGTH 128
+// Windows process ID range can be [0, 4294967295]
+#define PID_MAX_LENGTH 10
 
 ZEND_DECLARE_MODULE_GLOBALS(apm_ext)
 
@@ -34,6 +36,12 @@ STD_PHP_INI_ENTRY("apm_ext.cache_max_entries", "48", PHP_INI_ALL,
 STD_PHP_INI_ENTRY("apm_ext.settings_max_length", "2048", PHP_INI_ALL,
                   OnUpdateLongGEZero, settings_max_length, zend_apm_ext_globals,
                   apm_ext_globals)
+STD_PHP_INI_ENTRY("apm_ext.bucket_state_cache_max_entries", "512", PHP_INI_ALL,
+                  OnUpdateLongGEZero, bucket_state_cache_max_entries, zend_apm_ext_globals,
+                  apm_ext_globals)
+STD_PHP_INI_ENTRY("apm_ext.bucket_state_max_length", "2048", PHP_INI_ALL,
+                  OnUpdateLongGEZero, bucket_state_max_length, zend_apm_ext_globals,
+                  apm_ext_globals)
 PHP_INI_END()
 
 /* {{{ void Solarwinds\\Cache\\get() */
@@ -110,14 +118,67 @@ PHP_FUNCTION(Solarwinds_Cache_put) {
 }
 /* }}} */
 
+/* {{{ void Solarwinds\\Cache\\getBucketState() */
+PHP_FUNCTION(Solarwinds_Cache_getBucketState) {
+  char *pid;
+  size_t pid_len;
+
+  ZEND_PARSE_PARAMETERS_START(1, 1)
+  Z_PARAM_STRING(pid, pid_len)
+  ZEND_PARSE_PARAMETERS_END();
+
+  if (!pid_len) {
+    RETURN_FALSE;
+  }
+
+  zend_string *state = Cache_GetBucketState(APM_EXT_G(bucket_state_cache), pid);
+  if (state != NULL) {
+    RETURN_NEW_STR(state);
+  }
+  RETURN_FALSE;
+}
+/* }}} */
+
+/* {{{ void Solarwinds\\Cache\\putBucketState() */
+PHP_FUNCTION(Solarwinds_Cache_putBucketState) {
+  char *pid;
+  size_t pid_len;
+  char *bucket_state;
+  size_t bucket_state_len;
+
+  ZEND_PARSE_PARAMETERS_START(2, 2)
+  Z_PARAM_STRING(pid, pid_len)
+  Z_PARAM_STRING(bucket_state, bucket_state_len)
+  ZEND_PARSE_PARAMETERS_END();
+
+  if (pid_len && bucket_state_len) {
+    if (pid_len > PID_MAX_LENGTH) {
+      fprintf(stderr, "apm_ext: pid length %zu exceeds max length %d\n", pid_len, PID_MAX_LENGTH);
+      RETURN_FALSE;
+    }
+    if (bucket_state_len > (size_t)APM_EXT_G(bucket_state_max_length)) {
+      fprintf(stderr, "apm_ext: bucket state length %zu exceeds max length %ld\n",
+              bucket_state_len, (long)APM_EXT_G(bucket_state_max_length));
+      RETURN_FALSE;
+    }
+    Cache_PutBucketState(APM_EXT_G(bucket_state_cache), pid, bucket_state);
+    RETURN_TRUE;
+  }
+  RETURN_FALSE;
+}
+/* }}} */
+
 #ifndef _WIN32
 void prefork() {
+  Cache_Free(APM_EXT_G(bucket_state_cache));
+  APM_EXT_G(bucket_state_cache) = NULL;
   Cache_Free(APM_EXT_G(cache));
   APM_EXT_G(cache) = NULL;
 }
 
 void postfork() {
   APM_EXT_G(cache) = Cache_Allocate(APM_EXT_G(cache_max_entries));
+  APM_EXT_G(bucket_state_cache) = Cache_Allocate(APM_EXT_G(bucket_state_cache_max_entries));
 }
 #endif
 
@@ -129,7 +190,7 @@ PHP_MINIT_FUNCTION(apm_ext) {
   REGISTER_INI_ENTRIES();
 
   APM_EXT_G(cache) = Cache_Allocate(APM_EXT_G(cache_max_entries));
-
+  APM_EXT_G(bucket_state_cache) = Cache_Allocate(APM_EXT_G(bucket_state_cache_max_entries));
 #ifndef _WIN32
   pthread_atfork(prefork, postfork, postfork);
 #endif
@@ -140,6 +201,8 @@ PHP_MINIT_FUNCTION(apm_ext) {
 
 /* {{{ PHP_MSHUTDOWN_FUNCTION */
 PHP_MSHUTDOWN_FUNCTION(apm_ext) {
+  Cache_Free(APM_EXT_G(bucket_state_cache));
+  APM_EXT_G(bucket_state_cache) = NULL;
   Cache_Free(APM_EXT_G(cache));
   APM_EXT_G(cache) = NULL;
 

ext/apm_ext.stub.php

@@ -7,3 +7,7 @@
 function get(string $collector, string $token, string $serviceName): string|false {}
 
 function put(string $collector, string $token, string $serviceName, string $settings): bool {}
+
+function getBucketState(string $pid): string|false {}
+
+function putBucketState(string $pid, string $bucketState): bool {}

ext/apm_ext_arginfo.h

@@ -1,26 +1,39 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 109c874d71ea223fc4aa34c5a131dd8ac8e46c8b */
+ * Stub hash: 8b79f936643750559071769b234b5f16f17bcaf1 */
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_Solarwinds_Cache_get, 0, 3,
-                                        MAY_BE_STRING | MAY_BE_FALSE)
-ZEND_ARG_TYPE_INFO(0, collector, IS_STRING, 0)
-ZEND_ARG_TYPE_INFO(0, token, IS_STRING, 0)
-ZEND_ARG_TYPE_INFO(0, serviceName, IS_STRING, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_Solarwinds_Cache_get, 0, 3, MAY_BE_STRING|MAY_BE_FALSE)
+	ZEND_ARG_TYPE_INFO(0, collector, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, token, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, serviceName, IS_STRING, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_Solarwinds_Cache_put, 0, 4,
-                                        _IS_BOOL, 0)
-ZEND_ARG_TYPE_INFO(0, collector, IS_STRING, 0)
-ZEND_ARG_TYPE_INFO(0, token, IS_STRING, 0)
-ZEND_ARG_TYPE_INFO(0, serviceName, IS_STRING, 0)
-ZEND_ARG_TYPE_INFO(0, settings, IS_STRING, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_Solarwinds_Cache_put, 0, 4, _IS_BOOL, 0)
+	ZEND_ARG_TYPE_INFO(0, collector, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, token, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, serviceName, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, settings, IS_STRING, 0)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_Solarwinds_Cache_getBucketState, 0, 1, MAY_BE_STRING|MAY_BE_FALSE)
+	ZEND_ARG_TYPE_INFO(0, pid, IS_STRING, 0)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_Solarwinds_Cache_putBucketState, 0, 2, _IS_BOOL, 0)
+	ZEND_ARG_TYPE_INFO(0, pid, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO(0, bucketState, IS_STRING, 0)
+ZEND_END_ARG_INFO()
+
+
 ZEND_FUNCTION(Solarwinds_Cache_get);
 ZEND_FUNCTION(Solarwinds_Cache_put);
+ZEND_FUNCTION(Solarwinds_Cache_getBucketState);
+ZEND_FUNCTION(Solarwinds_Cache_putBucketState);
+
 
 static const zend_function_entry ext_functions[] = {
-    ZEND_NS_FALIAS("Solarwinds\\Cache", get, Solarwinds_Cache_get,
-                   arginfo_Solarwinds_Cache_get)
-        ZEND_NS_FALIAS("Solarwinds\\Cache", put, Solarwinds_Cache_put,
-                       arginfo_Solarwinds_Cache_put) ZEND_FE_END};
+	ZEND_NS_FALIAS("Solarwinds\\Cache", get, Solarwinds_Cache_get, arginfo_Solarwinds_Cache_get)
+	ZEND_NS_FALIAS("Solarwinds\\Cache", put, Solarwinds_Cache_put, arginfo_Solarwinds_Cache_put)
+	ZEND_NS_FALIAS("Solarwinds\\Cache", getBucketState, Solarwinds_Cache_getBucketState, arginfo_Solarwinds_Cache_getBucketState)
+	ZEND_NS_FALIAS("Solarwinds\\Cache", putBucketState, Solarwinds_Cache_putBucketState, arginfo_Solarwinds_Cache_putBucketState)
+	ZEND_FE_END
+};

ext/cache.cpp

@@ -3,10 +3,7 @@
 namespace Solarwinds {
 Cache::Cache(size_t max_entries) : max_entries_(max_entries) {}
 
-void Cache::Put(const std::string &collector, const std::string &token,
-                const std::string &serviceName, const std::string &settings) {
-  auto key =
-      collector + std::to_string(std::hash<std::string>{}(token)) + serviceName;
+void Cache::Put(const std::string &key, const std::string &value) {
   std::lock_guard<std::mutex> lock(cache_mutex_);
   if (cache_map_.find(key) == cache_map_.end()) {
     // new
@@ -15,31 +12,27 @@ void Cache::Put(const std::string &collector, const std::string &token,
       cache_map_.erase(it.first);
       cache_.pop_front();
     }
-    cache_.push_back(std::make_pair(key, settings));
+    cache_.push_back(std::make_pair(key, value));
     cache_map_[key] = std::prev(cache_.end());
   } else {
     // update, move cache
     auto it = cache_map_[key];
     cache_.erase(it);
-    cache_.push_back(std::make_pair(key, settings));
+    cache_.push_back(std::make_pair(key, value));
     cache_map_[key] = std::prev(cache_.end());
   }
 }
-std::pair<bool, std::string> Cache::Get(const std::string &collector,
-                                        const std::string &token,
-                                        const std::string &serviceName) {
-  auto key =
-      collector + std::to_string(std::hash<std::string>{}(token)) + serviceName;
+std::pair<bool, std::string> Cache::Get(const std::string &key) {
   std::lock_guard<std::mutex> lock(cache_mutex_);
   auto it = cache_map_.find(key);
   if (it == cache_map_.end()) {
     return std::make_pair(false, "");
   }
   auto cache_iterator = it->second;
-  auto settings = cache_iterator->second;
+  auto value = cache_iterator->second;
   cache_.erase(cache_iterator);
-  cache_.push_back(std::make_pair(key, settings));
+  cache_.push_back(std::make_pair(key, value));
   cache_map_[key] = std::prev(cache_.end());
-  return std::make_pair(true, settings);
+  return std::make_pair(true, value);
 }
 } // namespace Solarwinds

ext/cache.h

@@ -8,8 +8,7 @@
 #include <utility>
 
 /**
- * Simple in-memory cache for storing settings based on collector, token, and
- * service name.
+ * Simple in-memory cache for storing value based on a key.
  */
 namespace Solarwinds {
 
@@ -23,25 +22,18 @@ class Cache {
   /**
    * Store settings in the cache.
    *
-   * @param collector The collector endpoint.
-   * @param token The token.
-   * @param serviceName The name of the service.
-   * @param settings The settings to be cached.
+   * @param key The key.
+   * @param value The value.
    */
-  void Put(const std::string &collector, const std::string &token,
-           const std::string &serviceName, const std::string &settings);
+  void Put(const std::string &key, const std::string &value);
   /**
    * Retrieve settings from the cache.
    *
-   * @param collector The collector endpoint.
-   * @param token The token.
-   * @param serviceName The name of the service.
-   * @return A pair where the first element indicates if the settings were
-   * found, and the second element is the cached settings (if found).
+   * @param key The key.
+   * @return A pair where the first element indicates if the value were
+   * found, and the second element is the value (if found).
    */
-  std::pair<bool, std::string> Get(const std::string &collector,
-                                   const std::string &token,
-                                   const std::string &serviceName);
+  std::pair<bool, std::string> Get(const std::string &key);
 
 private:
   /**