Skip to content

Commit f16a4f8

Browse files
stayallivestayallive
committed
More refactorings
1 parent 302b6be commit f16a4f8

8 files changed

Lines changed: 118 additions & 55 deletions

File tree

‎src/Client.php‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco
281281

282282
$event->setSdkIdentifier($this->sdkIdentifier);
283283
$event->setSdkVersion($this->sdkVersion);
284+
284285
$event->setTags(array_merge($this->options->getTags(), $event->getTags()));
285286

286287
if ($event->getServerName() === null) {
@@ -344,6 +345,17 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco
344345
),
345346
['event' => $beforeSendCallback]
346347
);
348+
349+
return null;
350+
}
351+
352+
// We wait until here to set some default attributes on the log entries because in the meantime logs might have been filtered
353+
foreach ($event->getLogs() as $log) {
354+
$log->setAttribute('sentry.release', $this->options->getRelease());
355+
$log->setAttribute('sentry.sdk.name', $this->sdkIdentifier);
356+
$log->setAttribute('sentry.sdk.version', $this->sdkVersion);
357+
$log->setAttribute('sentry.environment', $this->options->getEnvironment() ?? Event::DEFAULT_ENVIRONMENT);
358+
$log->setAttribute('sentry.server.address', $this->options->getServerName());
347359
}
348360

349361
return $event;
@@ -412,16 +424,27 @@ private function applyBeforeSendCallback(Event $event, ?EventHint $hint): ?Event
412424
return ($this->options->getBeforeSendCheckInCallback())($event, $hint);
413425
case EventType::logs():
414426
$logs = array_filter(array_map(function (Log $log): ?Log {
415-
// @TODO: Should we emit a log that we dropped a log item?
416427
return ($this->options->getBeforeSendLogCallback())($log);
417428
}, $event->getLogs()));
418429

419430
if (empty($logs)) {
420431
return null;
421432
}
422433

434+
$logCountDifference = \count($event->getLogs()) - \count($logs);
435+
423436
$event->setLogs($logs);
424437

438+
if ($logCountDifference > 0) {
439+
$this->logger->info(
440+
\sprintf(
441+
'%s logs will be discarded because the "before_send_log" callback returned "null" for them.',
442+
$logCountDifference
443+
),
444+
['event' => $event]
445+
);
446+
}
447+
425448
return $event;
426449
default:
427450
return $event;
@@ -435,6 +458,8 @@ private function getBeforeSendCallbackName(Event $event): string
435458
return 'before_send_transaction';
436459
case EventType::checkIn():
437460
return 'before_send_check_in';
461+
case EventType::logs():
462+
return 'before_send_log';
438463
default:
439464
return 'before_send';
440465
}

‎src/Logger/LogsLogger.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class LogsLogger extends AbstractLogger
2020
*/
2121
public function log($level, $message, array $context = []): void
2222
{
23-
// @TODO: The $context might contain attributes we don't support yet (like arrays for example), should we ignore them? Type hint is basically mixed[].
2423
switch ($level) {
2524
case 'emergency':
2625
case 'critical':

‎src/Logs/Log.php‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,31 +41,32 @@ class Log implements \JsonSerializable
4141
/**
4242
* @var array<string, LogAttribute>
4343
*/
44-
private $attributes;
44+
private $attributes = [];
4545

46-
/**
47-
* @param array<string, LogAttribute> $attributes
48-
*/
4946
public function __construct(
5047
float $timestamp,
5148
string $traceId,
5249
LogLevel $level,
53-
string $body,
54-
array $attributes = []
50+
string $body
5551
) {
5652
$this->timestamp = $timestamp;
5753
$this->traceId = $traceId;
5854
$this->level = $level;
5955
$this->body = $body;
60-
$this->attributes = $attributes;
6156
}
6257

6358
/**
64-
* @param AttributeValue $value
59+
* @param mixed $value
6560
*/
6661
public function setAttribute(string $key, $value): self
6762
{
68-
$this->attributes[$key] = LogAttribute::fromValue($value);
63+
$attribute = $value instanceof LogAttribute
64+
? $value
65+
: LogAttribute::tryFromValue($value);
66+
67+
if ($attribute !== null) {
68+
$this->attributes[$key] = $attribute;
69+
}
6970

7071
return $this;
7172
}

‎src/Logs/LogAttribute.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,14 @@ public function __construct($value, string $type)
3535
}
3636

3737
/**
38-
* @param AttributeValue|\Stringable $value
38+
* @param mixed $value
3939
*/
40-
public static function fromValue($value): self
40+
public static function tryFromValue($value): ?self
4141
{
42+
if ($value === null) {
43+
return null;
44+
}
45+
4246
if (\is_bool($value)) {
4347
return new self($value, 'boolean');
4448
}
@@ -51,7 +55,17 @@ public static function fromValue($value): self
5155
return new self($value, 'double');
5256
}
5357

54-
return new self((string) $value, 'string');
58+
if (\is_string($value) || (\is_object($value) && method_exists($value, '__toString'))) {
59+
$stringValue = (string) $value;
60+
61+
if (empty($stringValue)) {
62+
return null;
63+
}
64+
65+
return new self($stringValue, 'string');
66+
}
67+
68+
return null;
5569
}
5670

