From ac7bf4caf60d995fd8e9e9daa0557aa8ac709a44 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 15 Apr 2025 12:04:48 +0200 Subject: [PATCH 1/5] Always add the SDK as module when modules integration is disabled --- src/Integration/IntegrationRegistry.php | 1 + src/Integration/SDKModuleIntegration.php | 37 +++++++ .../Integration/SDKModuleIntegrationTest.php | 99 +++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 src/Integration/SDKModuleIntegration.php create mode 100644 tests/Integration/SDKModuleIntegrationTest.php diff --git a/src/Integration/IntegrationRegistry.php b/src/Integration/IntegrationRegistry.php index be2df22eb..c20abdf7f 100644 --- a/src/Integration/IntegrationRegistry.php +++ b/src/Integration/IntegrationRegistry.php @@ -145,6 +145,7 @@ private function getDefaultIntegrations(Options $options): array new FrameContextifierIntegration(), new EnvironmentIntegration(), new ModulesIntegration(), + new SDKModuleIntegration(), ]; if ($options->getDsn() !== null) { diff --git a/src/Integration/SDKModuleIntegration.php b/src/Integration/SDKModuleIntegration.php new file mode 100644 index 000000000..ad48fd055 --- /dev/null +++ b/src/Integration/SDKModuleIntegration.php @@ -0,0 +1,37 @@ +getIntegration(self::class); + + // The integration could be bound to a client that is not the one + // attached to the current hub. If this is the case, bail out + if ($integration !== null && $event->getModules() === []) { + $event->setModules([ + 'sentry/sentry' => Client::SDK_VERSION, + ]); + } + + return $event; + }); + } +} diff --git a/tests/Integration/SDKModuleIntegrationTest.php b/tests/Integration/SDKModuleIntegrationTest.php new file mode 100644 index 000000000..8473971e8 --- /dev/null +++ b/tests/Integration/SDKModuleIntegrationTest.php @@ -0,0 +1,99 @@ +setupOnce(); + + /** @var ClientInterface&MockObject $client */ + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getIntegration') + ->willReturn($isIntegrationEnabled ? $integration : null); + + SentrySdk::getCurrentHub()->bindClient($client); + + withScope(function (Scope $scope) use ($expectedEmptyModules): void { + $event = $scope->applyToEvent(Event::createEvent()); + + $this->assertNotNull($event); + + if ($expectedEmptyModules) { + $this->assertEmpty($event->getModules()); + } else { + $this->assertNotEmpty($event->getModules()); + $this->assertEquals( + [ + 'sentry/sentry' => Client::SDK_VERSION, + ], + $event->getModules() + ); + } + }); + } + + public function testEnsureModulesIntegrationTakesPrecedence(): void + { + $integration1 = new ModulesIntegration(); + $integration1->setupOnce(); + + $integration2 = new SDKModuleIntegration(); + $integration2->setupOnce(); + + /** @var ClientInterface&MockObject $client */ + $client = $this->createMock(ClientInterface::class); + $client->expects($this->exactly(2)) + ->method('getIntegration') + ->willReturn($integration1, $integration2); + + SentrySdk::getCurrentHub()->bindClient($client); + + withScope(function (Scope $scope): void { + $event = $scope->applyToEvent(Event::createEvent()); + + $this->assertNotNull($event); + + $this->assertNotEmpty($event->getModules()); + $this->assertNotEquals( + [ + 'sentry/sentry' => Client::SDK_VERSION, + ], + $event->getModules() + ); + }); + } + + public static function invokeDataProvider(): \Generator + { + yield [ + false, + true, + ]; + + yield [ + true, + false, + ]; + } +} From 6eba98b7aeff80801654876a5dc3e50f26a43b36 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 15 Apr 2025 12:12:46 +0200 Subject: [PATCH 2/5] Update tests --- tests/Integration/IntegrationRegistryTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Integration/IntegrationRegistryTest.php b/tests/Integration/IntegrationRegistryTest.php index 43caf2872..4b5d5e70d 100644 --- a/tests/Integration/IntegrationRegistryTest.php +++ b/tests/Integration/IntegrationRegistryTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +use Sentry\Integration\SDKModuleIntegration; use Sentry\Integration\EnvironmentIntegration; use Sentry\Integration\ErrorListenerIntegration; use Sentry\Integration\ExceptionListenerIntegration; @@ -86,6 +87,7 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; @@ -121,6 +123,7 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), $integration1ClassName => $integration1, $integration2ClassName => $integration2, ], @@ -143,6 +146,7 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), TransactionIntegration::class => new TransactionIntegration(), $integration1ClassName => $integration1, ], @@ -164,6 +168,7 @@ public function setupOnce(): void TransactionIntegration::class => new TransactionIntegration(), FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), ModulesIntegration::class => new ModulesIntegration(), ], ]; @@ -208,6 +213,7 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; @@ -225,6 +231,7 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), + SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; } From 66577e3c4a63bfd3159c7c0a5122b9d5fb030943 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 15 Apr 2025 12:14:36 +0200 Subject: [PATCH 3/5] CS --- tests/Integration/IntegrationRegistryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/IntegrationRegistryTest.php b/tests/Integration/IntegrationRegistryTest.php index 4b5d5e70d..46848a227 100644 --- a/tests/Integration/IntegrationRegistryTest.php +++ b/tests/Integration/IntegrationRegistryTest.php @@ -6,7 +6,6 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Sentry\Integration\SDKModuleIntegration; use Sentry\Integration\EnvironmentIntegration; use Sentry\Integration\ErrorListenerIntegration; use Sentry\Integration\ExceptionListenerIntegration; @@ -16,6 +15,7 @@ use Sentry\Integration\IntegrationRegistry; use Sentry\Integration\ModulesIntegration; use Sentry\Integration\RequestIntegration; +use Sentry\Integration\SDKModuleIntegration; use Sentry\Integration\TransactionIntegration; use Sentry\Options; From a5a50ef0682c4eb735ef5ba4a5e3ffcc44781fdf Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 24 Apr 2025 09:54:46 +0200 Subject: [PATCH 4/5] Refactor to use packages entry on SDK interface --- src/Event.php | 47 +++++++++ src/Integration/IntegrationRegistry.php | 1 - src/Integration/SDKModuleIntegration.php | 37 ------- src/Serializer/EnvelopItems/EventItem.php | 5 +- .../EnvelopItems/TransactionItem.php | 5 +- src/Serializer/PayloadSerializer.php | 5 +- tests/Integration/IntegrationRegistryTest.php | 7 -- .../Integration/SDKModuleIntegrationTest.php | 99 ------------------- tests/Serializer/PayloadSerializerTest.php | 38 +++---- 9 files changed, 69 insertions(+), 175 deletions(-) delete mode 100644 src/Integration/SDKModuleIntegration.php delete mode 100644 tests/Integration/SDKModuleIntegrationTest.php diff --git a/src/Event.php b/src/Event.php index 0148069ac..04e1c2d26 100644 --- a/src/Event.php +++ b/src/Event.php @@ -19,6 +19,11 @@ * count: int, * tags: array, * } + * + * @phpstan-type SdkPackageEntry array{ + * name: string, + * version: string, + * } */ final class Event { @@ -174,6 +179,16 @@ final class Event */ private $sdkVersion = Client::SDK_VERSION; + /** + * @var SdkPackageEntry[] The Sentry SDK packages + */ + private $sdkPackages = [ + [ + 'name' => 'composer:sentry/sentry', + 'version' => Client::SDK_VERSION, + ], + ]; + /** * @var EventType The type of the Event */ @@ -276,6 +291,38 @@ public function setSdkVersion(string $sdkVersion): self return $this; } + /** + * Append a package to the list of SDK packages. + * + * @return $this + * + * @internal + */ + public function appendSdkPackage(array $package): self + { + $this->sdkPackages[] = $package; + + return $this; + } + + /** + * Gets the SDK playload that will be sent to Sentry. + * + * @see https://develop.sentry.dev/sdk/data-model/event-payloads/sdk/ + * + * @return array{name: string, version: string, packages: SdkPackageEntry[]} + * + * @internal + */ + public function getSdkPayload(): array + { + return [ + 'name' => $this->sdkIdentifier, + 'version' => $this->sdkVersion, + 'packages' => $this->sdkPackages, + ]; + } + /** * Gets the timestamp of when this event was generated. */ diff --git a/src/Integration/IntegrationRegistry.php b/src/Integration/IntegrationRegistry.php index c20abdf7f..be2df22eb 100644 --- a/src/Integration/IntegrationRegistry.php +++ b/src/Integration/IntegrationRegistry.php @@ -145,7 +145,6 @@ private function getDefaultIntegrations(Options $options): array new FrameContextifierIntegration(), new EnvironmentIntegration(), new ModulesIntegration(), - new SDKModuleIntegration(), ]; if ($options->getDsn() !== null) { diff --git a/src/Integration/SDKModuleIntegration.php b/src/Integration/SDKModuleIntegration.php deleted file mode 100644 index ad48fd055..000000000 --- a/src/Integration/SDKModuleIntegration.php +++ /dev/null @@ -1,37 +0,0 @@ -getIntegration(self::class); - - // The integration could be bound to a client that is not the one - // attached to the current hub. If this is the case, bail out - if ($integration !== null && $event->getModules() === []) { - $event->setModules([ - 'sentry/sentry' => Client::SDK_VERSION, - ]); - } - - return $event; - }); - } -} diff --git a/src/Serializer/EnvelopItems/EventItem.php b/src/Serializer/EnvelopItems/EventItem.php index dd37f70ff..ca6d1d38b 100644 --- a/src/Serializer/EnvelopItems/EventItem.php +++ b/src/Serializer/EnvelopItems/EventItem.php @@ -28,10 +28,7 @@ public static function toEnvelopeItem(Event $event): string $payload = [ 'timestamp' => $event->getTimestamp(), 'platform' => 'php', - 'sdk' => [ - 'name' => $event->getSdkIdentifier(), - 'version' => $event->getSdkVersion(), - ], + 'sdk' => $event->getSdkPayload(), ]; if ($event->getStartTimestamp() !== null) { diff --git a/src/Serializer/EnvelopItems/TransactionItem.php b/src/Serializer/EnvelopItems/TransactionItem.php index de6fd624a..679bd6bcd 100644 --- a/src/Serializer/EnvelopItems/TransactionItem.php +++ b/src/Serializer/EnvelopItems/TransactionItem.php @@ -35,10 +35,7 @@ public static function toEnvelopeItem(Event $event): string $payload = [ 'timestamp' => $event->getTimestamp(), 'platform' => 'php', - 'sdk' => [ - 'name' => $event->getSdkIdentifier(), - 'version' => $event->getSdkVersion(), - ], + 'sdk' => $event->getSdkPayload(), ]; if ($event->getStartTimestamp() !== null) { diff --git a/src/Serializer/PayloadSerializer.php b/src/Serializer/PayloadSerializer.php index c109e0336..b836fe044 100644 --- a/src/Serializer/PayloadSerializer.php +++ b/src/Serializer/PayloadSerializer.php @@ -42,10 +42,7 @@ public function serialize(Event $event): string 'event_id' => (string) $event->getId(), 'sent_at' => gmdate('Y-m-d\TH:i:s\Z'), 'dsn' => (string) $this->options->getDsn(), - 'sdk' => [ - 'name' => $event->getSdkIdentifier(), - 'version' => $event->getSdkVersion(), - ], + 'sdk' => $event->getSdkPayload(), ]; $dynamicSamplingContext = $event->getSdkMetadata('dynamic_sampling_context'); diff --git a/tests/Integration/IntegrationRegistryTest.php b/tests/Integration/IntegrationRegistryTest.php index 46848a227..43caf2872 100644 --- a/tests/Integration/IntegrationRegistryTest.php +++ b/tests/Integration/IntegrationRegistryTest.php @@ -15,7 +15,6 @@ use Sentry\Integration\IntegrationRegistry; use Sentry\Integration\ModulesIntegration; use Sentry\Integration\RequestIntegration; -use Sentry\Integration\SDKModuleIntegration; use Sentry\Integration\TransactionIntegration; use Sentry\Options; @@ -87,7 +86,6 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; @@ -123,7 +121,6 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), $integration1ClassName => $integration1, $integration2ClassName => $integration2, ], @@ -146,7 +143,6 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), TransactionIntegration::class => new TransactionIntegration(), $integration1ClassName => $integration1, ], @@ -168,7 +164,6 @@ public function setupOnce(): void TransactionIntegration::class => new TransactionIntegration(), FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), ModulesIntegration::class => new ModulesIntegration(), ], ]; @@ -213,7 +208,6 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; @@ -231,7 +225,6 @@ public function setupOnce(): void FrameContextifierIntegration::class => new FrameContextifierIntegration(), EnvironmentIntegration::class => new EnvironmentIntegration(), ModulesIntegration::class => new ModulesIntegration(), - SDKModuleIntegration::class => new SDKModuleIntegration(), ], ]; } diff --git a/tests/Integration/SDKModuleIntegrationTest.php b/tests/Integration/SDKModuleIntegrationTest.php deleted file mode 100644 index 8473971e8..000000000 --- a/tests/Integration/SDKModuleIntegrationTest.php +++ /dev/null @@ -1,99 +0,0 @@ -setupOnce(); - - /** @var ClientInterface&MockObject $client */ - $client = $this->createMock(ClientInterface::class); - $client->expects($this->once()) - ->method('getIntegration') - ->willReturn($isIntegrationEnabled ? $integration : null); - - SentrySdk::getCurrentHub()->bindClient($client); - - withScope(function (Scope $scope) use ($expectedEmptyModules): void { - $event = $scope->applyToEvent(Event::createEvent()); - - $this->assertNotNull($event); - - if ($expectedEmptyModules) { - $this->assertEmpty($event->getModules()); - } else { - $this->assertNotEmpty($event->getModules()); - $this->assertEquals( - [ - 'sentry/sentry' => Client::SDK_VERSION, - ], - $event->getModules() - ); - } - }); - } - - public function testEnsureModulesIntegrationTakesPrecedence(): void - { - $integration1 = new ModulesIntegration(); - $integration1->setupOnce(); - - $integration2 = new SDKModuleIntegration(); - $integration2->setupOnce(); - - /** @var ClientInterface&MockObject $client */ - $client = $this->createMock(ClientInterface::class); - $client->expects($this->exactly(2)) - ->method('getIntegration') - ->willReturn($integration1, $integration2); - - SentrySdk::getCurrentHub()->bindClient($client); - - withScope(function (Scope $scope): void { - $event = $scope->applyToEvent(Event::createEvent()); - - $this->assertNotNull($event); - - $this->assertNotEmpty($event->getModules()); - $this->assertNotEquals( - [ - 'sentry/sentry' => Client::SDK_VERSION, - ], - $event->getModules() - ); - }); - } - - public static function invokeDataProvider(): \Generator - { - yield [ - false, - true, - ]; - - yield [ - true, - false, - ]; - } -} diff --git a/tests/Serializer/PayloadSerializerTest.php b/tests/Serializer/PayloadSerializerTest.php index c9283825d..1e8eca0fe 100644 --- a/tests/Serializer/PayloadSerializerTest.php +++ b/tests/Serializer/PayloadSerializerTest.php @@ -63,9 +63,9 @@ public static function serializeAsEnvelopeDataProvider(): iterable yield [ Event::createEvent(new EventId('fc9442f5aef34234bb22b9a615e30ccd')), <<\/","server_name":"foo.example.com","release":"721e41770371db95eee98ca2707686226b993eda","environment":"production","fingerprint":["myrpc","POST","\/foo.bar"],"modules":{"my.module.name":"1.0"},"extra":{"my_key":1,"some_other_value":"foo bar"},"tags":{"ios_version":"4.0","context":"production"},"user":{"id":"unique_id","username":"my_user","email":"foo@example.com","ip_address":"127.0.0.1","segment":"my_segment"},"contexts":{"os":{"name":"Linux","version":"4.19.104-microsoft-standard","build":"#1 SMP Wed Feb 19 06:37:35 UTC 2020","kernel_version":"Linux 7944782cd697 4.19.104-microsoft-standard #1 SMP Wed Feb 19 06:37:35 UTC 2020 x86_64"},"runtime":{"name":"php","sapi":"cli","version":"7.4.3"},"electron":{"type":"runtime","name":"Electron","version":"4.0"}},"breadcrumbs":{"values":[{"type":"user","category":"log","level":"info","timestamp":1597790835},{"type":"navigation","category":"log","level":"info","timestamp":1597790835,"data":{"from":"\/login","to":"\/dashboard"}},{"type":"default","category":"log","level":"info","timestamp":1597790835,"data":{"0":"foo","1":"bar"}}]},"request":{"method":"POST","url":"http:\/\/absolute.uri\/foo","query_string":"query=foobar&page=2","data":{"foo":"bar"},"cookies":{"PHPSESSID":"298zf09hf012fh2"},"headers":{"content-type":"text\/html"},"env":{"REMOTE_ADDR":"127.0.0.1"}},"exception":{"values":[{"type":"Exception","value":"chained exception","stacktrace":{"frames":[{"filename":"file\/name.py","lineno":3,"in_app":true},{"filename":"file\/name.py","lineno":3,"in_app":false,"abs_path":"absolute\/file\/name.py","function":"myfunction","raw_function":"raw_function_name","pre_context":["def foo():"," my_var = 'foo'"],"context_line":" raise ValueError()","post_context":["","def main():"],"vars":{"my_var":"value"}}]},"mechanism":{"type":"generic","handled":true,"data":{"code":123}}},{"type":"Exception","value":"initial exception"}]}} +{"timestamp":1597790835,"platform":"php","sdk":{"name":"sentry.php","version":"$sdkVersion","packages":[{"name":"composer:sentry\/sentry","version":"$sdkVersion"}]},"start_timestamp":1597790835,"level":"error","logger":"app.php","transaction":"\/users\/\/","server_name":"foo.example.com","release":"721e41770371db95eee98ca2707686226b993eda","environment":"production","fingerprint":["myrpc","POST","\/foo.bar"],"modules":{"my.module.name":"1.0"},"extra":{"my_key":1,"some_other_value":"foo bar"},"tags":{"ios_version":"4.0","context":"production"},"user":{"id":"unique_id","username":"my_user","email":"foo@example.com","ip_address":"127.0.0.1","segment":"my_segment"},"contexts":{"os":{"name":"Linux","version":"4.19.104-microsoft-standard","build":"#1 SMP Wed Feb 19 06:37:35 UTC 2020","kernel_version":"Linux 7944782cd697 4.19.104-microsoft-standard #1 SMP Wed Feb 19 06:37:35 UTC 2020 x86_64"},"runtime":{"name":"php","sapi":"cli","version":"7.4.3"},"electron":{"type":"runtime","name":"Electron","version":"4.0"}},"breadcrumbs":{"values":[{"type":"user","category":"log","level":"info","timestamp":1597790835},{"type":"navigation","category":"log","level":"info","timestamp":1597790835,"data":{"from":"\/login","to":"\/dashboard"}},{"type":"default","category":"log","level":"info","timestamp":1597790835,"data":{"0":"foo","1":"bar"}}]},"request":{"method":"POST","url":"http:\/\/absolute.uri\/foo","query_string":"query=foobar&page=2","data":{"foo":"bar"},"cookies":{"PHPSESSID":"298zf09hf012fh2"},"headers":{"content-type":"text\/html"},"env":{"REMOTE_ADDR":"127.0.0.1"}},"exception":{"values":[{"type":"Exception","value":"chained exception","stacktrace":{"frames":[{"filename":"file\/name.py","lineno":3,"in_app":true},{"filename":"file\/name.py","lineno":3,"in_app":false,"abs_path":"absolute\/file\/name.py","function":"myfunction","raw_function":"raw_function_name","pre_context":["def foo():"," my_var = 'foo'"],"context_line":" raise ValueError()","post_context":["","def main():"],"vars":{"my_var":"value"}}]},"mechanism":{"type":"generic","handled":true,"data":{"code":123}}},{"type":"Exception","value":"initial exception"}]}} TEXT ]; @@ -181,9 +181,9 @@ public static function serializeAsEnvelopeDataProvider(): iterable yield [ $event, << Date: Thu, 24 Apr 2025 10:14:04 +0200 Subject: [PATCH 5/5] CS --- src/Event.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Event.php b/src/Event.php index 04e1c2d26..51f4563b2 100644 --- a/src/Event.php +++ b/src/Event.php @@ -19,7 +19,6 @@ * count: int, * tags: array, * } - * * @phpstan-type SdkPackageEntry array{ * name: string, * version: string, @@ -294,6 +293,8 @@ public function setSdkVersion(string $sdkVersion): self /** * Append a package to the list of SDK packages. * + * @param SdkPackageEntry $package The package to append + * * @return $this * * @internal