From cd71beba0ff5d5f2794e4066b4ae4dcedaf107fc Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 13 Apr 2026 10:53:27 -0700 Subject: [PATCH 1/3] fix: setting of LogSeverity --- Logging/src/Connection/Gapic.php | 3 - Logging/tests/Unit/Connection/GapicTest.php | 3 +- Logging/tests/Unit/PsrLoggerTest.php | 61 ++++++++++++++++++--- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/Logging/src/Connection/Gapic.php b/Logging/src/Connection/Gapic.php index ca0d6be3047..8595bf32ff1 100644 --- a/Logging/src/Connection/Gapic.php +++ b/Logging/src/Connection/Gapic.php @@ -77,9 +77,6 @@ public function __construct(private array $config = []) 'json_payload' => function ($v) { return $this->formatStructForApi($v); }, - 'severity' => function ($v) { - return array_flip(Logger::getLogLevelMap())[strtoupper($v)]; - } ], [ 'google.protobuf.Duration' => function ($v) { return $this->formatDurationForApi($v); diff --git a/Logging/tests/Unit/Connection/GapicTest.php b/Logging/tests/Unit/Connection/GapicTest.php index ae385345419..e543ecbd21f 100644 --- a/Logging/tests/Unit/Connection/GapicTest.php +++ b/Logging/tests/Unit/Connection/GapicTest.php @@ -24,6 +24,7 @@ use Google\Cloud\Core\GrpcRequestWrapper; use Google\ApiCore\Serializer; use Google\Cloud\Logging\Connection\Gapic; +use Google\Cloud\Logging\Logger; use Google\Cloud\Logging\V2\Client\ConfigServiceV2Client; use Google\Cloud\Logging\V2\Client\LoggingServiceV2Client; use Google\Cloud\Logging\V2\Client\MetricsServiceV2Client; @@ -167,7 +168,7 @@ public function methodProvider() 'latency' => '1.0s' ], 'timestamp' => date('Y-m-d H:i:s', 100), - 'severity' => 'DEBUG', + 'severity' => Logger::DEBUG, ] ] ], diff --git a/Logging/tests/Unit/PsrLoggerTest.php b/Logging/tests/Unit/PsrLoggerTest.php index 1b9dad28be3..9df614ce1be 100644 --- a/Logging/tests/Unit/PsrLoggerTest.php +++ b/Logging/tests/Unit/PsrLoggerTest.php @@ -17,12 +17,23 @@ namespace Google\Cloud\Logging\Tests\Unit; +use Google\Api\MonitoredResource; use Google\Cloud\Core\Batch\OpisClosureSerializerV4; use Google\Cloud\Core\Report\EmptyMetadataProvider; use Google\Cloud\Logging\Logger; use Google\Cloud\Logging\PsrLogger; use Google\Cloud\Logging\Connection\Gapic; +use Google\Cloud\Logging\V2\Client\LoggingServiceV2Client; +use Google\Cloud\Logging\V2\LogEntry; +use Google\Cloud\Logging\V2\WriteLogEntriesRequest; +use Google\Cloud\Logging\V2\WriteLogEntriesResponse; +use Google\Protobuf\Internal\GPBType; +use Google\Protobuf\Internal\MapField; +use Google\Protobuf\Struct; +use Google\Protobuf\Timestamp; +use Google\Protobuf\Value; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Psr\Log\InvalidArgumentException; use Prophecy\PhpUnit\ProphecyTrait; @@ -49,7 +60,7 @@ public function setUp(): void public function getPsrLogger($connection, ?array $resource = null, ?array $labels = null, $messageKey = 'message') { - $logger = new Logger($connection->reveal(), $this->logName, $this->projectId, $resource, $labels); + $logger = new Logger($connection, $this->logName, $this->projectId, $resource, $labels); return new PsrLogger($logger, $messageKey, ['metadataProvider' => new EmptyMetadataProvider()]); } @@ -71,7 +82,7 @@ public function testWritesEntryWithDefinedLevels($level) ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection); + $psrLogger = $this->getPsrLogger($this->connection->reveal()); $this->assertNull( $psrLogger->$level($this->textPayload, [ @@ -80,6 +91,40 @@ public function testWritesEntryWithDefinedLevels($level) ); } + /** + * @dataProvider levelProvider + */ + public function testWritesEntryRequestWithDefinedLevels($level) + { + $map = new MapField(GPBType::STRING, GPBType::MESSAGE, Value::class); + $map['message'] = new Value(['string_value' => $this->textPayload]); + $entry = new LogEntry([ + 'severity' => array_flip(Logger::getLogLevelMap())[$level], + 'log_name' => $this->formattedName, + 'resource' => new MonitoredResource($this->resource), + 'timestamp' => new Timestamp(['seconds' => 100]), + 'json_payload' => new Struct(['fields' => $map]) + ]); + $request = new WriteLogEntriesRequest(['entries' => [$entry]]); + + $loggingClient = $this->prophesize(LoggingServiceV2Client::class); + $loggingClient->writeLogEntries($request, []) + ->shouldBeCalledOnce() + ->willReturn(new WriteLogEntriesResponse()); + + $connection = new Gapic([ + 'loggingGapicClient' => $loggingClient->reveal() + ]); + + $psrLogger = $this->getPsrLogger($connection); + + $this->assertNull( + $psrLogger->$level($this->textPayload, [ + 'stackdriverOptions' => ['timestamp' => date('Y-m-d H:i:s', 100)] + ]) + ); + } + public function levelProvider() { return [ @@ -109,7 +154,7 @@ public function testWritesEntry() ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection); + $psrLogger = $this->getPsrLogger($this->connection->reveal()); $this->assertNull( $psrLogger->log($this->severity, $this->textPayload, [ @@ -136,7 +181,7 @@ public function testPsrLoggerUsesDefaults() ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection, $resource, $labels); + $psrLogger = $this->getPsrLogger($this->connection->reveal(), $resource, $labels); $this->assertNull( $psrLogger->log($this->severity, $this->textPayload, [ @@ -164,7 +209,7 @@ public function testOverridePsrLoggerDefaults() ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection, null, $defaultLabels); + $psrLogger = $this->getPsrLogger($this->connection->reveal(), null, $defaultLabels); $this->assertNull( $psrLogger->log($this->severity, $this->textPayload, [ @@ -181,7 +226,7 @@ public function testLogThrowsExceptionWithInvalidLevel() { $this->expectException(InvalidArgumentException::class); - $psrLogger = $this->getPsrLogger($this->connection); + $psrLogger = $this->getPsrLogger($this->connection->reveal()); $psrLogger->log('INVALID-LEVEL', $this->textPayload); } @@ -215,7 +260,7 @@ public function testUsesCustomMessageKey() ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection, null, null, $customKey); + $psrLogger = $this->getPsrLogger($this->connection->reveal(), null, null, $customKey); $psrLogger->log($this->severity, $this->textPayload, [ 'stackdriverOptions' => ['timestamp' => null] ]); @@ -291,7 +336,7 @@ private function expectLogWithExceptionInContext($throwable) ]) ->willReturn([]) ->shouldBeCalledTimes(1); - $psrLogger = $this->getPsrLogger($this->connection); + $psrLogger = $this->getPsrLogger($this->connection->reveal()); $psrLogger->log($this->severity, $this->textPayload, [ 'exception' => $throwable, 'stackdriverOptions' => ['timestamp' => null] From 867d9bf388280ed036c2a346ed285eb7e8400fb4 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 13 Apr 2026 11:25:49 -0700 Subject: [PATCH 2/3] fix updateSink and add sink tests --- Logging/src/Connection/Gapic.php | 4 +- Logging/tests/Unit/Connection/GapicTest.php | 2 - Logging/tests/Unit/PsrLoggerTest.php | 1 - Logging/tests/Unit/SinkTest.php | 60 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/Logging/src/Connection/Gapic.php b/Logging/src/Connection/Gapic.php index 8595bf32ff1..571bb2a6bd1 100644 --- a/Logging/src/Connection/Gapic.php +++ b/Logging/src/Connection/Gapic.php @@ -183,11 +183,13 @@ public function getSink(array $args = []) { /** * @var GetSinkRequest $getSinkRequest + * @var LogSink $_logSink * @var array $callOptions */ - [$getSinkRequest, $callOptions] = $this->validateOptions( + [$getSinkRequest, $_sink, $callOptions] = $this->validateOptions( $args, new GetSinkRequest(), + new LogSink(), // unused, for backwards compatibility CallOptions::class ); return $this->handleResponse( diff --git a/Logging/tests/Unit/Connection/GapicTest.php b/Logging/tests/Unit/Connection/GapicTest.php index e543ecbd21f..aa415801ec7 100644 --- a/Logging/tests/Unit/Connection/GapicTest.php +++ b/Logging/tests/Unit/Connection/GapicTest.php @@ -36,7 +36,6 @@ use Google\Cloud\Logging\V2\GetLogMetricRequest; use Google\Cloud\Logging\V2\GetSinkRequest; use Google\Cloud\Logging\V2\ListLogEntriesRequest; -use Google\Cloud\Logging\V2\ListLogEntriesResponse; use Google\Cloud\Logging\V2\ListLogMetricsRequest; use Google\Cloud\Logging\V2\ListSinksRequest; use Google\Cloud\Logging\V2\LogEntry; @@ -47,7 +46,6 @@ use Google\Cloud\Logging\V2\WriteLogEntriesRequest; use Google\Cloud\Logging\V2\WriteLogEntriesResponse; use Google\Protobuf\Internal\Message; -use Google\Protobuf\RepeatedField; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; diff --git a/Logging/tests/Unit/PsrLoggerTest.php b/Logging/tests/Unit/PsrLoggerTest.php index 9df614ce1be..39e1b0c6a9a 100644 --- a/Logging/tests/Unit/PsrLoggerTest.php +++ b/Logging/tests/Unit/PsrLoggerTest.php @@ -33,7 +33,6 @@ use Google\Protobuf\Timestamp; use Google\Protobuf\Value; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; use Psr\Log\InvalidArgumentException; use Prophecy\PhpUnit\ProphecyTrait; diff --git a/Logging/tests/Unit/SinkTest.php b/Logging/tests/Unit/SinkTest.php index 6959d66de9f..402568211d1 100644 --- a/Logging/tests/Unit/SinkTest.php +++ b/Logging/tests/Unit/SinkTest.php @@ -20,6 +20,10 @@ use Google\ApiCore\ApiException; use Google\Cloud\Logging\Sink; use Google\Cloud\Logging\Connection\Gapic; +use Google\Cloud\Logging\V2\Client\ConfigServiceV2Client; +use Google\Cloud\Logging\V2\GetSinkRequest; +use Google\Cloud\Logging\V2\LogSink; +use Google\Cloud\Logging\V2\UpdateSinkRequest; use Google\Rpc\Code; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -118,6 +122,62 @@ public function testUpdatesDataWithoutCachedData() $this->assertEquals($this->sinkData['destination'], $sink->info()['destination']); } + public function testUpdate() + { + $updatedSink = new LogSink([ + 'destination' => 'a new destination', + 'filter' => 'name=filteredname', + ]); + $updateRequest = new UpdateSinkRequest([ + 'sink_name' => $this->formattedName, + 'sink' => $updatedSink, + ]); + $configClient = $this->prophesize(ConfigServiceV2Client::class); + $configClient->updateSink($updateRequest, []) + ->willReturn($updatedSink) + ->shouldBeCalledOnce(); + $getRequest = new GetSinkRequest([ + 'sink_name' => $this->formattedName, + ]); + $configClient->getSink($getRequest, []) + ->willReturn($updatedSink) + ->shouldBeCalledOnce(); + + $connection = new Gapic(['configGapicClient' => $configClient->reveal()]); + $sink = new Sink($connection, $this->sinkName, $this->projectId); + + $sink->update(['destination' => 'a new destination', 'filter' => 'name=filteredname']); + + $this->assertEquals('a new destination', $sink->info()['destination']); + $this->assertEquals('name=filteredname', $sink->info()['filter']); + } + + public function testUpdateWithInfoDoesNotMakeGetRequest() + { + $updatedSink = new LogSink([ + 'destination' => 'a new destination', + 'filter' => 'name=filteredname', + ]); + $request = new UpdateSinkRequest([ + 'sink_name' => $this->formattedName, + 'sink' => $updatedSink, + ]); + $configClient = $this->prophesize(ConfigServiceV2Client::class); + $configClient->updateSink($request, []) + ->willReturn($updatedSink) + ->shouldBeCalledOnce(); + $configClient->getSink(Argument::type(GetSinkRequest::class), []) + ->shouldNotBeCalled(); + + $connection = new Gapic(['configGapicClient' => $configClient->reveal()]); + $sink = new Sink($connection, $this->sinkName, $this->projectId, ['destination' => 'previous destination']); + + $sink->update(['destination' => 'a new destination', 'filter' => 'name=filteredname']); + + $this->assertEquals('a new destination', $sink->info()['destination']); + $this->assertEquals('name=filteredname', $sink->info()['filter']); + } + public function testGetsInfo() { $this->connection->getSink(Argument::any())->shouldNotBeCalled(); From b744c963aeabec8558d025a45b348fa25654f126 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 13 Apr 2026 12:10:47 -0700 Subject: [PATCH 3/3] validate log level for string/int, add test for entry deserialization --- Logging/src/Connection/Gapic.php | 48 +++++++++++---------- Logging/src/PsrLogger.php | 15 ++++--- Logging/tests/Unit/Connection/GapicTest.php | 40 +++++++++++++++++ Logging/tests/Unit/PsrLoggerTest.php | 10 ++--- 4 files changed, 81 insertions(+), 32 deletions(-) diff --git a/Logging/src/Connection/Gapic.php b/Logging/src/Connection/Gapic.php index 571bb2a6bd1..528742272a8 100644 --- a/Logging/src/Connection/Gapic.php +++ b/Logging/src/Connection/Gapic.php @@ -63,28 +63,32 @@ class Gapic */ public function __construct(private array $config = []) { - $this->serializer = new Serializer([ - 'timestamp' => function ($v) { - return $this->formatTimestampFromApi($v); - }, - 'severity' => function ($v) { - return Logger::getLogLevelMap()[$v]; - }, - 'json_payload' => function ($v) { - return $this->unpackStructFromApi($v); - } - ], [], [ - 'json_payload' => function ($v) { - return $this->formatStructForApi($v); - }, - ], [ - 'google.protobuf.Duration' => function ($v) { - return $this->formatDurationForApi($v); - }, - 'google.protobuf.Timestamp' => function ($v) { - return $this->formatTimestampForApi($v); - } - ]); + $this->serializer = new Serializer( + fieldTransformers: [ + 'timestamp' => function ($v) { + return $this->formatTimestampFromApi($v); + }, + 'severity' => function ($v) { + return Logger::getLogLevelMap()[$v]; + }, + 'json_payload' => function ($v) { + return $this->unpackStructFromApi($v); + } + ], + decodeFieldTransformers: [ + 'json_payload' => function ($v) { + return $this->formatStructForApi($v); + }, + ], + decodeMessageTypeTransformers: [ + 'google.protobuf.Duration' => function ($v) { + return $this->formatDurationForApi($v); + }, + 'google.protobuf.Timestamp' => function ($v) { + return $this->formatTimestampForApi($v); + } + ] + ); $this->optionsValidator = new OptionsValidator($this->serializer); } diff --git a/Logging/src/PsrLogger.php b/Logging/src/PsrLogger.php index a6e694d0f42..d26d6af2815 100644 --- a/Logging/src/PsrLogger.php +++ b/Logging/src/PsrLogger.php @@ -374,7 +374,7 @@ public function debug(Stringable|string $message, array $context = []): void */ public function log($level, Stringable|string $message, array $context = []): void { - $this->validateLogLevel($level); + $level = $this->validateLogLevel($level); $options = []; if (isset($context['exception']) @@ -520,16 +520,21 @@ protected function getCallback() * Validates whether or not the provided log level exists. * * @param string|int $level The severity of the log entry. - * @return bool + * @return int * @throws \InvalidArgumentException */ - private function validateLogLevel($level) + private function validateLogLevel($level): int { $map = Logger::getLogLevelMap(); $level = (string) $level; - if (isset($map[$level]) || isset(array_flip($map)[strtoupper($level)])) { - return true; + if (isset($map[$level])) { + return $level; + } + + $levelFromString = array_flip($map)[strtoupper($level)] ?? null; + if (!is_null($levelFromString)) { + return $levelFromString; } throw new InvalidArgumentException("Severity level '$level' is not defined."); diff --git a/Logging/tests/Unit/Connection/GapicTest.php b/Logging/tests/Unit/Connection/GapicTest.php index aa415801ec7..35265bc3a8e 100644 --- a/Logging/tests/Unit/Connection/GapicTest.php +++ b/Logging/tests/Unit/Connection/GapicTest.php @@ -17,6 +17,7 @@ namespace Google\Cloud\Logging\Tests\Unit\Connection; +use Google\ApiCore\GPBType; use Google\ApiCore\Page; use Google\ApiCore\PagedListResponse; use Google\Cloud\Logging\Connection\Grpc; @@ -25,6 +26,7 @@ use Google\ApiCore\Serializer; use Google\Cloud\Logging\Connection\Gapic; use Google\Cloud\Logging\Logger; +use Google\Cloud\Logging\Type\LogSeverity; use Google\Cloud\Logging\V2\Client\ConfigServiceV2Client; use Google\Cloud\Logging\V2\Client\LoggingServiceV2Client; use Google\Cloud\Logging\V2\Client\MetricsServiceV2Client; @@ -36,6 +38,7 @@ use Google\Cloud\Logging\V2\GetLogMetricRequest; use Google\Cloud\Logging\V2\GetSinkRequest; use Google\Cloud\Logging\V2\ListLogEntriesRequest; +use Google\Cloud\Logging\V2\ListLogEntriesResponse; use Google\Cloud\Logging\V2\ListLogMetricsRequest; use Google\Cloud\Logging\V2\ListSinksRequest; use Google\Cloud\Logging\V2\LogEntry; @@ -45,7 +48,11 @@ use Google\Cloud\Logging\V2\UpdateSinkRequest; use Google\Cloud\Logging\V2\WriteLogEntriesRequest; use Google\Cloud\Logging\V2\WriteLogEntriesResponse; +use Google\Protobuf\Internal\MapField; use Google\Protobuf\Internal\Message; +use Google\Protobuf\Struct; +use Google\Protobuf\Timestamp; +use Google\Protobuf\Value; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -260,4 +267,37 @@ public function methodProvider() ] ]; } + + public function testSerializedEntry() + { + $map = new MapField(GPBType::STRING, GPBType::MESSAGE, Value::class); + $map['message'] = new Value(['string_value' => 'aPayload']); + + $entry = new LogEntry([ + 'severity' => LogSeverity::DEBUG, + 'timestamp' => new Timestamp(['seconds' => 100]), + 'json_payload' => new Struct(['fields' => $map]), + ]); + + $page = $this->prophesize(Page::class); + $page->getResponseObject()->willReturn(new ListLogEntriesResponse(['entries' => [$entry]])); + $pagedListResponse = $this->prophesize(PagedListResponse::class); + $pagedListResponse->getPage() + ->willReturn($page->reveal()); + + $this->loggingGapicClient->listLogEntries( + new ListLogEntriesRequest(['filter' => 'logName=foo']), + Argument::type('array') + ) + ->shouldBeCalledOnce() + ->willReturn($pagedListResponse->reveal()); + + $connection = new Gapic(['loggingGapicClient' => $this->loggingGapicClient->reveal()]); + + $entries = $connection->listLogEntries(['filter' => 'logName=foo']); + $info = $entries['entries'][0]; + $this->assertEquals('DEBUG', $info['severity']); + $this->assertEquals(date('Y-m-d\TH:i:s.u\Z', 100), $info['timestamp']); + $this->assertEquals(['message' => 'aPayload'], $info['jsonPayload']); + } } diff --git a/Logging/tests/Unit/PsrLoggerTest.php b/Logging/tests/Unit/PsrLoggerTest.php index 39e1b0c6a9a..ed56f2fed84 100644 --- a/Logging/tests/Unit/PsrLoggerTest.php +++ b/Logging/tests/Unit/PsrLoggerTest.php @@ -143,7 +143,7 @@ public function testWritesEntry() $this->connection->writeLogEntries([ 'entries' => [ [ - 'severity' => $this->severity, + 'severity' => array_flip(Logger::getLogLevelMap())[$this->severity], 'jsonPayload' => ['message' => $this->textPayload], 'logName' => $this->formattedName, 'resource' => $this->resource, @@ -169,7 +169,7 @@ public function testPsrLoggerUsesDefaults() $this->connection->writeLogEntries([ 'entries' => [ [ - 'severity' => $this->severity, + 'severity' => array_flip(Logger::getLogLevelMap())[$this->severity], 'jsonPayload' => ['message' => $this->textPayload], 'logName' => $this->formattedName, 'resource' => $resource, @@ -197,7 +197,7 @@ public function testOverridePsrLoggerDefaults() $this->connection->writeLogEntries([ 'entries' => [ [ - 'severity' => $this->severity, + 'severity' => array_flip(Logger::getLogLevelMap())[$this->severity], 'jsonPayload' => ['message' => $this->textPayload], 'logName' => $this->formattedName, 'resource' => $newResource, @@ -249,7 +249,7 @@ public function testUsesCustomMessageKey() $this->connection->writeLogEntries([ 'entries' => [ [ - 'severity' => $this->severity, + 'severity' => array_flip(Logger::getLogLevelMap())[$this->severity], 'jsonPayload' => [$customKey => $this->textPayload], 'logName' => $this->formattedName, 'resource' => $this->resource, @@ -322,7 +322,7 @@ private function expectLogWithExceptionInContext($throwable) $this->connection->writeLogEntries([ 'entries' => [ [ - 'severity' => $this->severity, + 'severity' => array_flip(Logger::getLogLevelMap())[$this->severity], 'jsonPayload' => [ 'message' => $this->textPayload, 'exception' => (string) $throwable