5771
/**

‎src/Logs/LogsAggregator.php‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44

55
namespace Sentry\Logs;
66

7-
use Sentry\Client;
87
use Sentry\Event;
98
use Sentry\EventId;
109
use Sentry\SentrySdk;
10+
use Sentry\State\Scope;
1111

1212
/**
1313
* @phpstan-import-type AttributeValue from LogAttribute
@@ -22,9 +22,9 @@ final class LogsAggregator
2222
private $logs = [];
2323

2424
/**
25-
* @param string $message see sprintf for a description of format
26-
* @param array<int, string|int|float> $values see sprintf for a description of values
27-
* @param array<string, AttributeValue> $attributes additional attributes to add to the log
25+
* @param string $message see sprintf for a description of format
26+
* @param array<int, string|int|float> $values see sprintf for a description of values
27+
* @param array<string, mixed> $attributes additional attributes to add to the log
2828
*/
2929
public function add(
3030
LogLevel $level,
@@ -38,42 +38,40 @@ public function add(
3838
$client = $hub->getClient();
3939

4040
// There is no need to continue if there is no client or if logs are disabled
41-
// @TODO: This might needs to be re-evaluated when we send logs to allow logging to start before init'ing the client
4241
if ($client === null || !$client->getOptions()->getEnableLogs()) {
4342
return;
4443
}
4544

46-
$span = $hub->getSpan();
47-
$traceId = $span !== null ? $span->getTraceId() : null;
45+
$scope = null;
4846

49-
$options = $client->getOptions();
47+
// This we push and pop a scope to get access to it because there is no accessor for the scope
48+
$hub->configureScope(function (Scope $hubScope) use (&$scope) {
49+
$scope = $hubScope;
50+
});
51+
52+
\assert($scope !== null, 'The scope comes from the hub and cannot be null at this point.');
53+
54+
$traceId = $scope->getPropagationContext()->getTraceId();
5055

5156
// @FIXME The SDK name and version won't work for Laravel & Symfony and other SDKs, needs to be more flexible
5257
$log = (new Log($timestamp, (string) $traceId, $level, vsprintf($message, $values)))
5358
->setAttribute('sentry.message.template', $message)
54-
->setAttribute('sentry.environment', $options->getEnvironment() ?? Event::DEFAULT_ENVIRONMENT)
55-
->setAttribute('sentry.sdk.name', Client::SDK_IDENTIFIER)
56-
->setAttribute('sentry.sdk.version', Client::SDK_VERSION);
59+
->setAttribute('sentry.trace.parent_span_id', $hub->getSpan() ? $hub->getSpan()->getSpanId() : null);
5760

5861
foreach ($values as $key => $value) {
5962
$log->setAttribute("sentry.message.parameter.{$key}", $value);
6063
}
6164

6265
foreach ($attributes as $key => $value) {
63-
$log->setAttribute($key, $value);
64-
}
65-
66-
if ($span !== null) {
67-
$log->setAttribute('sentry.trace.parent_span_id', (string) $span->getSpanId());
68-
}
69-
70-
// @TODO: Do we want to add the following attributes when we send the log rather then when we create the log?
71-
if ($options->getServerName() !== null) {
72-
$log->setAttribute('sentry.server.address', $options->getServerName());
73-
}
74-
75-
if ($options->getRelease() !== null) {
76-
$log->setAttribute('sentry.release', $options->getRelease());
66+
$attribute = LogAttribute::tryFromValue($value);
67+
68+
if ($attribute === null) {
69+
$client->getOptions()->getLoggerOrNullLogger()->info(
70+
\sprintf("Dropping log attribute {$key} with value of type '%s' because it is not serializable or an unsupported type.", \gettype($value))
71+
);
72+
} else {
73+
$log->setAttribute($key, $attribute);
74+
}
7775
}
7876

7977
$this->logs[] = $log;

‎tests/Logs/LogAttributeTest.php‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,22 @@
1414
final class LogAttributeTest extends TestCase
1515
{
1616
/**
17-
* @param AttributeValue $value
18-
* @param AttributeSerialized $expected
17+
* @param AttributeValue $value
18+
* @param AttributeSerialized|null $expected
1919
*
2020
* @dataProvider fromValueDataProvider
2121
*/
22-
public function testFromValue($value, array $expected, bool $expectError = false): void
22+
public function testFromValue($value, $expected): void
2323
{
24-
if ($expectError) {
25-
if (\PHP_VERSION_ID >= 70400) {
26-
$this->expectException(\Error::class);
27-
} else {
28-
$this->expectError();
29-
}
24+
$attribute = LogAttribute::tryFromValue($value);
25+
26+
if ($expected === null) {
27+
$this->assertNull($attribute);
28+
29+
return;
3030
}
3131

32-
$this->assertEquals($expected, LogAttribute::fromValue($value)->jsonSerialize());
32+
$this->assertEquals($expected, $attribute->jsonSerialize());
3333
}
3434

3535
public static function fromValueDataProvider(): \Generator
@@ -80,9 +80,18 @@ public function __toString(): string
8080
];
8181

8282
yield [
83-
new \stdClass(), // not stringable nor a scalar
84-
[],
85-
true,
83+
new class {},
84+
null,
85+
];
86+
87+
yield [
88+
new \stdClass(),
89+
null,
90+
];
91+
92+
yield [
93+
['key' => 'value'],
94+
null,
8695
];
8796
}
8897
}

‎tests/Logs/LogTest.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ public function testJsonSerializesToExpected(): void
1919
{
2020
$timestamp = microtime(true);
2121

22-
$log = new Log($timestamp, '123', LogLevel::debug(), 'foo', [
23-
'foo' => LogAttribute::fromValue('bar'),
24-
]);
22+
$log = new Log($timestamp, '123', LogLevel::debug(), 'foo');
23+
24+
$log->setAttribute('foo', 'bar');
25+
$log->setAttribute('should-be-missing', ['foo' => 'bar']);
2526

2627
$serialized = json_decode((string) json_encode($log), true);
2728

‎tests/Logs/LogsTest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,22 @@ public function testLogSentWhenEnabled(): void
5959
$this->assertNotNull(logger()->flush());
6060
}
6161

62+
public function testLogWithTemplate(): void
63+
{
64+
$this->assertEvent(function (Event $event) {
65+
$this->assertCount(1, $event->getLogs());
66+
67+
$logItem = $event->getLogs()[0]->jsonSerialize();
68+
69+
$this->assertEquals(LogLevel::info(), $logItem['level']);
70+
$this->assertEquals('Some info message', $logItem['body']);
71+
});
72+
73+
logger()->info('Some %s message', ['info']);
74+
75+
$this->assertNotNull(logger()->flush());
76+
}
77+
6278
/**
6379
* @param callable(Event): void $assert
6480
*/

0 commit comments

Comments
 (0)