Skip to content

Commit 819b9d3

Browse files
LitarnusLitarnus
committed
feat(pii): add data collection policy to simplify handling legacy config
1 parent 9ef4933 commit 819b9d3

6 files changed

Lines changed: 119 additions & 3 deletions

File tree

‎src/DataCollection/DataCollectionPolicy.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Sentry\SentryBundle\DataCollection;
6+
7+
use Sentry\Options;
8+
9+
/**
10+
* Resolves data collection options while preserving legacy behavior when the
11+
* data_collection option is not configured.
12+
*
13+
* @internal
14+
*/
15+
final class DataCollectionPolicy
16+
{
17+
private function __construct()
18+
{
19+
}
20+
21+
public static function shouldCollectUserInfo(Options $options): bool
22+
{
23+
$dataCollection = $options->getDataCollection();
24+
25+
if (null === $dataCollection) {
26+
return $options->shouldSendDefaultPii();
27+
}
28+
29+
return $dataCollection->shouldCollectUserInfo();
30+
}
31+
}

‎src/EventListener/LoginListener.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Sentry\SentryBundle\EventListener;
66

7+
use Sentry\SentryBundle\DataCollection\DataCollectionPolicy;
78
use Sentry\State\HubInterface;
89
use Sentry\State\Scope;
910
use Sentry\UserDataBag;
@@ -90,7 +91,7 @@ private function updateUserContext(TokenInterface $token): void
9091

9192
$client = $this->hub->getClient();
9293

93-
if (null === $client || !$client->getOptions()->shouldSendDefaultPii()) {
94+
if (null === $client || !DataCollectionPolicy::shouldCollectUserInfo($client->getOptions())) {
9495
return;
9596
}
9697

‎src/EventListener/RequestListener.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Sentry\SentryBundle\EventListener;
66

7+
use Sentry\SentryBundle\DataCollection\DataCollectionPolicy;
78
use Sentry\State\HubInterface;
89
use Sentry\State\Scope;
910
use Sentry\UserDataBag;
@@ -48,7 +49,7 @@ public function handleKernelRequestEvent(RequestEvent $event): void
4849

4950
$client = $this->hub->getClient();
5051

51-
if (null === $client || !$client->getOptions()->shouldSendDefaultPii()) {
52+
if (null === $client || !DataCollectionPolicy::shouldCollectUserInfo($client->getOptions())) {
5253
return;
5354
}
5455

‎src/EventListener/TracingRequestListener.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Sentry\SentryBundle\EventListener;
66

77
use Sentry\Integration\RequestFetcherInterface;
8+
use Sentry\SentryBundle\DataCollection\DataCollectionPolicy;
89
use Sentry\SentryBundle\Integration\RequestFetcher;
910
use Sentry\State\HubInterface;
1011
use Sentry\Tracing\TransactionSource;
@@ -131,7 +132,7 @@ private function getData(Request $request): array
131132
$data['net.host.name'] = $request->getHost();
132133
}
133134

134-
if (null !== $request->getClientIp() && null !== $client && $client->getOptions()->shouldSendDefaultPii()) {
135+
if (null !== $request->getClientIp() && null !== $client && DataCollectionPolicy::shouldCollectUserInfo($client->getOptions())) {
135136
$data['net.peer.ip'] = $request->getClientIp();
136137
}
137138

‎tests/DataCollection/DataCollectionPolicyTest.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Sentry\SentryBundle\Tests\DataCollection;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Sentry\Options;
9+
use Sentry\SentryBundle\DataCollection\DataCollectionPolicy;
10+
11+
final class DataCollectionPolicyTest extends TestCase
12+
{
13+
/**
14+
* @param array<string, mixed> $options
15+
*
16+
* @dataProvider shouldCollectUserInfoDataProvider
17+
*/
18+
public function testShouldCollectUserInfo(array $options, bool $expected): void
19+
{
20+
$this->assertSame($expected, DataCollectionPolicy::shouldCollectUserInfo(new Options($options)));
21+
}
22+
23+
/**
24+
* @return \Generator<string, array{array<string, mixed>, bool}>
25+
*/
26+
public function shouldCollectUserInfoDataProvider(): \Generator
27+
{
28+
yield 'legacy behavior is disabled' => [
29+
['send_default_pii' => false],
30+
false,
31+
];
32+
33+
yield 'legacy behavior is enabled' => [
34+
['send_default_pii' => true],
35+
true,
36+
];
37+
38+
yield 'data collection disables user info regardless of legacy option' => [
39+
[
40+
'send_default_pii' => true,
41+
'data_collection' => ['user_info' => false],
42+
],
43+
false,
44+
];
45+
46+
yield 'data collection enables user info regardless of legacy option' => [
47+
[
48+
'send_default_pii' => false,
49+
'data_collection' => ['user_info' => true],
50+
],
51+
true,
52+
];
53+
}
54+
}

‎tests/EventListener/RequestListenerTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,34 @@ public function handleKernelRequestEventDataProvider(): \Generator
8989
new UserDataBag(),
9090
];
9191

92+
yield 'options.data_collection.user_info = FALSE overrides send_default_pii' => [
93+
new RequestEvent(
94+
$this->createMock(HttpKernelInterface::class),
95+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
96+
\defined(HttpKernelInterface::class . '::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : (int) \constant(HttpKernelInterface::class . '::MASTER_REQUEST')
97+
),
98+
$this->getMockedClientWithOptions(new Options([
99+
'send_default_pii' => true,
100+
'data_collection' => ['user_info' => false],
101+
])),
102+
new UserDataBag(),
103+
new UserDataBag(),
104+
];
105+
106+
yield 'options.data_collection.user_info = TRUE overrides send_default_pii' => [
107+
new RequestEvent(
108+
$this->createMock(HttpKernelInterface::class),
109+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
110+
\defined(HttpKernelInterface::class . '::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : (int) \constant(HttpKernelInterface::class . '::MASTER_REQUEST')
111+
),
112+
$this->getMockedClientWithOptions(new Options([
113+
'send_default_pii' => false,
114+
'data_collection' => ['user_info' => true],
115+
])),
116+
new UserDataBag(),
117+
new UserDataBag(null, null, '127.0.0.1'),
118+
];
119+
92120
yield 'request.clientIp IS NULL' => [
93121
new RequestEvent(
94122
$this->createMock(HttpKernelInterface::class),

0 commit comments

Comments
 (0)