-
Notifications
You must be signed in to change notification settings - Fork 1
Enhanced context format #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
a2aff82
93024f0
3ca8bb6
00ac89b
af98022
4cbf8ac
0027fd2
0406f7f
50ba224
33f5a80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal\ContextProcessor; | ||
|
|
||
| use RoadRunner\PsrLogger\Internal\ObjectProcessor; | ||
|
|
||
| /** | ||
| * Processor for DateTime objects. | ||
| * | ||
| * Converts DateTime and DateTimeImmutable objects to ISO 8601 format | ||
| * for consistent structured logging. | ||
| * | ||
| * @implements ObjectProcessor<\DateTimeInterface> | ||
| * | ||
| * @internal | ||
| */ | ||
| final class DateTimeProcessor implements ObjectProcessor | ||
| { | ||
| public function canProcess(object $value): bool | ||
| { | ||
| return $value instanceof \DateTimeInterface; | ||
| } | ||
|
|
||
| public function process(object $value, callable $processor): mixed | ||
| { | ||
| return $value->format(\DateTimeInterface::ATOM); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal\ContextProcessor; | ||
|
|
||
| use RoadRunner\PsrLogger\Internal\ObjectProcessor; | ||
|
|
||
| /** | ||
| * Fallback processor for unknown objects. | ||
| * | ||
| * @internal | ||
| * | ||
| * @implements ObjectProcessor<object> | ||
| */ | ||
| final class FallbackProcessor implements ObjectProcessor | ||
| { | ||
| public function canProcess(object $value): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| public function process(object $value, callable $processor): string | ||
| { | ||
| $result = ['@class' => $value::class] + \get_object_vars($value); | ||
| foreach ($result as $k => &$v) { | ||
| if ($v === $value) { | ||
| unset($result[$k]); | ||
| } | ||
|
|
||
| $v = $processor($v); | ||
| } | ||
|
|
||
| return $result; | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal\ContextProcessor; | ||
|
|
||
| use RoadRunner\PsrLogger\Internal\ObjectProcessor; | ||
|
|
||
| /** | ||
| * Converts Stringable objects to their string representation. | ||
| * | ||
| * @internal | ||
| * | ||
| * @implements ObjectProcessor<\Stringable> | ||
| */ | ||
| final class StringableProcessor implements ObjectProcessor | ||
| { | ||
| public function canProcess(object $value): bool | ||
| { | ||
| return $value instanceof \Stringable; | ||
| } | ||
|
|
||
| public function process(object $value, callable $processor): mixed | ||
| { | ||
| return (string) $value; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal\ContextProcessor; | ||
|
|
||
| use RoadRunner\PsrLogger\Internal\ObjectProcessor; | ||
|
|
||
| /** | ||
| * Converts exceptions to structured data containing class, message, | ||
| * code, file, line, and stack trace information. | ||
| * | ||
| * @internal | ||
| * | ||
| * @implements ObjectProcessor<\Throwable> | ||
| */ | ||
| final class ThrowableProcessor implements ObjectProcessor | ||
| { | ||
| public function canProcess(object $value): bool | ||
| { | ||
| return $value instanceof \Throwable; | ||
| } | ||
|
|
||
| public function process(object $value, callable $processor): array | ||
| { | ||
| return [ | ||
| 'class' => \get_class($value), | ||
| 'message' => $value->getMessage(), | ||
| 'code' => $value->getCode(), | ||
| 'file' => $value->getFile(), | ||
| 'line' => $value->getLine(), | ||
| 'trace' => $value->getTraceAsString(), | ||
| ]; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal; | ||
|
|
||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\DateTimeProcessor; | ||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\FallbackProcessor; | ||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\StringableProcessor; | ||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\ThrowableProcessor; | ||
|
|
||
| final class DefaultProcessor | ||
| { | ||
| /** @var list<ObjectProcessor> */ | ||
| private array $processors = []; | ||
|
|
||
| public static function createDefault(): self | ||
| { | ||
| $self = new self(); | ||
| $self->processors = [ | ||
| new DateTimeProcessor(), | ||
| new StringableProcessor(), | ||
| new ThrowableProcessor(), | ||
| new FallbackProcessor(), | ||
| ]; | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Copy the current object and add Object Processors before existing ones. | ||
| */ | ||
| public function withObjectProcessors(ObjectProcessor ...$processors): self | ||
| { | ||
| $clone = clone $this; | ||
| $clone->processors = \array_merge(\array_values($processors), $clone->processors); | ||
| return $clone; | ||
| } | ||
|
|
||
| public function __invoke(mixed $value): mixed | ||
| { | ||
| if (\is_resource($value)) { | ||
| return \get_resource_type($value) . ' resource'; | ||
| } | ||
|
|
||
| if (\is_array($value)) { | ||
| foreach ($value as &$v) { | ||
| $v = $this($v); | ||
| } | ||
| } | ||
|
|
||
| if (\is_object($value)) { | ||
| foreach ($this->processors as $processor) { | ||
| if ($processor->canProcess($value)) { | ||
| return $processor->process($value, $this); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return $value; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Internal; | ||
|
|
||
| /** | ||
| * Converts an object into a scalar or an arra for serializable logger context. | ||
| * | ||
| * @template T | ||
| * | ||
| * @internal | ||
| */ | ||
| interface ObjectProcessor | ||
| { | ||
| /** | ||
| * Check if this processor can handle the given value. | ||
| */ | ||
| public function canProcess(object $value): bool; | ||
|
|
||
| /** | ||
| * @param T $value | ||
| * @param callable(mixed): mixed $processor Function to process nested object values | ||
| */ | ||
| public function process(object $value, callable $processor): mixed; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,16 +10,21 @@ | |
| use Psr\Log\InvalidArgumentException as PsrInvalidArgumentException; | ||
| use RoadRunner\Logger\Logger as AppLogger; | ||
| use RoadRunner\Logger\LogLevel; | ||
| use RoadRunner\PsrLogger\Internal\DefaultProcessor; | ||
| use RoadRunner\PsrLogger\Internal\ObjectProcessor; | ||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\ObjectProcessorManager; | ||
|
|
||
| class RpcLogger implements LoggerInterface | ||
| { | ||
| use LoggerTrait; | ||
|
|
||
| private readonly AppLogger $logger; | ||
| private readonly ObjectProcessor $contextProcessor; | ||
|
|
||
| public function __construct(AppLogger $logger) | ||
| public function __construct(AppLogger $logger, ?callable $processor = null) | ||
| { | ||
| $this->logger = $logger; | ||
| $this->contextProcessor = $processor ?? DefaultProcessor::createDefault(); | ||
| } | ||
roxblnfk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
|
|
@@ -32,23 +37,25 @@ public function __construct(AppLogger $logger) | |
| public function log($level, \Stringable|string $message, array $context = []): void | ||
| { | ||
| $normalizedLevel = \strtolower(match (true) { | ||
| \is_string($level), | ||
| \is_string($level) => $level, | ||
| $level instanceof \Stringable => (string) $level, | ||
| $level instanceof \BackedEnum => (string) $level->value, | ||
| $level instanceof LogLevel => $level->name, | ||
| default => throw new PsrInvalidArgumentException('Invalid log level type provided.'), | ||
| }); | ||
|
|
||
| /** @var array<string, mixed> $context */ | ||
| // Process context data for structured logging using the processor manager | ||
| $processedContext = $this->contextProcessor->processContext($context); | ||
|
|
||
| match ($normalizedLevel) { | ||
| PsrLogLevel::EMERGENCY, | ||
| PsrLogLevel::ALERT, | ||
| PsrLogLevel::CRITICAL, | ||
| PsrLogLevel::ERROR => $this->logger->error($message, $context), | ||
| PsrLogLevel::WARNING => $this->logger->warning($message, $context), | ||
| PsrLogLevel::NOTICE, PsrLogLevel::INFO => $this->logger->info((string) $message, $context), | ||
| 'log' => $this->logger->log((string) $message, $context), | ||
| PsrLogLevel::DEBUG => $this->logger->debug($message, $context), | ||
| PsrLogLevel::ERROR => $this->logger->error($message, $processedContext), | ||
| PsrLogLevel::WARNING => $this->logger->warning($message, $processedContext), | ||
| PsrLogLevel::NOTICE, PsrLogLevel::INFO => $this->logger->info((string) $message, $processedContext), | ||
| 'log' => $this->logger->log((string) $message, $processedContext), | ||
| PsrLogLevel::DEBUG => $this->logger->debug($message, $processedContext), | ||
|
Comment on lines
43
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify: is custom level 'log' intentionally supported as an input level? PSR-3 levels don’t include 'log'. Keeping this branch is fine if RoadRunner’s Logger explicitly accepts a 'log' level alias; otherwise consider removing it to avoid accepting invalid levels. 🌐 Web query: 💡 Result: RoadRunner\Logger\Logger does not accept a 'log' level as a distinct log level; 'log' is only the generic method name defined by PSR-3 for submitting logs at any severity. Only the standard PSR-3 levels (emergency, alert, critical, error, warning, notice, info, debug) are accepted; custom or unknown levels, such as a literal 'log' level, are not supported[6][4][10]. Supporting details:
Summary of best practice: Additional context:
Citations:
Remove unsupported ‘log’ level and enforce PSR-3 compliance • Remove the default => throw new \Psr\Log\InvalidArgumentException(
sprintf('Unsupported log level "%s". Valid levels: emergency, alert, critical, error, warning, notice, info, debug.', $normalizedLevel)
),• Update or add tests to assert that providing an invalid level (including “log”) triggers the exception. This change ensures that only valid PSR-3 levels are accepted and that any unknown level is explicitly rejected, in line with PSR-3 guidelines. 🤖 Prompt for AI Agents |
||
| default => throw new PsrInvalidArgumentException("Invalid log level `$normalizedLevel` provided."), | ||
|
Comment on lines
+56
to
61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential TypeError: cast Stringable messages to string for all logger calls AppLogger methods likely accept string messages. Passing a \Stringable without casting can throw a TypeError with strict types. Some branches already cast; others don’t. Apply this diff to consistently cast: - PsrLogLevel::ERROR => $this->logger->error($message, $processedContext),
- PsrLogLevel::WARNING => $this->logger->warning($message, $processedContext),
- PsrLogLevel::NOTICE, PsrLogLevel::INFO => $this->logger->info((string) $message, $processedContext),
- 'log' => $this->logger->log((string) $message, $processedContext),
- PsrLogLevel::DEBUG => $this->logger->debug($message, $processedContext),
+ PsrLogLevel::ERROR => $this->logger->error((string) $message, $processedContext),
+ PsrLogLevel::WARNING => $this->logger->warning((string) $message, $processedContext),
+ PsrLogLevel::NOTICE, PsrLogLevel::INFO => $this->logger->info((string) $message, $processedContext),
+ 'log' => $this->logger->log((string) $message, $processedContext),
+ PsrLogLevel::DEBUG => $this->logger->debug((string) $message, $processedContext),Optional follow-up: cast once after processing context to reduce repetition. // Process context data for structured logging using the processor manager
$processedContext = $this->contextProcessor->processContext($context);
+ $message = (string) $message;Then remove per-branch casts. 🤖 Prompt for AI Agents |
||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RoadRunner\PsrLogger\Tests\Unit\ContextProcessor; | ||
|
|
||
| use PHPUnit\Framework\Attributes\CoversClass; | ||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
| use PHPUnit\Framework\TestCase; | ||
| use RoadRunner\PsrLogger\Internal\ContextProcessor\BuiltInTypeProcessor; | ||
|
|
||
| #[CoversClass(BuiltInTypeProcessor::class)] | ||
| class BuiltInTypeProcessorTest extends TestCase | ||
| { | ||
| private BuiltInTypeProcessor $processor; | ||
|
|
||
| public static function builtInTypeValuesProvider(): array | ||
| { | ||
| return [ | ||
| 'string' => ['test string', true], | ||
| 'integer' => [42, true], | ||
| 'float' => [3.14, true], | ||
| 'boolean true' => [true, true], | ||
| 'boolean false' => [false, true], | ||
| 'null' => [null, true], // null is now handled by BuiltInTypeProcessor | ||
| 'empty array' => [[], true], | ||
| 'simple array' => [[1, 2, 3], true], | ||
| 'associative array' => [['key' => 'value'], true], | ||
| 'object' => [new \stdClass(), false], | ||
| 'resource' => [\fopen('php://memory', 'r'), false], | ||
| ]; | ||
| } | ||
|
|
||
| #[DataProvider('builtInTypeValuesProvider')] | ||
| public function testCanProcessBuiltInTypes(mixed $value, bool $expected): void | ||
| { | ||
| $this->assertSame($expected, $this->processor->canProcess($value)); | ||
| } | ||
|
|
||
| public function testProcessNull(): void | ||
| { | ||
| $recursiveProcessor = static fn($v) => $v; | ||
| $result = $this->processor->process(null, $recursiveProcessor); | ||
| $this->assertNull($result); | ||
| } | ||
|
|
||
| public function testProcessScalarValues(): void | ||
| { | ||
| $values = ['test string', 42, 3.14, true, false]; | ||
| $recursiveProcessor = static fn($v) => $v; | ||
|
|
||
| foreach ($values as $value) { | ||
| $result = $this->processor->process($value, $recursiveProcessor); | ||
| $this->assertSame($value, $result); | ||
| } | ||
| } | ||
|
|
||
| public function testProcessSimpleArray(): void | ||
| { | ||
| $array = [1, 2, 'three', true]; | ||
| $recursiveProcessor = static fn($v) => $v; // Identity function for simple values | ||
|
|
||
| $result = $this->processor->process($array, $recursiveProcessor); | ||
|
|
||
| $this->assertSame([1, 2, 'three', true], $result); | ||
| } | ||
|
|
||
| public function testProcessNestedArray(): void | ||
| { | ||
| $array = [ | ||
| 'level1' => [ | ||
| 'level2' => [ | ||
| 'value' => 'deep', | ||
| ], | ||
| ], | ||
| ]; | ||
|
|
||
| // Mock recursive processor that adds a prefix to strings and handles arrays recursively | ||
| $recursiveProcessor = static function ($value) use (&$recursiveProcessor) { | ||
| if (\is_string($value)) { | ||
| return 'processed:' . $value; | ||
| } | ||
| if (\is_array($value)) { | ||
| $processed = []; | ||
| foreach ($value as $key => $item) { | ||
| $processed[$key] = $recursiveProcessor($item); | ||
| } | ||
| return $processed; | ||
| } | ||
| return $value; | ||
| }; | ||
|
|
||
| $result = $this->processor->process($array, $recursiveProcessor); | ||
|
|
||
| $this->assertArrayHasKey('level1', $result); | ||
| $this->assertIsArray($result['level1']); | ||
| $this->assertArrayHasKey('level2', $result['level1']); | ||
| $this->assertIsArray($result['level1']['level2']); | ||
| $this->assertSame('processed:deep', $result['level1']['level2']['value']); | ||
| } | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
| $this->processor = new BuiltInTypeProcessor(); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.