Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Transport/HttpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ public function send(Event $event): Result
return new Result(ResultStatus::rateLimit());
}

// Since profiles are attached to transaction we have to check separately if they are rate limited.
// We can do this after transactions have been checked because if transactions are rate limited,
// so are profiles but not the other way around.
if ($event->getSdkMetadata('profile') !== null) {
if ($this->rateLimiter->isRateLimited(RateLimiter::DATA_CATEGORY_PROFILE)) {
// Just remove profiling data so the normal transaction can be sent.
$event->setSdkMetadata('profile', null);
$this->logger->warning(
'Rate limit exceeded for sending requests of type "profile". The profile has been dropped.',
['event' => $event]
);
}
}

$request = new Request();
$request->setStringBody($this->payloadSerializer->serialize($event));

Expand Down
16 changes: 13 additions & 3 deletions src/Transport/RateLimiter.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@

final class RateLimiter
{
/**
* @var string
*/
public const DATA_CATEGORY_PROFILE = 'profile';

/**
* @var string
*/
Expand All @@ -21,6 +26,11 @@ final class RateLimiter
*/
private const DATA_CATEGORY_LOG_ITEM = 'log_item';

/**
* @var string
*/
private const DATA_CATEGORY_CHECK_IN = 'monitor';

/**
* The name of the header to look at to know the rate limits for the events
* categories supported by the server.
Expand Down Expand Up @@ -103,9 +113,7 @@ public function handleResponse(Response $response): bool
*/
public function isRateLimited($eventType): bool
{
$disabledUntil = $this->getDisabledUntil($eventType);

return $disabledUntil > time();
return $this->getDisabledUntil($eventType) > time();
}

/**
Expand All @@ -119,6 +127,8 @@ public function getDisabledUntil($eventType): int
$eventType = self::DATA_CATEGORY_ERROR;
} elseif ($eventType === 'log') {
$eventType = self::DATA_CATEGORY_LOG_ITEM;
} elseif ($eventType === 'check_in') {
$eventType = self::DATA_CATEGORY_CHECK_IN;
}

return max($this->rateLimits['all'] ?? 0, $this->rateLimits[$eventType] ?? 0);
Expand Down
104 changes: 102 additions & 2 deletions tests/Transport/HttpTransportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Sentry\HttpClient\HttpClientInterface;
use Sentry\HttpClient\Response;
use Sentry\Options;
use Sentry\Profiling\Profile;
use Sentry\Serializer\PayloadSerializerInterface;
use Sentry\Transport\HttpTransport;
use Sentry\Transport\ResultStatus;
Expand All @@ -25,7 +26,7 @@ final class HttpTransportTest extends TestCase
private $logger;

/**
* @var MockObject&HttpAsyncClientInterface
* @var MockObject&HttpClientInterface
*/
private $httpClient;

Expand Down Expand Up @@ -180,7 +181,7 @@ public function testSendFailsDueToHttpClientException(): void
$this->assertSame(ResultStatus::failed(), $result->getStatus());
}

public function testSendFailsDueToCulrError(): void
public function testSendFailsDueToCurlError(): void
{
$event = Event::createEvent();

Expand Down Expand Up @@ -263,6 +264,105 @@ public function testSendFailsDueToExceedingRateLimits(): void
$this->assertSame(ResultStatus::rateLimit(), $result->getStatus());
}

/**
* @group time-sensitive
*/
public function testDropsProfileAndSendsTransactionWhenProfileRateLimited(): void
{
ClockMock::withClockMock(1644105600);

$transport = new HttpTransport(
new Options(['dsn' => 'http://public@example.com/1']),
$this->httpClient,
$this->payloadSerializer,
$this->logger
);

$event = Event::createTransaction();
$event->setSdkMetadata('profile', new Profile());

$this->payloadSerializer->expects($this->exactly(2))
->method('serialize')
->willReturn('{"foo":"bar"}');

$this->httpClient->expects($this->exactly(2))
->method('sendRequest')
->willReturnOnConsecutiveCalls(
new Response(429, ['X-Sentry-Rate-Limits' => ['60:profile:key']], ''),
new Response(200, [], '')
);

// First request is rate limited because of profiles
$result = $transport->send($event);

$this->assertEquals(ResultStatus::rateLimit(), $result->getStatus());

// profile information is still present
$this->assertNotNull($event->getSdkMetadata('profile'));

$event = Event::createTransaction();
$event->setSdkMetadata('profile', new Profile());

$this->logger->expects($this->once())
->method('warning')
->with(
$this->stringContains('Rate limit exceeded for sending requests of type "profile".'),
['event' => $event]
);

$result = $transport->send($event);

// Sending transaction is successful because only profiles are rate limited
$this->assertEquals(ResultStatus::success(), $result->getStatus());

// profile information is removed because it was rate limited
$this->assertNull($event->getSdkMetadata('profile'));
}

/**
* @group time-sensitive
*/
public function testCheckInsAreRateLimited(): void
{
ClockMock::withClockMock(1644105600);

$transport = new HttpTransport(
new Options(['dsn' => 'http://public@example.com/1']),
$this->httpClient,
$this->payloadSerializer,
$this->logger
);

$event = Event::createCheckIn();

$this->payloadSerializer->expects($this->exactly(1))
->method('serialize')
->willReturn('{"foo":"bar"}');

$this->httpClient->expects($this->exactly(1))
->method('sendRequest')
->willReturn(
new Response(429, ['X-Sentry-Rate-Limits' => ['60:monitor:key']], '')
);

$result = $transport->send($event);

$this->assertEquals(ResultStatus::rateLimit(), $result->getStatus());

$event = Event::createCheckIn();

$this->logger->expects($this->once())
->method('warning')
->with(
$this->stringContains('Rate limit exceeded for sending requests of type "check_in".'),
['event' => $event]
);

$result = $transport->send($event);

$this->assertEquals(ResultStatus::rateLimit(), $result->getStatus());
}

public function testClose(): void
{
$transport = new HttpTransport(
Expand Down