From 0933b7e46c9a98f6e932ef6cc7b3c15f4f5f703d Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Fri, 10 Nov 2023 18:59:22 -0300 Subject: [PATCH 01/12] experimental support for tracer --- src/Client.php | 41 ++++++++++++++++++++++-- src/Config/Tracer.php | 43 ++++++++++++++++++++++++++ src/Config/Utils.php | 12 ++++++-- src/Utils/NoOpTracerHook.php | 10 ++++++ src/Utils/Tracer.php | 37 ++++++++++++++++++++++ src/Utils/TracerHook.php | 8 +++++ tests/ClientTest.php | 60 +++++++++++++++++++++++++++++++++--- tests/Config/TraceTest.php | 31 +++++++++++++++++++ tests/Config/UtilsTest.php | 10 ++++++ 9 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 src/Config/Tracer.php create mode 100644 src/Utils/NoOpTracerHook.php create mode 100644 src/Utils/Tracer.php create mode 100644 src/Utils/TracerHook.php create mode 100644 tests/Config/TraceTest.php diff --git a/src/Client.php b/src/Client.php index 8ce2418..20dbcee 100644 --- a/src/Client.php +++ b/src/Client.php @@ -3,6 +3,8 @@ namespace SplitIO\ThinSdk; use \SplitIO\ThinSdk\Utils\ImpressionListener; +use \SplitIO\ThinSdk\Utils\Tracer; +use \SplitIO\ThinSdk\Utils\NoOpTracerHook; use \SplitIO\ThinSdk\Utils\EvalCache\Cache; use \SplitIO\ThinSdk\Utils\EvalCache\NoCache; use \SplitIO\ThinSdk\Utils\InputValidator\InputValidator; @@ -20,36 +22,45 @@ class Client implements ClientInterface private /*?ImpressionListener*/ $impressionListener; private /*InputValidator*/ $inputValidator; private /*Cache*/ $cache; + private /*Tracer*/ $tracer; - public function __construct(Manager $manager, LoggerInterface $logger, ?ImpressionListener $impressionListener, ?Cache $cache = null) + public function __construct(Manager $manager, LoggerInterface $logger, ?ImpressionListener $impressionListener, ?Cache $cache = null, ?Tracer $tracer = null) { $this->logger = $logger; $this->lm = $manager; $this->impressionListener = $impressionListener; $this->inputValidator = new InputValidator($logger); $this->cache = $cache ?? new NoCache(); + $this->tracer = $tracer ?? new Tracer(new NoOpTracerHook()); } public function getTreatment(string $key, ?string $bucketingKey, string $feature, ?array $attributes = null): string { try { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); if (($fromCache = $this->cache->get($key, $feature, $attributes)) != null) { return $fromCache; } + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_START, null); list($treatment, $ilData) = $this->lm->getTreatment($key, $bucketingKey, $feature, $attributes); + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_END, null); $this->handleListener($key, $bucketingKey, $feature, $attributes, $treatment, $ilData); $this->cache->set($key, $feature, $attributes, $treatment); return $treatment; } catch (\Exception $exc) { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_EXCEPTION, null); $this->logger->error($exc); return "control"; + } finally { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_END, null); } } public function getTreatments(string $key, ?string $bucketingKey, array $features, ?array $attributes = null): array { try { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); // try to fetch items from cache. return result if all evaluations are cached // otherwise, send a Treatments RPC for missing ones and return merged result $toReturn = $this->cache->getMany($key, $features, $attributes); @@ -58,7 +69,9 @@ public function getTreatments(string $key, ?string $bucketingKey, array $feature return $toReturn; } + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_START, null); $results = $this->lm->getTreatments($key, $bucketingKey, $features, $attributes); + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_END, null); foreach ($results as $feature => $result) { list($treatment, $ilData) = $result; $toReturn[$feature] = $treatment; @@ -67,35 +80,44 @@ public function getTreatments(string $key, ?string $bucketingKey, array $feature $this->cache->setMany($key, $attributes, $toReturn); return $toReturn; } catch (\Exception $exc) { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_EXCEPTION, null); $this->logger->error($exc); return array_reduce($features, function ($r, $k) { $r[$k] = "control"; return $r; }, []); + } finally { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_END, null); } } public function getTreatmentWithConfig(string $key, ?string $bucketingKey, string $feature, ?array $attributes = null): array { try { - + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); if (($fromCache = $this->cache->getWithConfig($key, $feature, $attributes)) != null) { return $fromCache; } + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_START, null); list($treatment, $ilData, $config) = $this->lm->getTreatmentWithConfig($key, $bucketingKey, $feature, $attributes); + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_END, null); $this->handleListener($key, $bucketingKey, $feature, $attributes, $treatment, $ilData); $this->cache->setWithConfig($key, $feature, $attributes, $treatment, $config); return ['treatment' => $treatment, 'config' => $config]; } catch (\Exception $exc) { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_EXCEPTION, null); $this->logger->error($exc); return "control"; + } finally { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_END, null); } } public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, array $features, ?array $attributes = null): array { try { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); $toReturn = $this->cache->getManyWithConfig($key, $features, $attributes); $features = self::getMissing($toReturn); @@ -103,7 +125,9 @@ public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, arra return $toReturn; } + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_START, null); $results = $this->lm->getTreatmentsWithConfig($key, $bucketingKey, $features, $attributes); + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_END, null); foreach ($results as $feature => $result) { list($treatment, $ilData, $config) = $result; $toReturn[$feature] = ['treatment' => $treatment, 'config' => $config]; @@ -112,23 +136,34 @@ public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, arra $this->cache->setManyWithConfig($key, $attributes, $toReturn); return $toReturn; } catch (\Exception $exc) { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_EXCEPTION, null); $this->logger->error($exc); return array_reduce($features, function ($r, $k) { $r[$k] = ['treatment' => 'control', 'config' => null]; return $r; }, []); + } finally { + $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_END, null); } } public function track(string $key, string $trafficType, string $eventType, ?float $value = null, ?array $properties = null): bool { try { + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); $properties = $this->inputValidator->validProperties($properties); - return $this->lm->track($key, $trafficType, $eventType, $value, $properties); + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_RPC_START, null); + $res = $this->lm->track($key, $trafficType, $eventType, $value, $properties); + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_RPC_END, null); + return $res; } catch (ValidationException $exc) { + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_EXCEPTION, null); $this->logger->error("error validating event properties: " . $exc->getMessage()); } catch (\Exception $exc) { + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_EXCEPTION, null); $this->logger->error($exc); + } finally { + $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_END, null); } return false; } diff --git a/src/Config/Tracer.php b/src/Config/Tracer.php new file mode 100644 index 0000000..78db156 --- /dev/null +++ b/src/Config/Tracer.php @@ -0,0 +1,43 @@ +hook = $hook; + $this->forwardArgs = $forwardArguments; + } + + public function hook(): ?TracerHook + { + return $this->hook; + } + + public function forwardArgs(): bool + { + return $this->forwardArgs; + } + + public static function fromArray(array $config): Tracer + { + $d = self::default(); + return new Tracer( + $config['hook'] ?? $d->hook(), + $config['forwardArgs'] ?? $d->forwardArgs, + ); + } + + public static function default(): Tracer + { + return new Tracer(null, false); + } +} diff --git a/src/Config/Utils.php b/src/Config/Utils.php index 2b38b31..d575830 100644 --- a/src/Config/Utils.php +++ b/src/Config/Utils.php @@ -9,11 +9,13 @@ class Utils { private /*?ImpressionListener*/ $listener; private /*?string*/ $evaluationCache; + private /*?TracerHook*/ $tracer; - private function __construct(?ImpressionListener $listener, EvaluationCache $cache) + private function __construct(?ImpressionListener $listener, EvaluationCache $cache, Tracer $tracer) { $this->listener = $listener; $this->evaluationCache = $cache; + $this->tracer = $tracer; } public function impressionListener(): ?ImpressionListener @@ -21,6 +23,11 @@ public function impressionListener(): ?ImpressionListener return $this->listener; } + public function tracer(): Tracer + { + return $this->tracer; + } + public function evaluationCache(): ?EvaluationCache { return $this->evaluationCache; @@ -32,11 +39,12 @@ public static function fromArray(array $config): Utils return new Utils( $config['impressionListener'] ?? $d->impressionListener(), EvaluationCache::fromArray($config['evaluationCache'] ?? []), + Tracer::fromArray($config['__tracer'] ?? []), ); } public static function default(): Utils { - return new Utils(null, EvaluationCache::default()); + return new Utils(null, EvaluationCache::default(), Tracer::default()); } } diff --git a/src/Utils/NoOpTracerHook.php b/src/Utils/NoOpTracerHook.php new file mode 100644 index 0000000..4c15368 --- /dev/null +++ b/src/Utils/NoOpTracerHook.php @@ -0,0 +1,10 @@ +hook = $hook ?? new NoOpTracerHook(); + $this->includeArgs = $includeArgs; + } + + public function includeArgs(): bool + { + return $this->includeArgs; + } + + public function trace(int $method, int $event, ?array $args) + { + $this->hook->on($method, $event, $args); + } +} diff --git a/src/Utils/TracerHook.php b/src/Utils/TracerHook.php new file mode 100644 index 0000000..f158a96 --- /dev/null +++ b/src/Utils/TracerHook.php @@ -0,0 +1,8 @@ +with('someKey', 'someBuck', 'someFeature', ['someAttr' => 123]) ->willReturn(['on', null, null]); - $client = new Client($manager, $this->logger, null); + $tracer = $this->createMock(Tracer::class); + $tracer->expects($this->once()) + ->method('includeArgs')->willReturn(true); + $tracer->expects($this->exactly(4)) + ->method('trace') + ->withConsecutive( + [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_START, ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]]], + [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_START, null], + [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_END, null], + [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_END, null], + ); + + $client = new Client($manager, $this->logger, null, null, $tracer); $this->assertEquals('on', $client->getTreatment('someKey', 'someBuck', 'someFeature', ['someAttr' => 123])); } @@ -78,6 +91,18 @@ public function testGetTreatmentsWithImpListener() 'someFeature3' => ['n/a', new ImpressionListenerData('lab1', 125, 123458), null], ]); + $tracer = $this->createMock(Tracer::class); + $tracer->expects($this->once()) + ->method('includeArgs')->willReturn(true); + $tracer->expects($this->exactly(4)) + ->method('trace') + ->withConsecutive( + [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_START, ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]]], + [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_START, null], + [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_END, null], + [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_END, null], + ); + $ilMock = $this->createMock(ImpressionListener::class); $ilMock->expects($this->exactly(3)) ->method('accept') @@ -88,7 +113,7 @@ public function testGetTreatmentsWithImpListener() ); - $client = new Client($manager, $this->logger, $ilMock); + $client = new Client($manager, $this->logger, $ilMock, null, $tracer); $this->assertEquals( ['someFeature1' => 'on', 'someFeature2' => 'off', 'someFeature3' => 'n/a'], $client->getTreatments('someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]) @@ -107,7 +132,19 @@ public function testGetTreatmentWithConfigAndListener() ->method('accept') ->with(new Impression('someKey', 'someBuck', 'someFeature', 'on', 'lab1', 123, 123456), ['someAttr' => 123]); - $client = new Client($manager, $this->logger, $ilMock); + $tracer = $this->createMock(Tracer::class); + $tracer->expects($this->once()) + ->method('includeArgs')->willReturn(true); + $tracer->expects($this->exactly(4)) + ->method('trace') + ->withConsecutive( + [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_START, ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]]], + [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_START, null], + [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_END, null], + [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_END, null], + ); + + $client = new Client($manager, $this->logger, $ilMock, null, $tracer); $this->assertEquals( ['treatment' => 'on', 'config' => '{"a": 1}'], $client->getTreatmentWithConfig('someKey', 'someBuck', 'someFeature', ['someAttr' => 123]) @@ -134,8 +171,23 @@ public function testGetTreatmentsWithConfigAndListener() [new Impression('someKey', 'someBuck', 'someFeature3', 'n/a', 'lab1', 125, 123458), ['someAttr' => 123]] ); + $tracer = $this->createMock(Tracer::class); + $tracer->expects($this->once()) + ->method('includeArgs')->willReturn(true); + $tracer->expects($this->exactly(4)) + ->method('trace') + ->withConsecutive( + [ + Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, + Tracer::EVENT_START, + ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]], + ], + [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_START, null], + [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_END, null], + [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_END, null], + ); - $client = new Client($manager, $this->logger, $ilMock); + $client = new Client($manager, $this->logger, $ilMock, null, $tracer); $this->assertEquals( [ 'someFeature1' => ['treatment' => 'on', 'config' => null], diff --git a/tests/Config/TraceTest.php b/tests/Config/TraceTest.php new file mode 100644 index 0000000..6a2341e --- /dev/null +++ b/tests/Config/TraceTest.php @@ -0,0 +1,31 @@ +assertEquals(null, $cfg->hook()); + $this->assertEquals(false, $cfg->forwardArgs()); + } + + public function testConfigParsing() + { + $tMock = $this->createMock(TracerHook::class); + $cfg = Tracer::fromArray([ + 'hook' => $tMock, + 'forwardArgs' => true, + ]); + + $this->assertEquals($tMock, $cfg->hook()); + $this->assertEquals(true, $cfg->forwardArgs()); + } +} diff --git a/tests/Config/UtilsTest.php b/tests/Config/UtilsTest.php index d42eb6d..3619dac 100644 --- a/tests/Config/UtilsTest.php +++ b/tests/Config/UtilsTest.php @@ -4,8 +4,10 @@ use SplitIO\ThinSdk\Config\Utils; use SplitIO\ThinSdk\Config\EvaluationCache; +use SplitIO\ThinSdk\Config\Tracer; use SplitIO\ThinSdk\Utils\ImpressionListener; use SplitIO\ThinSdk\Utils\EvalCache\InputHasher; +use SplitIO\ThinSdk\Utils\TracerHook; use PHPUnit\Framework\TestCase; @@ -17,12 +19,14 @@ public function testConfigDefault() $cfg = Utils::default(); $this->assertEquals(null, $cfg->impressionListener()); $this->assertEquals(EvaluationCache::default(), $cfg->evaluationCache()); + $this->assertEquals(Tracer::default(), $cfg->tracer()); } public function testConfigParsing() { $ilMock = $this->createMock(ImpressionListener::class); $ihMock = $this->createMock(InputHasher::class); + $tMock = $this->createMock(TracerHook::class); $cfg = Utils::fromArray([ 'impressionListener' => $ilMock, 'evaluationCache' => [ @@ -31,6 +35,10 @@ public function testConfigParsing() 'maxSize' => 1234, 'customHash' => $ihMock, ], + '__tracer' => [ + 'hook' => $tMock, + 'forwardArgs' => true, + ], ]); $this->assertEquals($ilMock, $cfg->impressionListener()); @@ -38,5 +46,7 @@ public function testConfigParsing() $this->assertEquals('random', $cfg->evaluationCache()->evictionPolicy()); $this->assertEquals($ihMock, $cfg->evaluationCache()->customHash()); $this->assertEquals(1234, $cfg->evaluationCache()->maxSize()); + $this->assertEquals($tMock, $cfg->tracer()->hook()); + $this->assertEquals(true, $cfg->tracer()->forwardArgs()); } } From e38bb0d571cf3fb7d27711fdd75965b1eccda760 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Fri, 10 Nov 2023 19:25:42 -0300 Subject: [PATCH 02/12] add example --- examples/tracer.php | 68 ++++++++++++++++++++++++++++++++++++++++++++ src/Factory.php | 4 ++- src/Utils/Tracer.php | 6 ++-- 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 examples/tracer.php diff --git a/examples/tracer.php b/examples/tracer.php new file mode 100644 index 0000000..fc0cd2c --- /dev/null +++ b/examples/tracer.php @@ -0,0 +1,68 @@ +events, "start (" . json_encode($arguments) . ") -- " . microtime(true)); + break; + case Tracer::EVENT_RPC_START: + array_push($this->events, "about to send rpc -- " . microtime(true)); + break; + case Tracer::EVENT_RPC_END: + array_push($this->events, "rpc completed -- " . microtime(true)); + break; + case Tracer::EVENT_EXCEPTION: + array_push($this->events, "exception occured -- " . microtime(true)); + break; + case Tracer::EVENT_END: + array_push($this->events, "end -- " . microtime(true)); + break; + } + } + + public function getEvents(): array + { + return $this->events; + } +} + +$ct = new CustomTracer(); + +$factory = Factory::withConfig([ + 'transfer' => [ + 'address' => '../../splitd.sock', + 'type' => 'unix-stream', + ], + 'logging' => [ + 'level' => \Psr\Log\LogLevel::INFO, + ], + 'utils' => [ + '__tracer' => [ + 'hook' => $ct, + 'forwardArgs' => true, + ] + ], + +]); + +$manager = $factory->manager(); +$client = $factory->client(); +echo $client->getTreatment("key", null, $manager->splitNames()[0], ['age' => 22]); +print_r($ct->getEvents()); diff --git a/src/Factory.php b/src/Factory.php index b821128..ac6d98f 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -4,6 +4,7 @@ use SplitIO\ThinSdk\Foundation\Logging; use SplitIO\ThinSdk\Utils\EvalCache; +use SplitIO\ThinSdk\Utils\Tracer; class Factory implements FactoryInterface { @@ -65,7 +66,8 @@ public function client(): ClientInterface $this->linkManager, $this->logger, $uc->impressionListener(), - EvalCache\Helpers::getCache($uc->evaluationCache(), $this->logger) + EvalCache\Helpers::getCache($uc->evaluationCache(), $this->logger), + new Tracer($uc->tracer()->hook(), $uc->tracer()->forwardArgs()), ); } diff --git a/src/Utils/Tracer.php b/src/Utils/Tracer.php index d122919..92a9412 100644 --- a/src/Utils/Tracer.php +++ b/src/Utils/Tracer.php @@ -12,9 +12,9 @@ class Tracer public const EVENT_START = 30; public const EVENT_RPC_START = 31; - public const EVENT_RPC_END = 31; - public const EVENT_END = 32; - public const EVENT_EXCEPTION = 33; + public const EVENT_RPC_END = 32; + public const EVENT_END = 33; + public const EVENT_EXCEPTION = 34; private /*TracerHook*/ $hook; private /*bool*/ $includeArgs; From 096f1036585575c94cea2a1017399402094288b0 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Tue, 14 Nov 2023 20:14:07 -0300 Subject: [PATCH 03/12] more granularity on timeouts --- src/Link/Transfer/ConnectionFactory.php | 22 ++++++++--- src/Version.php | 2 +- tests/Link/Transfer/ConnectionFactoryTest.php | 38 +++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 tests/Link/Transfer/ConnectionFactoryTest.php diff --git a/src/Link/Transfer/ConnectionFactory.php b/src/Link/Transfer/ConnectionFactory.php index 2d57b99..0f0f0ea 100644 --- a/src/Link/Transfer/ConnectionFactory.php +++ b/src/Link/Transfer/ConnectionFactory.php @@ -32,16 +32,26 @@ public function create(): RawConnection throw new \Exception("invalid connection type " . $this->sockType); } - private static function formatTimeout(?int $milliseconds)/*: ?int */ + private static function formatTimeout($timeout)/*: ?int */ { - if ($milliseconds == null) { - $milliseconds = 1000; + if (is_array($timeout)) { + // assume it's a properly formatted unix-like timeout (including 'sec' & 'usec') + if (!array_key_exists('sec', $timeout) || !array_key_exists('usec', $timeout)) { + throw new \Exception("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); + } + return $timeout; + } + + if (!is_null($timeout) && !is_int($timeout)) { + throw new \Exception("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); } + if ($timeout == null || $timeout == 0) { + $timeout = 1000; + } return [ - 'sec' => $milliseconds / 1000, - 'usec' => 0, // TODO(mredolatti): handle seconds fractions in usec units + 'sec' => floor($timeout / 1000), + 'usec' => ($timeout % 1000) * 1000, ]; } - } diff --git a/src/Version.php b/src/Version.php index c9b6dab..a94a7a9 100644 --- a/src/Version.php +++ b/src/Version.php @@ -4,5 +4,5 @@ class Version { - const CURRENT = '1.3.0'; + const CURRENT = '1.3.1'; } diff --git a/tests/Link/Transfer/ConnectionFactoryTest.php b/tests/Link/Transfer/ConnectionFactoryTest.php new file mode 100644 index 0000000..f2270cc --- /dev/null +++ b/tests/Link/Transfer/ConnectionFactoryTest.php @@ -0,0 +1,38 @@ +getMethod('formatTimeout'); + $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, ['sec' =>1, 'usec' => 0])); + $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, 0)); + $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, 1000)); + $this->assertEquals(['sec' => 1, 'usec' => 500000], $m->invoke(null, 1500)); + $this->assertEquals(['sec' => 0, 'usec' => 500000], $m->invoke(null, 500)); + $this->assertEquals(['sec' => 0, 'usec' => 1000], $m->invoke(null, 1)); + } + + public function testInvalidArrayTimeout(): void + { + $c = new \ReflectionClass(ConnectionFactory::class); + $m = $c->getMethod('formatTimeout'); + $this->expectExceptionMessage("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); + $m->invoke(null, []); + } + + public function testNonArrayNonIntTimeout(): void + { + $c = new \ReflectionClass(ConnectionFactory::class); + $m = $c->getMethod('formatTimeout'); + $this->expectExceptionMessage("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); + $m->invoke(null, 98.7); + } + +} From c78dd40c88245d8dc563f4b612ca9f0238b25e87 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Tue, 14 Nov 2023 20:15:17 -0300 Subject: [PATCH 04/12] support sec/usec array for timeouts or int(milliseconds) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index a0505bd..ec9d81f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +1.3.1 (TBD): +- Support finer granularity timeouts and milliseconds granularity + 1.3.0 (Nov 10, 2023): - Added in-memory evaluation cache for the duration of a request. From e9717d6585079940ef204df464abd4a7b05722b0 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 15 Nov 2023 09:19:57 -0300 Subject: [PATCH 05/12] sonarqube suggestions --- src/Utils/NoOpTracerHook.php | 2 +- src/Utils/TracerHook.php | 2 +- tests/Link/Transfer/ConnectionFactoryTest.php | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Utils/NoOpTracerHook.php b/src/Utils/NoOpTracerHook.php index 4c15368..4a9dfb9 100644 --- a/src/Utils/NoOpTracerHook.php +++ b/src/Utils/NoOpTracerHook.php @@ -4,7 +4,7 @@ class NoOpTracerHook implements TracerHook { - function on(int $method, int $event, ?array $args) + public function on(int $method, int $event, ?array $args) { } } diff --git a/src/Utils/TracerHook.php b/src/Utils/TracerHook.php index f158a96..f4aa05b 100644 --- a/src/Utils/TracerHook.php +++ b/src/Utils/TracerHook.php @@ -4,5 +4,5 @@ interface TracerHook { - function on(int $method, int $event, ?array $args); + public function on(int $method, int $event, ?array $args); } diff --git a/tests/Link/Transfer/ConnectionFactoryTest.php b/tests/Link/Transfer/ConnectionFactoryTest.php index f2270cc..c2c868b 100644 --- a/tests/Link/Transfer/ConnectionFactoryTest.php +++ b/tests/Link/Transfer/ConnectionFactoryTest.php @@ -7,10 +7,11 @@ class ConnectionFactoryTest extends TestCase { - public function testHappyExchange(): void + public function testProperTimeouts(): void { $c = new \ReflectionClass(ConnectionFactory::class); $m = $c->getMethod('formatTimeout'); + $m->setAccessible(true); $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, ['sec' =>1, 'usec' => 0])); $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, 0)); $this->assertEquals(['sec' => 1, 'usec' => 0], $m->invoke(null, 1000)); @@ -23,6 +24,7 @@ public function testInvalidArrayTimeout(): void { $c = new \ReflectionClass(ConnectionFactory::class); $m = $c->getMethod('formatTimeout'); + $m->setAccessible(true); $this->expectExceptionMessage("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); $m->invoke(null, []); } @@ -31,6 +33,7 @@ public function testNonArrayNonIntTimeout(): void { $c = new \ReflectionClass(ConnectionFactory::class); $m = $c->getMethod('formatTimeout'); + $m->setAccessible(true); $this->expectExceptionMessage("timeout must either be an int (milliseconds) or an array with keys 'sec' & 'usec'"); $m->invoke(null, 98.7); } From 88a13078399f4286e18e379d79af2278efff7fb4 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Mon, 11 Dec 2023 20:08:58 -0300 Subject: [PATCH 06/12] tracer interface revamp --- src/Client.php | 69 ++++++---- src/Config/Tracer.php | 2 +- src/Factory.php | 2 +- src/Utils/NoOpTracerHook.php | 10 -- src/Utils/TracerHook.php | 8 -- src/Utils/Tracing/NoOpTracerHook.php | 10 ++ src/Utils/{ => Tracing}/Tracer.php | 11 +- src/Utils/Tracing/TracerHook.php | 8 ++ src/Utils/Tracing/TracingEventFactory.php | 43 ++++++ tests/ClientTest.php | 124 ++++++++++++++---- tests/Config/TraceTest.php | 2 +- tests/Config/UtilsTest.php | 2 +- .../Utils/Tracing/TracingEventFactoryTest.php | 40 ++++++ 13 files changed, 248 insertions(+), 83 deletions(-) delete mode 100644 src/Utils/NoOpTracerHook.php delete mode 100644 src/Utils/TracerHook.php create mode 100644 src/Utils/Tracing/NoOpTracerHook.php rename src/Utils/{ => Tracing}/Tracer.php (81%) create mode 100644 src/Utils/Tracing/TracerHook.php create mode 100644 src/Utils/Tracing/TracingEventFactory.php create mode 100644 tests/Utils/Tracing/TracingEventFactoryTest.php diff --git a/src/Client.php b/src/Client.php index 20dbcee..d1a5d3c 100644 --- a/src/Client.php +++ b/src/Client.php @@ -3,8 +3,9 @@ namespace SplitIO\ThinSdk; use \SplitIO\ThinSdk\Utils\ImpressionListener; -use \SplitIO\ThinSdk\Utils\Tracer; -use \SplitIO\ThinSdk\Utils\NoOpTracerHook; +use \SplitIO\ThinSdk\Utils\Tracing\TracingEventFactory as TEF; +use \SplitIO\ThinSdk\Utils\Tracing\Tracer; +use \SplitIO\ThinSdk\Utils\Tracing\NoOpTracerHook; use \SplitIO\ThinSdk\Utils\EvalCache\Cache; use \SplitIO\ThinSdk\Utils\EvalCache\NoCache; use \SplitIO\ThinSdk\Utils\InputValidator\InputValidator; @@ -37,30 +38,34 @@ public function __construct(Manager $manager, LoggerInterface $logger, ?Impressi public function getTreatment(string $key, ?string $bucketingKey, string $feature, ?array $attributes = null): string { try { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); + $id = $this->tracer->makeId(); + $method = Tracer::METHOD_GET_TREATMENT; + $this->tracer->trace(TEF::forStart($method, $id, $this->tracer->includeArgs() ? func_get_args() : [])); if (($fromCache = $this->cache->get($key, $feature, $attributes)) != null) { return $fromCache; } - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_START, null); + $this->tracer->trace(TEF::forRPCStart($method, $id)); list($treatment, $ilData) = $this->lm->getTreatment($key, $bucketingKey, $feature, $attributes); - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_END, null); + $this->tracer->trace(TEF::forRPCEnd($method, $id)); $this->handleListener($key, $bucketingKey, $feature, $attributes, $treatment, $ilData); $this->cache->set($key, $feature, $attributes, $treatment); return $treatment; } catch (\Exception $exc) { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error($exc); return "control"; } finally { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_END, null); + $this->tracer->trace(TEF::forEnd($method, $id)); } } public function getTreatments(string $key, ?string $bucketingKey, array $features, ?array $attributes = null): array { try { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); + $id = $this->tracer->makeId(); + $method = Tracer::METHOD_GET_TREATMENTS; + $this->tracer->trace(TEF::forStart($method, $id, $this->tracer->includeArgs() ? func_get_args() : [])); // try to fetch items from cache. return result if all evaluations are cached // otherwise, send a Treatments RPC for missing ones and return merged result $toReturn = $this->cache->getMany($key, $features, $attributes); @@ -69,9 +74,9 @@ public function getTreatments(string $key, ?string $bucketingKey, array $feature return $toReturn; } - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_START, null); + $this->tracer->trace(TEF::forRPCStart($method, $id)); $results = $this->lm->getTreatments($key, $bucketingKey, $features, $attributes); - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_END, null); + $this->tracer->trace(TEF::forRPCEnd($method, $id)); foreach ($results as $feature => $result) { list($treatment, $ilData) = $result; $toReturn[$feature] = $treatment; @@ -80,44 +85,48 @@ public function getTreatments(string $key, ?string $bucketingKey, array $feature $this->cache->setMany($key, $attributes, $toReturn); return $toReturn; } catch (\Exception $exc) { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error($exc); return array_reduce($features, function ($r, $k) { $r[$k] = "control"; return $r; }, []); } finally { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_END, null); + $this->tracer->trace(TEF::forEnd($method, $id)); } } public function getTreatmentWithConfig(string $key, ?string $bucketingKey, string $feature, ?array $attributes = null): array { try { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); + $id = $this->tracer->makeId(); + $method = Tracer::METHOD_GET_TREATMENT_WITH_CONFIG; + $this->tracer->trace(TEF::forStart($method, $id, $this->tracer->includeArgs() ? func_get_args() : [])); if (($fromCache = $this->cache->getWithConfig($key, $feature, $attributes)) != null) { return $fromCache; } - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_START, null); + $this->tracer->trace(TEF::forRPCStart($method, $id)); list($treatment, $ilData, $config) = $this->lm->getTreatmentWithConfig($key, $bucketingKey, $feature, $attributes); - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_END, null); + $this->tracer->trace(TEF::forRPCEnd($method, $id)); $this->handleListener($key, $bucketingKey, $feature, $attributes, $treatment, $ilData); $this->cache->setWithConfig($key, $feature, $attributes, $treatment, $config); return ['treatment' => $treatment, 'config' => $config]; } catch (\Exception $exc) { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error($exc); - return "control"; + return ['treatment' => "control", 'config' => null]; } finally { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_END, null); + $this->tracer->trace(TEF::forEnd($method, $id)); } } public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, array $features, ?array $attributes = null): array { try { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); + $id = $this->tracer->makeId(); + $method = Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG; + $this->tracer->trace(TEF::forStart($method, $id, $this->tracer->includeArgs() ? func_get_args() : [])); $toReturn = $this->cache->getManyWithConfig($key, $features, $attributes); $features = self::getMissing($toReturn); @@ -125,9 +134,9 @@ public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, arra return $toReturn; } - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_START, null); + $this->tracer->trace(TEF::forRPCStart($method, $id)); $results = $this->lm->getTreatmentsWithConfig($key, $bucketingKey, $features, $attributes); - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_END, null); + $this->tracer->trace(TEF::forRPCEnd($method, $id)); foreach ($results as $feature => $result) { list($treatment, $ilData, $config) = $result; $toReturn[$feature] = ['treatment' => $treatment, 'config' => $config]; @@ -136,34 +145,36 @@ public function getTreatmentsWithConfig(string $key, ?string $bucketingKey, arra $this->cache->setManyWithConfig($key, $attributes, $toReturn); return $toReturn; } catch (\Exception $exc) { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error($exc); return array_reduce($features, function ($r, $k) { $r[$k] = ['treatment' => 'control', 'config' => null]; return $r; }, []); } finally { - $this->tracer->trace(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_END, null); + $this->tracer->trace(TEF::forEnd($method, $id)); } } public function track(string $key, string $trafficType, string $eventType, ?float $value = null, ?array $properties = null): bool { try { - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_START, $this->tracer->includeArgs() ? func_get_args() : []); + $id = $this->tracer->makeId(); + $method = Tracer::METHOD_TRACK; + $this->tracer->trace(TEF::forStart($method, $id, $this->tracer->includeArgs() ? func_get_args() : [])); $properties = $this->inputValidator->validProperties($properties); - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_RPC_START, null); + $this->tracer->trace(TEF::forRPCStart($method, $id)); $res = $this->lm->track($key, $trafficType, $eventType, $value, $properties); - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_RPC_END, null); + $this->tracer->trace(TEF::forRPCEnd($method, $id)); return $res; } catch (ValidationException $exc) { - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error("error validating event properties: " . $exc->getMessage()); } catch (\Exception $exc) { - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_EXCEPTION, null); + $this->tracer->trace(TEF::forException($method, $id, $exc)); $this->logger->error($exc); } finally { - $this->tracer->trace(Tracer::METHOD_TRACK, Tracer::EVENT_END, null); + $this->tracer->trace(TEF::forEnd($method, $id)); } return false; } diff --git a/src/Config/Tracer.php b/src/Config/Tracer.php index 78db156..82bc4e6 100644 --- a/src/Config/Tracer.php +++ b/src/Config/Tracer.php @@ -2,7 +2,7 @@ namespace SplitIO\ThinSdk\Config; -use SplitIO\ThinSdk\Utils\TracerHook; +use SplitIO\ThinSdk\Utils\Tracing\TracerHook; class Tracer diff --git a/src/Factory.php b/src/Factory.php index ac6d98f..aca0fbf 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -4,7 +4,7 @@ use SplitIO\ThinSdk\Foundation\Logging; use SplitIO\ThinSdk\Utils\EvalCache; -use SplitIO\ThinSdk\Utils\Tracer; +use SplitIO\ThinSdk\Utils\Tracing\Tracer; class Factory implements FactoryInterface { diff --git a/src/Utils/NoOpTracerHook.php b/src/Utils/NoOpTracerHook.php deleted file mode 100644 index 4c15368..0000000 --- a/src/Utils/NoOpTracerHook.php +++ /dev/null @@ -1,10 +0,0 @@ -includeArgs; } - public function trace(int $method, int $event, ?array $args) + public function trace(array $event) { - $this->hook->on($method, $event, $args); + $this->hook->on($event); + } + + public function makeId(): string + { + return uniqid(); } } diff --git a/src/Utils/Tracing/TracerHook.php b/src/Utils/Tracing/TracerHook.php new file mode 100644 index 0000000..405399f --- /dev/null +++ b/src/Utils/Tracing/TracerHook.php @@ -0,0 +1,8 @@ + $id, 'method' => $method, 'event' => Tracer::EVENT_START]; + return is_null($arguments) + ? $base + : array_merge($base, ['arguments' => $arguments]); + } + + public static function forRPCStart(int $method, string $id): array + { + return ['id' => $id, 'method' => $method, 'event' => Tracer::EVENT_RPC_START]; + } + + public static function forRPCEnd(int $method, string $id): array + { + return ['id' => $id, 'method' => $method, 'event' => Tracer::EVENT_RPC_END]; + } + + public static function forException(int $method, string $id, \Exception $exception): array + { + return [ + 'id' => $id, + 'method' => $method, + 'event' => Tracer::EVENT_EXCEPTION, + 'exception' => $exception, + ]; + } + + public static function forEnd(int $method, string $id): array + { + return [ + 'id' => $id, + 'method' => $method, + 'event' => Tracer::EVENT_END, + ]; + } +} diff --git a/tests/ClientTest.php b/tests/ClientTest.php index b91d12f..c6e9838 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -4,7 +4,7 @@ use SplitIO\ThinSdk\Client; use SplitIO\ThinSdk\Utils\ImpressionListener; -use SplitIO\ThinSdk\Utils\Tracer; +use SplitIO\ThinSdk\Utils\Tracing\Tracer; use SplitIO\ThinSdk\Utils\EvalCache\CacheImpl; use SplitIO\ThinSdk\Utils\EvalCache\KeyAttributeCRC32Hasher; use SplitIO\ThinSdk\Utils\EvalCache\NoEviction; @@ -33,15 +33,33 @@ public function testGetTreatmentNoImpListener() ->willReturn(['on', null, null]); $tracer = $this->createMock(Tracer::class); - $tracer->expects($this->once()) - ->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('makeId')->willReturn('some_id'); + $tracer->expects($this->exactly(4)) ->method('trace') ->withConsecutive( - [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_START, ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]]], - [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_START, null], - [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_RPC_END, null], - [Tracer::METHOD_GET_TREATMENT, Tracer::EVENT_END, null], + [[ + 'id' => 'some_id', + 'method' => Tracer::METHOD_GET_TREATMENT, + 'event' => Tracer::EVENT_START, + 'arguments' => ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]], + ]], + [[ + 'id' => 'some_id', + 'method' => Tracer::METHOD_GET_TREATMENT, + 'event' => Tracer::EVENT_RPC_START, + ]], + [[ + 'id' => 'some_id', + 'method' => Tracer::METHOD_GET_TREATMENT, + 'event' => Tracer::EVENT_RPC_END, + ]], + [[ + 'id' => 'some_id', + 'method' => Tracer::METHOD_GET_TREATMENT, + 'event' => Tracer::EVENT_END, + ]], ); $client = new Client($manager, $this->logger, null, null, $tracer); @@ -92,17 +110,35 @@ public function testGetTreatmentsWithImpListener() ]); $tracer = $this->createMock(Tracer::class); - $tracer->expects($this->once()) - ->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('makeId')->willReturn('some_id2'); $tracer->expects($this->exactly(4)) ->method('trace') ->withConsecutive( - [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_START, ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]]], - [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_START, null], - [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_RPC_END, null], - [Tracer::METHOD_GET_TREATMENTS, Tracer::EVENT_END, null], + [[ + 'id' => 'some_id2', + 'method' => Tracer::METHOD_GET_TREATMENTS, + 'event' => Tracer::EVENT_START, + 'arguments' => ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]], + ]], + [[ + 'id' => 'some_id2', + 'method' => Tracer::METHOD_GET_TREATMENTS, + 'event' => Tracer::EVENT_RPC_START, + ]], + [[ + 'id' => 'some_id2', + 'method' => Tracer::METHOD_GET_TREATMENTS, + 'event' => Tracer::EVENT_RPC_END, + ]], + [[ + 'id' => 'some_id2', + 'method' => Tracer::METHOD_GET_TREATMENTS, + 'event' => Tracer::EVENT_END, + ]] ); + $ilMock = $this->createMock(ImpressionListener::class); $ilMock->expects($this->exactly(3)) ->method('accept') @@ -133,15 +169,32 @@ public function testGetTreatmentWithConfigAndListener() ->with(new Impression('someKey', 'someBuck', 'someFeature', 'on', 'lab1', 123, 123456), ['someAttr' => 123]); $tracer = $this->createMock(Tracer::class); - $tracer->expects($this->once()) - ->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('makeId')->willReturn('some_id3'); $tracer->expects($this->exactly(4)) ->method('trace') ->withConsecutive( - [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_START, ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]]], - [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_START, null], - [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_RPC_END, null], - [Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, Tracer::EVENT_END, null], + [[ + 'id' => 'some_id3', + 'method' => Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, + 'event' => Tracer::EVENT_START, + 'arguments' => ['someKey', 'someBuck', 'someFeature', ['someAttr' => 123]], + ]], + [[ + 'id' => 'some_id3', + 'method' => Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, + 'event' => Tracer::EVENT_RPC_START, + ]], + [[ + 'id' => 'some_id3', + 'method' => Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, + 'event' => Tracer::EVENT_RPC_END, + ]], + [[ + 'id' => 'some_id3', + 'method' => Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, + 'event' => Tracer::EVENT_END, + ]], ); $client = new Client($manager, $this->logger, $ilMock, null, $tracer); @@ -172,19 +225,32 @@ public function testGetTreatmentsWithConfigAndListener() ); $tracer = $this->createMock(Tracer::class); - $tracer->expects($this->once()) - ->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('includeArgs')->willReturn(true); + $tracer->expects($this->once())->method('makeId')->willReturn('some_id4'); $tracer->expects($this->exactly(4)) ->method('trace') ->withConsecutive( - [ - Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, - Tracer::EVENT_START, - ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]], - ], - [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_START, null], - [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_RPC_END, null], - [Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, Tracer::EVENT_END, null], + [[ + 'id' => 'some_id4', + 'method' => Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, + 'event' => Tracer::EVENT_START, + 'arguments' => ['someKey', 'someBuck', ['someFeature1', 'someFeature2', 'someFeature3'], ['someAttr' => 123]], + ]], + [[ + 'id' => 'some_id4', + 'method' => Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, + 'event' => Tracer::EVENT_RPC_START, + ]], + [[ + 'id' => 'some_id4', + 'method' => Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, + 'event' => Tracer::EVENT_RPC_END, + ]], + [[ + 'id' => 'some_id4', + 'method' => Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, + 'event' => Tracer::EVENT_END, + ]] ); $client = new Client($manager, $this->logger, $ilMock, null, $tracer); diff --git a/tests/Config/TraceTest.php b/tests/Config/TraceTest.php index 6a2341e..e187422 100644 --- a/tests/Config/TraceTest.php +++ b/tests/Config/TraceTest.php @@ -3,7 +3,7 @@ namespace SplitIO\Test\Link\Consumer; use SplitIO\ThinSdk\Config\Tracer; -use SplitIO\ThinSdk\Utils\TracerHook; +use SplitIO\ThinSdk\Utils\Tracing\TracerHook; use PHPUnit\Framework\TestCase; diff --git a/tests/Config/UtilsTest.php b/tests/Config/UtilsTest.php index 3619dac..b033d6a 100644 --- a/tests/Config/UtilsTest.php +++ b/tests/Config/UtilsTest.php @@ -7,7 +7,7 @@ use SplitIO\ThinSdk\Config\Tracer; use SplitIO\ThinSdk\Utils\ImpressionListener; use SplitIO\ThinSdk\Utils\EvalCache\InputHasher; -use SplitIO\ThinSdk\Utils\TracerHook; +use SplitIO\ThinSdk\Utils\Tracing\TracerHook; use PHPUnit\Framework\TestCase; diff --git a/tests/Utils/Tracing/TracingEventFactoryTest.php b/tests/Utils/Tracing/TracingEventFactoryTest.php new file mode 100644 index 0000000..43d0a96 --- /dev/null +++ b/tests/Utils/Tracing/TracingEventFactoryTest.php @@ -0,0 +1,40 @@ +assertEquals( + ['id' => '123', 'method' => Tracer::METHOD_GET_TREATMENT, 'event' => Tracer::EVENT_START, 'arguments' => ['a', 'b', 'c']], + TEF::forStart(Tracer::METHOD_GET_TREATMENT, '123', ['a', 'b', 'c']), + ); + + $this->assertEquals( + ['id' => '123', 'method' => Tracer::METHOD_GET_TREATMENTS, 'event' => Tracer::EVENT_RPC_START], + TEF::forRPCStart(Tracer::METHOD_GET_TREATMENTS, '123'), + ); + + $this->assertEquals( + ['id' => '123', 'method' => Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, 'event' => Tracer::EVENT_RPC_END], + TEF::forRPCEnd(Tracer::METHOD_GET_TREATMENT_WITH_CONFIG, '123'), + ); + + $this->assertEquals( + ['id' => '123', 'method' => Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, 'event' => Tracer::EVENT_END], + TEF::forEnd(Tracer::METHOD_GET_TREATMENTS_WITH_CONFIG, '123'), + ); + + $this->assertEquals( + ['id' => '123', 'method' => Tracer::METHOD_TRACK, 'event' => Tracer::EVENT_EXCEPTION, 'exception' => new \Exception("sarasa")], + TEF::forException(Tracer::METHOD_TRACK, '123', new \Exception("sarasa")), + ); + } +} From afb6991e677f6d643450387458ac8d68b3cdf43b Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 13 Dec 2023 12:20:04 -0300 Subject: [PATCH 07/12] use more entropy for uniqid in traces --- src/Utils/Tracing/Tracer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/Tracing/Tracer.php b/src/Utils/Tracing/Tracer.php index 5c12979..bf530a5 100644 --- a/src/Utils/Tracing/Tracer.php +++ b/src/Utils/Tracing/Tracer.php @@ -37,6 +37,6 @@ public function trace(array $event) public function makeId(): string { - return uniqid(); + return uniqid("", true); } } From b55f35405e2d3773b1963b29f2ab03b32e04b975 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 13 Dec 2023 14:54:39 -0300 Subject: [PATCH 08/12] tracer cleanup --- CHANGES | 5 +++-- examples/tracer.php | 14 +++++++------- src/Config/Utils.php | 2 +- tests/Config/UtilsTest.php | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index ec9d81f..df11509 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,6 @@ -1.3.1 (TBD): -- Support finer granularity timeouts and milliseconds granularity +1.4.0 (TBD): +- Add support for a custom tracer for client methods. +- Support finer granularity timeouts and milliseconds granularity. 1.3.0 (Nov 10, 2023): - Added in-memory evaluation cache for the duration of a request. diff --git a/examples/tracer.php b/examples/tracer.php index fc0cd2c..877eda7 100644 --- a/examples/tracer.php +++ b/examples/tracer.php @@ -3,24 +3,24 @@ require_once '../vendor/autoload.php'; use \SplitIO\ThinSdk\Factory; -use \SplitIO\ThinSdk\Utils\Tracer; -use \SplitIO\ThinSdk\Utils\TracerHook; +use \SplitIO\ThinSdk\Utils\Tracing\Tracer; +use \SplitIO\ThinSdk\Utils\Tracing\TracerHook; class CustomTracer implements TracerHook { private $events = []; - public function on(int $method, int $event, ?array $arguments) + public function on(array $event) { // assume we only care about getTreatment() calls... - if ($method != Tracer::METHOD_GET_TREATMENT) { + if ($event['method'] != Tracer::METHOD_GET_TREATMENT) { return; } - switch ($event) { + switch ($event['event']) { case Tracer::EVENT_START: - array_push($this->events, "start (" . json_encode($arguments) . ") -- " . microtime(true)); + array_push($this->events, "start (" . json_encode($event['arguments']) . ") -- " . microtime(true)); break; case Tracer::EVENT_RPC_START: array_push($this->events, "about to send rpc -- " . microtime(true)); @@ -54,7 +54,7 @@ public function getEvents(): array 'level' => \Psr\Log\LogLevel::INFO, ], 'utils' => [ - '__tracer' => [ + 'tracer' => [ 'hook' => $ct, 'forwardArgs' => true, ] diff --git a/src/Config/Utils.php b/src/Config/Utils.php index d575830..89be7e0 100644 --- a/src/Config/Utils.php +++ b/src/Config/Utils.php @@ -39,7 +39,7 @@ public static function fromArray(array $config): Utils return new Utils( $config['impressionListener'] ?? $d->impressionListener(), EvaluationCache::fromArray($config['evaluationCache'] ?? []), - Tracer::fromArray($config['__tracer'] ?? []), + Tracer::fromArray($config['tracer'] ?? []), ); } diff --git a/tests/Config/UtilsTest.php b/tests/Config/UtilsTest.php index b033d6a..674cfd1 100644 --- a/tests/Config/UtilsTest.php +++ b/tests/Config/UtilsTest.php @@ -35,7 +35,7 @@ public function testConfigParsing() 'maxSize' => 1234, 'customHash' => $ihMock, ], - '__tracer' => [ + 'tracer' => [ 'hook' => $tMock, 'forwardArgs' => true, ], From 189c5d7c48a67edd057e7c22a622e5105e58c68c Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 13 Dec 2023 15:11:20 -0300 Subject: [PATCH 09/12] sq feedback --- src/Utils/Tracing/NoOpTracerHook.php | 2 +- src/Version.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Utils/Tracing/NoOpTracerHook.php b/src/Utils/Tracing/NoOpTracerHook.php index 33265aa..674bfa6 100644 --- a/src/Utils/Tracing/NoOpTracerHook.php +++ b/src/Utils/Tracing/NoOpTracerHook.php @@ -4,7 +4,7 @@ class NoOpTracerHook implements TracerHook { - function on(array $event) + public function on(array $event) { } } diff --git a/src/Version.php b/src/Version.php index a94a7a9..d4ff141 100644 --- a/src/Version.php +++ b/src/Version.php @@ -4,5 +4,5 @@ class Version { - const CURRENT = '1.3.1'; + const CURRENT = '1.4.0'; } From 34644bfb739ca6e8973f167891833e4a7bae5fea Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 13 Dec 2023 18:47:17 -0300 Subject: [PATCH 10/12] set relesae date --- CHANGES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index df11509..8d9c850 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,4 @@ -1.4.0 (TBD): +1.4.0 (Dec 13, 2023): - Add support for a custom tracer for client methods. - Support finer granularity timeouts and milliseconds granularity. From d258f9f743d544048b4adac0f273be76605db4d6 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Thu, 14 Dec 2023 11:22:05 -0300 Subject: [PATCH 11/12] better tracer example --- examples/tracer.php | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/examples/tracer.php b/examples/tracer.php index 877eda7..973b970 100644 --- a/examples/tracer.php +++ b/examples/tracer.php @@ -9,37 +9,43 @@ class CustomTracer implements TracerHook { - private $events = []; + private $traces = []; + // assume we only care about getTreatment() calls... public function on(array $event) { - // assume we only care about getTreatment() calls... + if ($event['method'] != Tracer::METHOD_GET_TREATMENT) { return; } + $trace = $this->traces[$event['id']] ?? []; + switch ($event['event']) { case Tracer::EVENT_START: - array_push($this->events, "start (" . json_encode($event['arguments']) . ") -- " . microtime(true)); + $trace['start'] = microtime(true); + $trace['args'] = $event['arguments']; break; case Tracer::EVENT_RPC_START: - array_push($this->events, "about to send rpc -- " . microtime(true)); + $trace['rpc_start'] = microtime(true); break; case Tracer::EVENT_RPC_END: - array_push($this->events, "rpc completed -- " . microtime(true)); + $trace['rpc_end'] = microtime(true); break; case Tracer::EVENT_EXCEPTION: - array_push($this->events, "exception occured -- " . microtime(true)); + $trace['exception'] = $event['exception']; break; case Tracer::EVENT_END: - array_push($this->events, "end -- " . microtime(true)); + $trace['end'] = microtime(true); break; } + + $this->traces[$event['id']] = $trace; } - public function getEvents(): array + public function getTraces(): array { - return $this->events; + return $this->traces; } } @@ -65,4 +71,4 @@ public function getEvents(): array $manager = $factory->manager(); $client = $factory->client(); echo $client->getTreatment("key", null, $manager->splitNames()[0], ['age' => 22]); -print_r($ct->getEvents()); +var_dump($ct->getTraces()); From 05c465084c2ef8062cc82614ca44545b177eb2e0 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Thu, 14 Dec 2023 11:32:08 -0300 Subject: [PATCH 12/12] prepare for release --- CHANGES | 4 ++-- examples/tracer.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 8d9c850..50fff84 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,6 @@ -1.4.0 (Dec 13, 2023): +1.4.0 (Dec 14, 2023): - Add support for a custom tracer for client methods. -- Support finer granularity timeouts and milliseconds granularity. +- Support finer granularity on timeouts. 1.3.0 (Nov 10, 2023): - Added in-memory evaluation cache for the duration of a request. diff --git a/examples/tracer.php b/examples/tracer.php index 973b970..b2fc176 100644 --- a/examples/tracer.php +++ b/examples/tracer.php @@ -11,10 +11,10 @@ class CustomTracer implements TracerHook private $traces = []; - // assume we only care about getTreatment() calls... public function on(array $event) { + // assume we only care about getTreatment() calls... if ($event['method'] != Tracer::METHOD_GET_TREATMENT) { return; }