From b6c692304c411b8dfdad4c30342cab02934c936a Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Mon, 14 Apr 2025 16:03:20 +0200 Subject: [PATCH 1/6] Add rate-limiter to forwarder --- src/Envelope.php | 47 +++++++++++++++++++++++++++++++++++++++ src/EnvelopeForwarder.php | 36 +++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/Envelope.php b/src/Envelope.php index a80af1f..0ca7bf6 100644 --- a/src/Envelope.php +++ b/src/Envelope.php @@ -4,6 +4,8 @@ namespace Sentry\Agent; +use Sentry\EventType; + /** * @internal */ @@ -36,6 +38,34 @@ public function getDsn(): ?string return null; } + public function getFirstItemType(): ?EventType + { + $header = $this->getFirstItemHeader(); + + if ($header === null) { + return null; + } + + $parsedHeader = json_decode($header, true); + + $type = null; + + if (\is_array($parsedHeader) && !empty($parsedHeader['type']) && \is_string($parsedHeader['type'])) { + $type = $parsedHeader['type']; + } + + switch ($type) { + case (string) EventType::event(): + return EventType::event(); + case (string) EventType::transaction(): + return EventType::transaction(); + case (string) EventType::checkIn(): + return EventType::checkIn(); + default: + return null; + } + } + public function getData(): string { return $this->data; @@ -51,4 +81,21 @@ public function getHeader(): ?string return substr($this->data, 0, $position); } + + public function getFirstItemHeader(): ?string + { + $headerEndsAt = strpos($this->data, "\n"); + + if ($headerEndsAt === false) { + return null; + } + + $itemHeaderEndsAt = strpos($this->data, "\n", $headerEndsAt + 1); + + if ($itemHeaderEndsAt === false) { + return null; + } + + return substr($this->data, $headerEndsAt + 1, $itemHeaderEndsAt - $headerEndsAt - 1); + } } diff --git a/src/EnvelopeForwarder.php b/src/EnvelopeForwarder.php index ca9260a..27c59cf 100644 --- a/src/EnvelopeForwarder.php +++ b/src/EnvelopeForwarder.php @@ -8,6 +8,8 @@ use React\Http\Browser; use React\Promise\PromiseInterface; use Sentry\Dsn; +use Sentry\HttpClient\Response; +use Sentry\Transport\RateLimiter; /** * @internal @@ -44,6 +46,11 @@ class EnvelopeForwarder */ private $onEnvelopeError; + /** + * @var array + */ + private $rateLimiters = []; + /** * @param callable(ResponseInterface): void $onEnvelopeSent called when the envelope is sent * @param callable(\Throwable): void $onEnvelopeError called when the envelope fails to send @@ -68,6 +75,16 @@ public function forward(Envelope $envelope): PromiseInterface $dsn = Dsn::createFromString($dsn); + $rateLimiter = $this->getRateLimiter($dsn); + + // @TODO: We might need to get a little more advanced here and extract all items (and their headers) from the evelope and check each individually so we can remove the ones that are rate limited + $envelopeItemType = $envelope->getFirstItemType(); + + if ($envelopeItemType !== null && $rateLimiter->isRateLimited($envelopeItemType)) { + // @TODO: More information needs to be shown perhaps? Which DSN? Which project? + throw new \RuntimeException("Rate limit exceeded for item of type {$envelopeItemType}"); + } + $authHeader = [ 'sentry_version=' . self::PROTOCOL_VERSION, 'sentry_client=' . self::IDENTIFIER . '/' . self::VERSION, @@ -80,6 +97,23 @@ public function forward(Envelope $envelope): PromiseInterface 'User-Agent' => self::IDENTIFIER . '/' . self::VERSION, 'Content-Type' => 'application/x-sentry-envelope', 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), - ], $envelope->getData())->then($this->onEnvelopeSent, $this->onEnvelopeError); + ], $envelope->getData())->then(function (ResponseInterface $response) use ($rateLimiter) { + $rateLimiter->handleResponse( + new Response($response->getStatusCode(), $response->getHeaders(), $response->getStatusCode() > 400 ? $response->getBody()->getContents() : '') + ); + + \call_user_func($this->onEnvelopeSent, $response); + }, $this->onEnvelopeError); + } + + private function getRateLimiter(Dsn $dsn): RateLimiter + { + $key = $dsn->getEnvelopeApiEndpointUrl(); + + if (!isset($this->rateLimiters[$key])) { + $this->rateLimiters[$key] = new RateLimiter(); + } + + return $this->rateLimiters[$key]; } } From 8a1ba3b8d9072d80461b2a9aef6fd87468ad9bbf Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Mon, 14 Apr 2025 16:03:34 +0200 Subject: [PATCH 2/6] Do not crash agent if forwarder throws --- bin/sentry-agent | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/sentry-agent b/bin/sentry-agent index 60b83a4..0e477e2 100755 --- a/bin/sentry-agent +++ b/bin/sentry-agent @@ -63,7 +63,13 @@ $queue = new EnvelopeQueue( $upstreamConcurrency, $queueLimit, function (Envelope $envelope) use ($forwarder) { - return $forwarder->forward($envelope); + try { + return $forwarder->forward($envelope); + } catch (Exception $e) { + Log::error("Failed to forward envelope: {$e->getMessage()}"); + + return new React\Promise\Internal\RejectedPromise($e); + } } ); From 5cfafe967ad576b6dd1788bdec15425971c3794b Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 6 May 2025 16:56:02 +0200 Subject: [PATCH 3/6] Fully parse envelopes --- composer.json | 13 +- phpunit.xml | 21 ++ src/Envelope.php | 191 +++++++++++++----- src/EnvelopeForwarder.php | 36 ++-- src/EnvelopeItem.php | 69 +++++++ src/Exceptions/MalformedEnvelope.php | 9 + src/Server.php | 7 +- tests/EnvelopeTest.php | 52 +++++ .../envelopes/envelope_with_2_items.dat | 5 + .../envelopes/example_php_sdk_event.dat | 3 + 10 files changed, 331 insertions(+), 75 deletions(-) create mode 100644 phpunit.xml create mode 100644 src/EnvelopeItem.php create mode 100644 src/Exceptions/MalformedEnvelope.php create mode 100644 tests/EnvelopeTest.php create mode 100644 tests/fixtures/envelopes/envelope_with_2_items.dat create mode 100644 tests/fixtures/envelopes/example_php_sdk_event.dat diff --git a/composer.json b/composer.json index 2cad05e..982f9b2 100644 --- a/composer.json +++ b/composer.json @@ -12,6 +12,7 @@ ], "require": { "php": "^7.2|^8", + "ext-json": "*", "clue/mq-react": "^1.6", "react/http": "^1.11", "react/socket": "^1.16", @@ -24,7 +25,13 @@ }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.70", - "phpstan/phpstan": "^2.1" + "phpstan/phpstan": "^2.1", + "phpunit/phpunit": "^8.5|^9.6" + }, + "autoload-dev": { + "psr-4": { + "Sentry\\Agent\\Tests\\": "tests/" + } }, "bin": [ "bin/sentry-agent" @@ -32,8 +39,10 @@ "scripts": { "check": [ "@cs-check", - "@phpstan" + "@phpstan", + "@tests" ], + "tests": "vendor/bin/phpunit --verbose", "cs-check": "vendor/bin/php-cs-fixer fix --verbose --diff --dry-run", "cs-fix": "vendor/bin/php-cs-fixer fix --verbose --diff", "phpstan": "vendor/bin/phpstan analyse" diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..263d883 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,21 @@ + + + + + tests + + + + + + src + + + diff --git a/src/Envelope.php b/src/Envelope.php index 0ca7bf6..ce75ebf 100644 --- a/src/Envelope.php +++ b/src/Envelope.php @@ -4,98 +4,177 @@ namespace Sentry\Agent; -use Sentry\EventType; +use Sentry\Agent\Exceptions\MalformedEnvelope; +use Sentry\Dsn; /** * @internal + * + * @phpstan-type EnvelopeHeader array{ + * dsn: string, + * } */ class Envelope { + public const CONTENT_TYPE = 'application/x-sentry-envelope'; + + /** + * @var EnvelopeHeader The envelope header + */ + private $header; + /** - * @var string + * @var EnvelopeItem[] The envelope items */ - private $data; + private $items; - public function __construct(string $data) + /** + * @param EnvelopeHeader $header + * @param EnvelopeItem[] $items + */ + public function __construct(array $header, array $items) { - $this->data = $data; + $this->header = $header; + $this->items = $items; } - public function getDsn(): ?string + /** + * @return EnvelopeHeader + */ + public function getHeader(): array { - $header = $this->getHeader(); + return $this->header; + } - if ($header === null) { - return null; - } + public function getDsn(): Dsn + { + return Dsn::createFromString($this->header['dsn']); + } - $parsedHeader = json_decode($header, true); + /** + * @return EnvelopeItem[] + */ + public function getItems(): array + { + return $this->items; + } - if (\is_array($parsedHeader) && !empty($parsedHeader['dsn']) && \is_string($parsedHeader['dsn'])) { - return $parsedHeader['dsn']; - } + /** + * @param callable(EnvelopeItem): bool $callback + */ + public function filterItems(callable $callback): void + { + $this->items = array_filter($this->items, $callback); + } - return null; + public function __toString() + { + $data = implode( + "\n", + array_map( + static function (EnvelopeItem $item): string { + return (string) $item; + }, $this->items + ) + ); + + // We always terminate with an additional newline + return json_encode($this->header) . "\n{$data}"; } - public function getFirstItemType(): ?EventType + /** + * @throws MalformedEnvelope + */ + public static function fromString(string $envelope): self { - $header = $this->getFirstItemHeader(); + $consumePart = static function () use (&$envelope): ?string { + // Once we fully consumed the envelope, we return null indicating EOF + if ($envelope === '') { + return null; + } - if ($header === null) { - return null; - } + // Parts are newline delimited so we can find the next newline to find the end of the next part + $nextNewline = strpos($envelope, "\n"); - $parsedHeader = json_decode($header, true); + if ($nextNewline === false) { + $nextNewline = \strlen($envelope); + } - $type = null; + $part = substr($envelope, 0, $nextNewline); - if (\is_array($parsedHeader) && !empty($parsedHeader['type']) && \is_string($parsedHeader['type'])) { - $type = $parsedHeader['type']; - } + // We consume the newline as well + $envelope = substr($envelope, $nextNewline + 1); - switch ($type) { - case (string) EventType::event(): - return EventType::event(); - case (string) EventType::transaction(): - return EventType::transaction(); - case (string) EventType::checkIn(): - return EventType::checkIn(); - default: + // Empty parts are additional trailing newlines, which can be ignored + if ($part === '') { return null; - } - } + } - public function getData(): string - { - return $this->data; - } + return $part; + }; - public function getHeader(): ?string - { - $position = strpos($this->data, "\n"); + $consumeBytes = static function (int $bytes) use (&$envelope): string { + if (\strlen($envelope) < $bytes) { + throw new MalformedEnvelope('Envelope reached EOF before consuming expected bytes'); + } - if ($position === false) { - return null; - } + $part = substr($envelope, 0, $bytes); - return substr($this->data, 0, $position); - } + $envelope = substr($envelope, $bytes + 1); - public function getFirstItemHeader(): ?string - { - $headerEndsAt = strpos($this->data, "\n"); + return $part; + }; + + $parseJson = static function (?string $json): array { + if ($json === null) { + throw new MalformedEnvelope('Envelope reached EOF before consuming expected JSON'); + } + + $decoded = json_decode($json, true); + + if (!\is_array($decoded)) { + // Technically we could have a non-JSON error here (if we try to parse a single JSON scalar for example) + // but we don't really care if that happens and we can just assume there was a problem parsing the JSON if we don't get an array + throw new MalformedEnvelope('Failed to decode JSON: ' . json_last_error_msg()); + } + + return $decoded; + }; + + // The first part is always the envelope header + $header = $parseJson($consumePart()); - if ($headerEndsAt === false) { - return null; + // Technically the header could not contain the DSN key, but we don't really care about that case since we won't be able to forward the envelope + if (!isset($header['dsn'])) { + throw new MalformedEnvelope('Envelope header does not contain a DSN'); } - $itemHeaderEndsAt = strpos($this->data, "\n", $headerEndsAt + 1); + $items = []; + + while ($rawItemHeader = $consumePart()) { + $itemHeader = $parseJson($rawItemHeader); + + // The item header should always contain the type + if (!isset($itemHeader['type'])) { + throw new MalformedEnvelope('Envelope item header does not contain a type'); + } + + // The size in the header is optional + $itemContentLength = $itemHeader['length'] ?? null; + + if ($itemContentLength === null) { + $itemContent = $consumePart(); + + if ($itemContent === null) { + throw new MalformedEnvelope('Envelope reached EOF before consuming expected item content'); + } + } else { + $itemContent = $consumeBytes($itemContentLength); + } - if ($itemHeaderEndsAt === false) { - return null; + $items[] = new EnvelopeItem($itemHeader, $itemContent); } - return substr($this->data, $headerEndsAt + 1, $itemHeaderEndsAt - $headerEndsAt - 1); + return new self($header, $items); } } diff --git a/src/EnvelopeForwarder.php b/src/EnvelopeForwarder.php index 27c59cf..72c2e5a 100644 --- a/src/EnvelopeForwarder.php +++ b/src/EnvelopeForwarder.php @@ -69,21 +69,20 @@ public function forward(Envelope $envelope): PromiseInterface { $dsn = $envelope->getDsn(); - if ($dsn === null) { - throw new \RuntimeException('The envelope does not contain a DSN.'); - } + $rateLimiter = $this->getRateLimiter($dsn); - $dsn = Dsn::createFromString($dsn); + $envelope->filterItems(static function (EnvelopeItem $envelopeItem) use ($rateLimiter) { + $envelopeItemType = $envelopeItem->getItemType(); - $rateLimiter = $this->getRateLimiter($dsn); + // @TODO: We should make the rate limiter accept an arbitrary item type to allow for flexibility when adding new item types + if ($envelopeItemType === null) { + return false; + } - // @TODO: We might need to get a little more advanced here and extract all items (and their headers) from the evelope and check each individually so we can remove the ones that are rate limited - $envelopeItemType = $envelope->getFirstItemType(); + return $rateLimiter->isRateLimited($envelopeItemType); + }); - if ($envelopeItemType !== null && $rateLimiter->isRateLimited($envelopeItemType)) { - // @TODO: More information needs to be shown perhaps? Which DSN? Which project? - throw new \RuntimeException("Rate limit exceeded for item of type {$envelopeItemType}"); - } + // @TODO: If we rate limit all the items we have an empty envelope which we should not send and just return $authHeader = [ 'sentry_version=' . self::PROTOCOL_VERSION, @@ -93,11 +92,16 @@ public function forward(Envelope $envelope): PromiseInterface // @TODO: Implement any number of missing options like the user-agent, encoding, proxy etc. - return (new Browser())->withTimeout($this->timeout)->post($dsn->getEnvelopeApiEndpointUrl(), [ - 'User-Agent' => self::IDENTIFIER . '/' . self::VERSION, - 'Content-Type' => 'application/x-sentry-envelope', - 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), - ], $envelope->getData())->then(function (ResponseInterface $response) use ($rateLimiter) { + // @TODO: We might want to replace this Browser API with a cURL implementation using curl_multi_exec + return (new Browser())->withTimeout($this->timeout)->post( + $dsn->getEnvelopeApiEndpointUrl(), + [ + 'User-Agent' => self::IDENTIFIER . '/' . self::VERSION, + 'Content-Type' => Envelope::CONTENT_TYPE, + 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), + ], + (string) $envelope + )->then(function (ResponseInterface $response) use ($rateLimiter) { $rateLimiter->handleResponse( new Response($response->getStatusCode(), $response->getHeaders(), $response->getStatusCode() > 400 ? $response->getBody()->getContents() : '') ); diff --git a/src/EnvelopeItem.php b/src/EnvelopeItem.php new file mode 100644 index 0000000..4a4a6d4 --- /dev/null +++ b/src/EnvelopeItem.php @@ -0,0 +1,69 @@ +header = $header; + $this->data = $data; + } + + /** + * @return EnvelopeItemHeader + */ + public function getHeader(): array + { + return $this->header; + } + + public function getItemType(): ?EventType + { + switch ($this->header['type']) { + case (string) EventType::event(): + return EventType::event(); + case (string) EventType::transaction(): + return EventType::transaction(); + case (string) EventType::checkIn(): + return EventType::checkIn(); + default: + return null; + } + } + + public function getData(): string + { + return $this->data; + } + + public function __toString() + { + return json_encode($this->header) . "\n" . $this->data; + } +} diff --git a/src/Exceptions/MalformedEnvelope.php b/src/Exceptions/MalformedEnvelope.php new file mode 100644 index 0000000..2dcf03c --- /dev/null +++ b/src/Exceptions/MalformedEnvelope.php @@ -0,0 +1,9 @@ +onEnvelopeReceived, new Envelope(substr($connectionBuffer, 4, $messageLength))); + try { + \call_user_func($this->onEnvelopeReceived, Envelope::fromString(substr($connectionBuffer, 4, $messageLength))); + } catch (MalformedEnvelope $e) { + \call_user_func($this->onConnectionError, $e); + } $connectionBuffer = substr($connectionBuffer, $messageLength); $messageLength = 0; diff --git a/tests/EnvelopeTest.php b/tests/EnvelopeTest.php new file mode 100644 index 0000000..b17ea63 --- /dev/null +++ b/tests/EnvelopeTest.php @@ -0,0 +1,52 @@ +getFixture('example_php_sdk_event'); + + $envelope = Envelope::fromString($payload); + + $this->assertCount(1, $envelope->getItems()); + + $this->assertEquals('event', $envelope->getItems()[0]->getHeader()['type']); + + $this->assertEquals($payload, (string) $envelope); + } + + public function testCanParseEnvelopeWith2Items(): void + { + $payload = $this->getFixture('envelope_with_2_items'); + + $envelope = Envelope::fromString($payload); + + $this->assertEquals('https://e12d836b15bb49d7bbf99e64295d995b:@sentry.io/42', $envelope->getHeader()['dsn']); + + $this->assertCount(2, $envelope->getItems()); + + $this->assertEquals('attachment', $envelope->getItems()[0]->getHeader()['type']); + $this->assertEquals('event', $envelope->getItems()[1]->getHeader()['type']); + + $this->assertEquals($payload, (string) $envelope); + } + + private function getFixture(string $name): string + { + $fixture = file_get_contents(__DIR__ . "/fixtures/envelopes/{$name}.dat"); + + if ($fixture === false) { + throw new \RuntimeException("Failed to read fixture: {$name}"); + } + + // To make it easier to edit the fixtures, we remove the trailing new line + return rtrim($fixture, "\n"); + } +} diff --git a/tests/fixtures/envelopes/envelope_with_2_items.dat b/tests/fixtures/envelopes/envelope_with_2_items.dat new file mode 100644 index 0000000..8c0f57c --- /dev/null +++ b/tests/fixtures/envelopes/envelope_with_2_items.dat @@ -0,0 +1,5 @@ +{"event_id":"9ec79c33ec9942ab8353589fcb2e04dc","dsn":"https:\/\/e12d836b15bb49d7bbf99e64295d995b:@sentry.io\/42"} +{"type":"attachment","length":12,"content_type":"text\/plain","filename":"hello.txt"} +Hello, world +{"type":"event","length":41,"content_type":"application\/json","filename":"application.log"} +{"message":"hello world","level":"error"} diff --git a/tests/fixtures/envelopes/example_php_sdk_event.dat b/tests/fixtures/envelopes/example_php_sdk_event.dat new file mode 100644 index 0000000..a413839 --- /dev/null +++ b/tests/fixtures/envelopes/example_php_sdk_event.dat @@ -0,0 +1,3 @@ +{"event_id":"b19688f4f3db401dbc7b49d954897516","sent_at":"2025-05-06T14:44:01Z","dsn":"https:\/\/e12d836b15bb49d7bbf99e64295d995b:@sentry.io\/42","sdk":{"name":"sentry.php","version":"4.10.0"},"trace":{"trace_id":"9b002d0fb3d0436cb40a3a643e080d76","public_key":"e12d836b15bb49d7bbf99e64295d995b"}} +{"type":"event","content_type":"application\/json"} +{"timestamp":1746542641.990966,"platform":"php","sdk":{"name":"sentry.php","version":"4.10.0"},"server_name":"Mac.localdomain","environment":"production","modules":{"clue\/mq-react":"v1.6.0","clue\/ndjson-react":"v1.3.0","composer\/pcre":"3.3.2","composer\/semver":"3.4.3","composer\/xdebug-handler":"3.0.5","evenement\/evenement":"v3.0.2","fidry\/cpu-core-counter":"1.2.0","fig\/http-message-util":"1.1.5","friendsofphp\/php-cs-fixer":"v3.72.0","guzzlehttp\/psr7":"2.7.0","jean85\/pretty-package-versions":"2.1.0","myclabs\/deep-copy":"1.13.1","nikic\/php-parser":"v5.4.0","phar-io\/manifest":"2.0.4","phar-io\/version":"3.2.1","phpstan\/phpstan":"2.1.8","phpunit\/php-code-coverage":"12.2.1","phpunit\/php-file-iterator":"6.0.0","phpunit\/php-invoker":"6.0.0","phpunit\/php-text-template":"5.0.0","phpunit\/php-timer":"8.0.0","phpunit\/phpunit":"12.1.4","psr\/container":"2.0.2","psr\/event-dispatcher":"1.0.0","psr\/http-factory":"1.1.0","psr\/http-message":"1.1","psr\/log":"3.0.2","ralouphie\/getallheaders":"3.0.3","react\/cache":"v1.2.0","react\/child-process":"v0.6.6","react\/dns":"v1.13.0","react\/event-loop":"v1.5.0","react\/http":"v1.11.0","react\/promise":"v3.2.0","react\/socket":"v1.16.0","react\/stream":"v1.4.0","sebastian\/cli-parser":"4.0.0","sebastian\/comparator":"7.0.1","sebastian\/complexity":"5.0.0","sebastian\/diff":"7.0.0","sebastian\/environment":"8.0.0","sebastian\/exporter":"7.0.0","sebastian\/global-state":"8.0.0","sebastian\/lines-of-code":"4.0.0","sebastian\/object-enumerator":"7.0.0","sebastian\/object-reflector":"5.0.0","sebastian\/recursion-context":"7.0.0","sebastian\/type":"6.0.2","sebastian\/version":"6.0.0","sentry\/sentry":"4.10.0","sentry\/sentry-agent":"dev-main@8a1ba3b","staabm\/side-effects-detector":"1.0.5","symfony\/console":"v7.2.1","symfony\/deprecation-contracts":"v3.5.1","symfony\/event-dispatcher":"v7.2.0","symfony\/event-dispatcher-contracts":"v3.5.1","symfony\/filesystem":"v7.2.0","symfony\/finder":"v7.2.2","symfony\/options-resolver":"v7.2.0","symfony\/polyfill-ctype":"v1.31.0","symfony\/polyfill-intl-grapheme":"v1.31.0","symfony\/polyfill-intl-normalizer":"v1.31.0","symfony\/polyfill-mbstring":"v1.31.0","symfony\/polyfill-php80":"v1.31.0","symfony\/polyfill-php81":"v1.31.0","symfony\/process":"v7.2.4","symfony\/service-contracts":"v3.5.1","symfony\/stopwatch":"v7.2.4","symfony\/string":"v7.2.0","theseer\/tokenizer":"1.2.3"},"contexts":{"os":{"name":"Darwin","version":"24.4.0","build":"Darwin Kernel Version 24.4.0: Fri Apr 11 18:33:40 PDT 2025; root:xnu-11417.101.15~117\/RELEASE_ARM64_T6031","kernel_version":"Darwin Mac.localdomain 24.4.0 Darwin Kernel Version 24.4.0: Fri Apr 11 18:33:40 PDT 2025; root:xnu-11417.101.15~117\/RELEASE_ARM64_T6031 arm64"},"runtime":{"name":"php","sapi":"cli","version":"8.4.5"},"trace":{"trace_id":"9b002d0fb3d0436cb40a3a643e080d76","span_id":"a8287de017514245"}},"message":"Hello world!"} From db322c65e627c041227ec34c98cf961a0c9fe842 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 7 May 2025 22:05:28 +0200 Subject: [PATCH 4/6] Fix reversed logic for rate limiting --- src/EnvelopeForwarder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EnvelopeForwarder.php b/src/EnvelopeForwarder.php index 72c2e5a..2b600e1 100644 --- a/src/EnvelopeForwarder.php +++ b/src/EnvelopeForwarder.php @@ -76,10 +76,10 @@ public function forward(Envelope $envelope): PromiseInterface // @TODO: We should make the rate limiter accept an arbitrary item type to allow for flexibility when adding new item types if ($envelopeItemType === null) { - return false; + return true; } - return $rateLimiter->isRateLimited($envelopeItemType); + return !$rateLimiter->isRateLimited($envelopeItemType); }); // @TODO: If we rate limit all the items we have an empty envelope which we should not send and just return From ad7b5fe7778d8022885af6cfb1d93ba8de11ec03 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 7 May 2025 22:05:43 +0200 Subject: [PATCH 5/6] Add test case with logs payload --- tests/EnvelopeTest.php | 15 ++++++++++++++- .../fixtures/envelopes/envelope_with_log_item.dat | 3 +++ ..._event.dat => envelope_with_php_sdk_event.dat} | 0 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/envelopes/envelope_with_log_item.dat rename tests/fixtures/envelopes/{example_php_sdk_event.dat => envelope_with_php_sdk_event.dat} (100%) diff --git a/tests/EnvelopeTest.php b/tests/EnvelopeTest.php index b17ea63..92daf53 100644 --- a/tests/EnvelopeTest.php +++ b/tests/EnvelopeTest.php @@ -11,7 +11,7 @@ class EnvelopeTest extends TestCase { public function testCanParsePhpSdkExampleEvent(): void { - $payload = $this->getFixture('example_php_sdk_event'); + $payload = $this->getFixture('envelope_with_php_sdk_event'); $envelope = Envelope::fromString($payload); @@ -22,6 +22,19 @@ public function testCanParsePhpSdkExampleEvent(): void $this->assertEquals($payload, (string) $envelope); } + public function testCanParseLogsExampleEvent(): void + { + $payload = $this->getFixture('envelope_with_log_item'); + + $envelope = Envelope::fromString($payload); + + $this->assertCount(1, $envelope->getItems()); + + $this->assertEquals('log', $envelope->getItems()[0]->getHeader()['type']); + + $this->assertEquals($payload, (string) $envelope); + } + public function testCanParseEnvelopeWith2Items(): void { $payload = $this->getFixture('envelope_with_2_items'); diff --git a/tests/fixtures/envelopes/envelope_with_log_item.dat b/tests/fixtures/envelopes/envelope_with_log_item.dat new file mode 100644 index 0000000..7c711df --- /dev/null +++ b/tests/fixtures/envelopes/envelope_with_log_item.dat @@ -0,0 +1,3 @@ +{"event_id":"5b99b51b11c448668a2f7bb3d7cf326c","sent_at":"2025-05-07T19:58:25Z","dsn":"https:\/\/e12d836b15bb49d7bbf99e64295d995b:@sentry.io\/42","sdk":{"name":"sentry.php","version":"4.11.0"},"trace":{"trace_id":"e41ac4ffb6364fe2be41992a32118122","transaction":"","public_key":"37b2e15ddcf0af5695c1f12cb65afcdb","sampled":"false","sample_rand":"0.879018"}} +{"type":"log","item_count":1,"content_type":"application\/vnd.sentry.items.log+json"} +{"items":[{"timestamp":1746647905,"trace_id":"e41ac4ffb6364fe2be41992a32118122","level":"info","body":"Hello from the PHP SDK through the Agent!","attributes":{"sentry.environment":{"type":"string","value":"production"},"sentry.sdk.name":{"type":"string","value":"sentry.php"},"sentry.sdk.version":{"type":"string","value":"4.11.0"},"sentry.trace.parent_span_id":{"type":"string","value":"25dd80b92fa74189"}}}]} diff --git a/tests/fixtures/envelopes/example_php_sdk_event.dat b/tests/fixtures/envelopes/envelope_with_php_sdk_event.dat similarity index 100% rename from tests/fixtures/envelopes/example_php_sdk_event.dat rename to tests/fixtures/envelopes/envelope_with_php_sdk_event.dat From 3a226899a8cd1ab977948d415b99b6330433f51b Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 7 May 2025 22:08:02 +0200 Subject: [PATCH 6/6] Replace filterItems with rejectItems --- src/Envelope.php | 11 ++++++++--- src/EnvelopeForwarder.php | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Envelope.php b/src/Envelope.php index ce75ebf..1082f6b 100644 --- a/src/Envelope.php +++ b/src/Envelope.php @@ -60,11 +60,16 @@ public function getItems(): array } /** - * @param callable(EnvelopeItem): bool $callback + * @param callable(EnvelopeItem): bool $callback if the callback returns true, the item will be removed from the envelope */ - public function filterItems(callable $callback): void + public function rejectItems(callable $callback): void { - $this->items = array_filter($this->items, $callback); + $this->items = array_filter( + $this->items, + static function (EnvelopeItem $item) use ($callback) { + return !$callback($item); + } + ); } public function __toString() diff --git a/src/EnvelopeForwarder.php b/src/EnvelopeForwarder.php index 2b600e1..d8a4a7c 100644 --- a/src/EnvelopeForwarder.php +++ b/src/EnvelopeForwarder.php @@ -71,15 +71,15 @@ public function forward(Envelope $envelope): PromiseInterface $rateLimiter = $this->getRateLimiter($dsn); - $envelope->filterItems(static function (EnvelopeItem $envelopeItem) use ($rateLimiter) { + $envelope->rejectItems(static function (EnvelopeItem $envelopeItem) use ($rateLimiter) { $envelopeItemType = $envelopeItem->getItemType(); // @TODO: We should make the rate limiter accept an arbitrary item type to allow for flexibility when adding new item types if ($envelopeItemType === null) { - return true; + return false; } - return !$rateLimiter->isRateLimited($envelopeItemType); + return $rateLimiter->isRateLimited($envelopeItemType); }); // @TODO: If we rate limit all the items we have an empty envelope which we should not send and just return