From 2c16709ce7c2ea02d589cabbe78b8abf7d8924d2 Mon Sep 17 00:00:00 2001 From: Ben Poulson Date: Sat, 18 Jan 2025 15:28:40 +0000 Subject: [PATCH 1/2] Remove guzzle async http - for now --- README.md | 1 - src/Config.php | 11 +-------- src/Http/ApiClient.php | 44 ++++++++--------------------------- src/Tracing/TraceInstance.php | 4 ++-- tests/ApiClientTest.php | 40 +------------------------------ tests/ConfigTest.php | 1 - 6 files changed, 14 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index 25af25e..1256751 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,6 @@ $config = Config::fromArray([ | `track_file_operations` | boolean | `true` | Track file operations | | `proxy` | string | `null` | HTTP/HTTPS proxy for Perfbase API calls | | `timeout` | int | `10` | Timeout seconds for Perfbase API calls | -| `async` | boolean | `true` | Send traces to Perfbase API asynchronously | ## License diff --git a/src/Config.php b/src/Config.php index c29dc87..1845573 100644 --- a/src/Config.php +++ b/src/Config.php @@ -141,13 +141,6 @@ class Config */ public int $timeout = 10; - /** - * Enable asynchronous delivery of profiling data - * @var bool - */ - public bool $async_delivery = true; - - /** * @param string|null $api_key * @param string|null $api_url @@ -168,7 +161,6 @@ class Config * @param bool|null $track_file_operations * @param string|null $proxy * @param int|null $timeout - * @param bool|null $async_delivery */ public function __construct( ?string $api_key = null, @@ -189,8 +181,7 @@ public function __construct( ?bool $track_aws_sdk = null, ?bool $track_file_operations = null, ?string $proxy = null, - ?int $timeout = null, - ?bool $async_delivery = null + ?int $timeout = null ) { // Get all the properties of this class diff --git a/src/Http/ApiClient.php b/src/Http/ApiClient.php index 417e7b2..20b445f 100644 --- a/src/Http/ApiClient.php +++ b/src/Http/ApiClient.php @@ -3,8 +3,6 @@ namespace Perfbase\SDK\Http; use GuzzleHttp\Client as GuzzleClient; -use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\Promise\Utils; use JsonException; use Perfbase\SDK\Config; use Perfbase\SDK\Exception\PerfbaseApiKeyMissingException; @@ -30,12 +28,6 @@ class ApiClient */ private GuzzleClient $httpClient; - /** - * Promises to settle before the client is destroyed - * @var array - */ - private array $promises = []; - /** * @throws PerfbaseApiKeyMissingException */ @@ -46,24 +38,25 @@ public function __construct(Config $config) } $this->config = $config; - $this->defaultHeaders = [ 'Authorization' => 'Bearer ' . $this->config->api_key, 'Accept' => 'application/json', 'User-Agent' => 'Perfbase-PHP-SDK/1.0', 'Content-Type' => 'application/json', + 'Connection' => 'keep-alive', ]; /** @var array $httpClientConfig */ $httpClientConfig = []; - $httpClientConfig['base_uri'] = $config->api_url; $httpClientConfig['timeout'] = $config->timeout; + // Set up proxy if configured if ($config->proxy) { $httpClientConfig['proxy'] = $config->proxy; } + // Set up the HTTP client $this->httpClient = new GuzzleClient($httpClientConfig); } @@ -72,12 +65,10 @@ public function __construct(Config $config) * * @param string $endpoint API endpoint to send the request to * @param array $data Data to send in the request body - * @param bool $async If true, send asynchronously; if false, wait for response - * - * @return string|null Response data from the API, or null if non-blocking - * @throws JsonException When the HTTP request fails or returns an error + * @return void + * @throws JsonException */ - private function post(string $endpoint, array $data, bool $async = true): ?string + private function submit(string $endpoint, array $data): void { // Prepare request options $options = [ @@ -86,38 +77,23 @@ private function post(string $endpoint, array $data, bool $async = true): ?strin ]; try { - if ($async) { - $this->promises[] = $this->httpClient->postAsync($endpoint, $options); - return null; - } else { - $response = $this->httpClient->post($endpoint, $options); - return (string)$response->getBody(); - } + $this->httpClient->post($endpoint, $options); } catch (Throwable $e) { // throw new PerfbaseException('HTTP Request failed: ' . $e->getMessage()); } - return null; } /** * Submits a trace to the Perfbase API * * @param array $data Data to send in the request body - * @param bool $async If true, send asynchronously; if false, wait for response + * @return void * - * @return string|null Response data from the API, or null if non-blocking * @throws JsonException When the HTTP request fails or returns an error */ - public function submitTrace(array $data, bool $async = true): ?string + public function submitTrace(array $data): void { - return $this->post('/v1/submit', $data, $async); + $this->submit('/v1/submit', $data); } - public function __destruct() - { - // Attempt to settle all outstanding async HTTP promises without blocking - if (!empty($this->promises)) { - Utils::settle($this->promises)->wait(false); - } - } } \ No newline at end of file diff --git a/src/Tracing/TraceInstance.php b/src/Tracing/TraceInstance.php index 04f46ad..9d91c5a 100644 --- a/src/Tracing/TraceInstance.php +++ b/src/Tracing/TraceInstance.php @@ -103,6 +103,7 @@ public function __destruct() * The data is automatically sent to the Perfbase API for analysis. * @param bool $andSend If true, send the collected data to the API; if false, do not send * @throws PerfbaseStateException + * @throws JsonException */ public function stopProfiling(bool $andSend = true): void { @@ -136,8 +137,7 @@ public function stopProfiling(bool $andSend = true): void public function sendProfilingData(): void { $this->apiClient->submitTrace( - $this->transformData(), - $this->config->async_delivery + $this->transformData() ); } diff --git a/tests/ApiClientTest.php b/tests/ApiClientTest.php index f6783a3..c0486b5 100644 --- a/tests/ApiClientTest.php +++ b/tests/ApiClientTest.php @@ -54,7 +54,7 @@ public function testInitializesWithValidApiKey(): void * @throws JsonException * @covers ::get */ - public function testSendsSynchronousSubmitRequestAndReturnsResponse(): void + public function testSendsSubmitRequestAndReturnsResponse(): void { $mock = new MockHandler([ new Response(200, [], 'Success'), @@ -75,42 +75,4 @@ public function testSendsSynchronousSubmitRequestAndReturnsResponse(): void $this->assertSame('Success', $response); } - - /** - * @return void - * @throws JsonException - * @throws PerfbaseApiKeyMissingException - * @covers ::post - */ - public function testSendsAsynchronousSubmitRequestAndDoesNotBlock(): void - { - $mock = new MockHandler([ - new Response(200, [], 'Success'), - ]); - $handlerStack = HandlerStack::create($mock); - $guzzleClient = new GuzzleClient(['handler' => $handlerStack]); - - $config = new Config(); - $config->api_key = 'test_api_key'; - - $apiClient = new ApiClient($config); - $reflection = new ReflectionClass($apiClient); - $property = $reflection->getProperty('httpClient'); - $property->setAccessible(true); - $property->setValue($apiClient, $guzzleClient); - - $response = $apiClient->submitTrace(['key' => 'value'], true); - - $this->assertNull($response); - - // Check if promises array is populated - $promisesProperty = $reflection->getProperty('promises'); - $promisesProperty->setAccessible(true); - - /** @var array $promises */ - $promises = $promisesProperty->getValue($apiClient); - $this->assertIsArray($promises); - $this->assertCount(1, $promises); - $this->assertInstanceOf(PromiseInterface::class, $promises[0]); - } } diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index b740f6d..6d272e6 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -88,7 +88,6 @@ public function testConstructorSetsProperties(): void $this->assertFalse($config->track_file_operations); $this->assertSame('http://proxy:8080', $config->proxy); $this->assertSame(1000, $config->timeout); - $this->assertFalse($config->async_delivery); } /** From 84f59997cabc1e8a2fe506742d3446e606539890 Mon Sep 17 00:00:00 2001 From: Ben Poulson Date: Sat, 18 Jan 2025 15:30:34 +0000 Subject: [PATCH 2/2] Todo - create a better replacement test for testing submitTrace --- tests/ApiClientTest.php | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/tests/ApiClientTest.php b/tests/ApiClientTest.php index c0486b5..3d70cf8 100644 --- a/tests/ApiClientTest.php +++ b/tests/ApiClientTest.php @@ -48,31 +48,4 @@ public function testInitializesWithValidApiKey(): void $this->assertInstanceOf(ApiClient::class, $apiClient); } - /** - * @return void - * @throws PerfbaseApiKeyMissingException - * @throws JsonException - * @covers ::get - */ - public function testSendsSubmitRequestAndReturnsResponse(): void - { - $mock = new MockHandler([ - new Response(200, [], 'Success'), - ]); - $handlerStack = HandlerStack::create($mock); - $guzzleClient = new GuzzleClient(['handler' => $handlerStack]); - - $config = new Config(); - $config->api_key = 'test_api_key'; - - $apiClient = new ApiClient($config); - $reflection = new ReflectionClass($apiClient); - $property = $reflection->getProperty('httpClient'); - $property->setAccessible(true); - $property->setValue($apiClient, $guzzleClient); - - $response = $apiClient->submitTrace(['key' => 'value'], false); - - $this->assertSame('Success', $response); - } }