From 56f50dc35c22628eb3e96ae2220cfd5bdc177568 Mon Sep 17 00:00:00 2001 From: Matt Kynaston Date: Wed, 4 Sep 2024 18:02:37 +0100 Subject: [PATCH 1/4] feat: [Logging] Add resource labels to logs from Cloud Run services and jobs --- .../Report/CloudRunJobMetadataProvider.php | 144 ++++++++++++++++++ Core/src/Report/CloudRunMetadataProvider.php | 107 ------------- .../CloudRunServiceMetadataProvider.php | 138 +++++++++++++++++ Core/src/Report/MetadataProviderUtils.php | 11 +- .../CloudRunJobMetadataProviderTest.php | 94 ++++++++++++ .../Report/CloudRunMetadataProviderTest.php | 52 ------- .../CloudRunServiceMetadataProviderTest.php | 94 ++++++++++++ .../Unit/Report/MetadataProviderUtilsTest.php | 44 ++++++ Logging/tests/Unit/PsrLoggerBatchTest.php | 22 ++- 9 files changed, 541 insertions(+), 165 deletions(-) create mode 100644 Core/src/Report/CloudRunJobMetadataProvider.php delete mode 100644 Core/src/Report/CloudRunMetadataProvider.php create mode 100644 Core/src/Report/CloudRunServiceMetadataProvider.php create mode 100644 Core/tests/Unit/Report/CloudRunJobMetadataProviderTest.php delete mode 100644 Core/tests/Unit/Report/CloudRunMetadataProviderTest.php create mode 100644 Core/tests/Unit/Report/CloudRunServiceMetadataProviderTest.php diff --git a/Core/src/Report/CloudRunJobMetadataProvider.php b/Core/src/Report/CloudRunJobMetadataProvider.php new file mode 100644 index 000000000000..ca5bd591cbde --- /dev/null +++ b/Core/src/Report/CloudRunJobMetadataProvider.php @@ -0,0 +1,144 @@ +serviceId = $env['CLOUD_RUN_JOB'] ?? 'unknown-job'; + $this->revisionId = $env['CLOUD_RUN_EXECUTION'] ?? ''; + $this->taskIndex = $env['CLOUD_RUN_TASK_INDEX'] ?? ''; + $this->taskAttempt = $env['CLOUD_RUN_TASK_ATTEMPT'] ?? ''; + + $this->traceId = isset($env['HTTP_X_CLOUD_TRACE_CONTEXT']) + ? \substr($env['HTTP_X_CLOUD_TRACE_CONTEXT'], 0, 32) + : null; + + $this->metadata = $metadata ?? new Metadata(); + $this->region = \basename($this->metadata->get('instance/region')); + $this->instanceId = $this->metadata->get('instance/id'); + } + + /** + * @return array + */ + public function monitoredResource() + { + return [ + 'type' => 'cloud_run_job', + 'labels' => [ + 'job_name' => $this->serviceId, + 'location' => $this->region, + 'project_id' => $this->projectId(), + ], + ]; + } + + /** + * Return the project id. + * @return string + */ + public function projectId() + { + return $this->metadata->getProjectId(); + } + + /** + * Return the service id. + * @return string + */ + public function serviceId() + { + return $this->serviceId; + } + + /** + * Return the version id. + * @return string + */ + public function versionId() + { + return $this->revisionId; + } + + /** + * Return the labels. + * @return array + */ + public function labels() + { + $labels = [ + 'instanceId' => $this->instanceId, + 'run.googleapis.com/execution_name' => $this->revisionId, + 'run.googleapis.com/task_attempt' => $this->taskAttempt, + 'run.googleapis.com/task_index' => $this->taskIndex, + 'run.googleapis.com/trace_id' => $this->traceId, + ]; + return \array_filter($labels); + } +} diff --git a/Core/src/Report/CloudRunMetadataProvider.php b/Core/src/Report/CloudRunMetadataProvider.php deleted file mode 100644 index 5b7d771924b4..000000000000 --- a/Core/src/Report/CloudRunMetadataProvider.php +++ /dev/null @@ -1,107 +0,0 @@ -serviceId = isset($env['K_SERVICE']) - ? $env['K_SERVICE'] - : 'unknown-service'; - $this->revisionId = isset($env['K_REVISION']) - ? $env['K_REVISION'] - : 'unknown-revision'; - $this->traceId = isset($env['HTTP_X_CLOUD_TRACE_CONTEXT']) - ? substr($env['HTTP_X_CLOUD_TRACE_CONTEXT'], 0, 32) - : null; - $this->metadata = new Metadata(); - } - - /** - * not implemented - * @TODO - */ - public function monitoredResource() - { - return []; - } - - /** - * not implemented - * @TODO - */ - public function projectId() - { - return $this->metadata->getProjectId(); - } - - /** - * Return the service id. - * @return string - */ - public function serviceId() - { - return $this->serviceId; - } - - /** - * Return the version id. - * @return string - */ - public function versionId() - { - return $this->revisionId; - } - - /** - * Return the labels. We need to evaluate $_SERVER for each request. - * @return array - */ - public function labels() - { - return !empty($this->traceId) - ? ['run.googleapis.com/trace_id' => $this->traceId ] - : []; - } -} diff --git a/Core/src/Report/CloudRunServiceMetadataProvider.php b/Core/src/Report/CloudRunServiceMetadataProvider.php new file mode 100644 index 000000000000..0c16cce17b09 --- /dev/null +++ b/Core/src/Report/CloudRunServiceMetadataProvider.php @@ -0,0 +1,138 @@ +configurationId = $env['K_CONFIGURATION'] ?? 'unknown-configuration'; + $this->serviceId = $env['K_SERVICE'] ?? 'unknown-service'; + $this->revisionId = $env['K_REVISION'] ?? 'unknown-revision'; + $this->traceId = isset($env['HTTP_X_CLOUD_TRACE_CONTEXT']) + ? \substr($env['HTTP_X_CLOUD_TRACE_CONTEXT'], 0, 32) + : null; + + $this->metadata = $metadata ?? new Metadata(); + $this->region = \basename($this->metadata->get('instance/region')); + $this->instanceId = $this->metadata->get('instance/id'); + } + + /** + * @return array + */ + public function monitoredResource() + { + return [ + 'type' => 'cloud_run_revision', + 'labels' => [ + 'configuration_name' => $this->configurationId, + 'location' => $this->region, + 'project_id' => $this->projectId(), + 'revision_name' => $this->revisionId, + 'service_name' => $this->serviceId, + ], + ]; + } + + /** + * Return the project id. + * @return string + */ + public function projectId() + { + return $this->metadata->getProjectId(); + } + + /** + * Return the service id. + * @return string + */ + public function serviceId() + { + return $this->serviceId; + } + + /** + * Return the version id. + * @return string + */ + public function versionId() + { + return $this->revisionId; + } + + /** + * Return the labels. We need to evaluate $_SERVER for each request. + * + * @return array + * @todo Figure out where to get the `container_name` from + */ + public function labels() + { + $labels = [ + 'instanceId' => $this->instanceId, + 'run.googleapis.com/trace_id' => $this->traceId, + ]; + return \array_filter($labels); + } +} diff --git a/Core/src/Report/MetadataProviderUtils.php b/Core/src/Report/MetadataProviderUtils.php index 20ce785bd469..cdd3e51309e5 100644 --- a/Core/src/Report/MetadataProviderUtils.php +++ b/Core/src/Report/MetadataProviderUtils.php @@ -17,6 +17,8 @@ namespace Google\Cloud\Core\Report; +use Google\Cloud\Core\Compute\Metadata; + /** * Utility class for MetadataProvider. */ @@ -28,7 +30,7 @@ class MetadataProviderUtils * @param array $server Normally pass the $_SERVER. * @return MetadataProviderInterface */ - public static function autoSelect($server) + public static function autoSelect($server, ?Metadata $metadata = null) { if (isset($server['GAE_SERVICE'])) { if (isset($server['GAE_ENV']) && $server['GAE_ENV'] === 'standard') { @@ -36,8 +38,11 @@ public static function autoSelect($server) } return new GAEFlexMetadataProvider($server); } - if (!empty(getenv('K_CONFIGURATION'))) { - return new CloudRunMetadataProvider(getenv()); + if (!empty(\getenv('K_CONFIGURATION'))) { + return new CloudRunServiceMetadataProvider(\getenv(), $metadata); + } + if (!empty(\getenv('CLOUD_RUN_JOB'))) { + return new CloudRunJobMetadataProvider(\getenv(), $metadata); } return new EmptyMetadataProvider(); } diff --git a/Core/tests/Unit/Report/CloudRunJobMetadataProviderTest.php b/Core/tests/Unit/Report/CloudRunJobMetadataProviderTest.php new file mode 100644 index 000000000000..9593b3b72aa9 --- /dev/null +++ b/Core/tests/Unit/Report/CloudRunJobMetadataProviderTest.php @@ -0,0 +1,94 @@ +method('read') + ->willReturnMap([ + ['instance/region', 'projects/my-project/regions/my-region'], + ['instance/id', 'my-instance-id'], + ['project/project-id', 'my-project'], + ]); + + $provider = new CloudRunJobMetadataProvider([ + 'CLOUD_RUN_JOB' => 'my-job', + 'CLOUD_RUN_EXECUTION' => 'my-execution', + 'CLOUD_RUN_TASK_INDEX' => '1', + 'CLOUD_RUN_TASK_ATTEMPT' => '3', + 'HTTP_X_CLOUD_TRACE_CONTEXT' => 'my-traceId', + ], new Metadata($reader)); + $this->assertEquals('my-job', $provider->serviceId()); + $this->assertEquals('my-execution', $provider->versionId()); + $this->assertEquals( + [ + 'type' => 'cloud_run_job', + 'labels' => [ + 'job_name' => 'my-job', + 'location' => 'my-region', + 'project_id' => 'my-project', + ], + ], + $provider->monitoredResource() + ); + $this->assertEquals( + [ + 'instanceId' => 'my-instance-id', + 'run.googleapis.com/execution_name' => 'my-execution', + 'run.googleapis.com/task_attempt' => '3', + 'run.googleapis.com/task_index' => '1', + 'run.googleapis.com/trace_id' => 'my-traceId', + ], + $provider->labels() + ); + } + + public function testDefaults() + { + $reader = self::createStub(ReaderInterface::class); + $reader->method('read') + ->willReturn(''); + + $provider = new CloudRunJobMetadataProvider([], new Metadata($reader)); + $this->assertEquals('unknown-job', $provider->serviceId()); + $this->assertEquals('', $provider->versionId()); + $this->assertEquals( + [ + 'type' => 'cloud_run_job', + 'labels' => [ + 'job_name' => 'unknown-job', + 'location' => '', + 'project_id' => '', + ], + ], + $provider->monitoredResource() + ); + $this->assertEquals([], $provider->labels()); + } +} diff --git a/Core/tests/Unit/Report/CloudRunMetadataProviderTest.php b/Core/tests/Unit/Report/CloudRunMetadataProviderTest.php deleted file mode 100644 index f355e62f39d2..000000000000 --- a/Core/tests/Unit/Report/CloudRunMetadataProviderTest.php +++ /dev/null @@ -1,52 +0,0 @@ - 'my-service', - 'K_REVISION' => 'my-revision', - 'HTTP_X_CLOUD_TRACE_CONTEXT' => 'my-traceId' - ]); - $this->assertEquals('my-service', $provider->serviceId()); - $this->assertEquals('my-revision', $provider->versionId()); - $this->assertEquals( - [ - 'run.googleapis.com/trace_id' => 'my-traceId' - ], - $provider->labels() - ); - } - - public function testDefaults() - { - $provider = new CloudRunMetadataProvider([]); - $this->assertEquals('unknown-service', $provider->serviceId()); - $this->assertEquals('unknown-revision', $provider->versionId()); - $this->assertEquals([], $provider->labels()); - } -} diff --git a/Core/tests/Unit/Report/CloudRunServiceMetadataProviderTest.php b/Core/tests/Unit/Report/CloudRunServiceMetadataProviderTest.php new file mode 100644 index 000000000000..1b499e2b5c2a --- /dev/null +++ b/Core/tests/Unit/Report/CloudRunServiceMetadataProviderTest.php @@ -0,0 +1,94 @@ +method('read') + ->willReturnMap([ + ['instance/region', 'projects/my-project/regions/my-region'], + ['instance/id', 'my-instance-id'], + ['project/project-id', 'my-project'], + ]); + + $provider = new CloudRunServiceMetadataProvider([ + 'K_CONFIGURATION' => 'my-configuration', + 'K_SERVICE' => 'my-service', + 'K_REVISION' => 'my-revision', + 'HTTP_X_CLOUD_TRACE_CONTEXT' => 'my-traceId', + ], new Metadata($reader)); + $this->assertEquals('my-service', $provider->serviceId()); + $this->assertEquals('my-revision', $provider->versionId()); + $this->assertEquals( + [ + 'type' => 'cloud_run_revision', + 'labels' => [ + 'configuration_name' => 'my-configuration', + 'location' => 'my-region', + 'project_id' => 'my-project', + 'revision_name' => 'my-revision', + 'service_name' => 'my-service', + ], + ], + $provider->monitoredResource() + ); + $this->assertEquals( + [ + 'instanceId' => 'my-instance-id', + 'run.googleapis.com/trace_id' => 'my-traceId', + ], + $provider->labels() + ); + } + + public function testDefaults() + { + $reader = self::createStub(ReaderInterface::class); + $reader->method('read') + ->willReturn(''); + + $provider = new CloudRunServiceMetadataProvider([], new Metadata($reader)); + $this->assertEquals('unknown-service', $provider->serviceId()); + $this->assertEquals('unknown-revision', $provider->versionId()); + $this->assertEquals( + [ + 'type' => 'cloud_run_revision', + 'labels' => [ + 'configuration_name' => 'unknown-configuration', + 'location' => '', + 'project_id' => '', + 'revision_name' => 'unknown-revision', + 'service_name' => 'unknown-service', + ], + ], + $provider->monitoredResource() + ); + $this->assertEquals([], $provider->labels()); + } +} diff --git a/Core/tests/Unit/Report/MetadataProviderUtilsTest.php b/Core/tests/Unit/Report/MetadataProviderUtilsTest.php index 367383e48147..13901c46a53d 100644 --- a/Core/tests/Unit/Report/MetadataProviderUtilsTest.php +++ b/Core/tests/Unit/Report/MetadataProviderUtilsTest.php @@ -17,6 +17,10 @@ namespace Google\Cloud\Core\Tests\Unit\Report; +use Google\Cloud\Core\Compute\Metadata; +use Google\Cloud\Core\Compute\Metadata\Readers\ReaderInterface; +use Google\Cloud\Core\Report\CloudRunJobMetadataProvider; +use Google\Cloud\Core\Report\CloudRunServiceMetadataProvider; use Google\Cloud\Core\Report\EmptyMetadataProvider; use Google\Cloud\Core\Report\GAEFlexMetadataProvider; use Google\Cloud\Core\Report\GAEStandardMetadataProvider; @@ -30,6 +34,20 @@ class MetadataProviderUtilsTest extends TestCase { private $envs = ['GAE_SERVICE' => 'my-service']; private $std_envs = ['GAE_SERVICE' => 'my-service', 'GAE_ENV' => 'standard']; + private $configuration; + private $jobName; + + protected function setUp(): void + { + $this->configuration = \getenv('K_CONFIGURATION'); + $this->jobName = \getenv('CLOUD_RUN_JOB'); + } + + protected function tearDown(): void + { + \putenv('K_CONFIGURATION=' . $this->configuration); + \putenv('CLOUD_RUN_JOB=' . $this->jobName); + } public function testAutoSelect() { @@ -49,4 +67,30 @@ public function testAutoSelect() $metadataProvider ); } + + public function testAutoSelectReturnsCloudRunJobMetadataProvider(): void + { + \putenv('CLOUD_RUN_JOB=my-job'); + $metadataProvider = MetadataProviderUtils::autoSelect( + [], + new Metadata(self::createStub(ReaderInterface::class)) + ); + $this->assertInstanceOf( + CloudRunJobMetadataProvider::class, + $metadataProvider + ); + } + + public function testAutoSelectReturnsCloudRunServiceMetadataProvider(): void + { + \putenv('K_CONFIGURATION=my-configuration'); + $metadataProvider = MetadataProviderUtils::autoSelect( + [], + new Metadata(self::createStub(ReaderInterface::class)) + ); + $this->assertInstanceOf( + CloudRunServiceMetadataProvider::class, + $metadataProvider + ); + } } diff --git a/Logging/tests/Unit/PsrLoggerBatchTest.php b/Logging/tests/Unit/PsrLoggerBatchTest.php index e1e9e03502bc..40e69c9753ff 100644 --- a/Logging/tests/Unit/PsrLoggerBatchTest.php +++ b/Logging/tests/Unit/PsrLoggerBatchTest.php @@ -18,7 +18,10 @@ namespace Google\Cloud\Logging\Tests\Unit; use Google\Cloud\Core\Batch\BatchRunner; -use Google\Cloud\Core\Report\CloudRunMetadataProvider; +use Google\Cloud\Core\Compute\Metadata; +use Google\Cloud\Core\Compute\Metadata\Readers\ReaderInterface; +use Google\Cloud\Core\Report\CloudRunJobMetadataProvider; +use Google\Cloud\Core\Report\CloudRunServiceMetadataProvider; use Google\Cloud\Core\Report\GAEFlexMetadataProvider; use Google\Cloud\Logging\Connection\Rest; use Google\Cloud\Logging\Entry; @@ -30,6 +33,8 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; +use function str_repeat; + /** * @group logging */ @@ -121,13 +126,18 @@ public function testTraceIdLabelOnServerlessPlatforms( 'my-project' ); + $reader = self::createStub(ReaderInterface::class); + $reader->method('read') + ->willReturn(''); + $metadata = new Metadata($reader); + $psrBatchLogger = new PsrLogger( $logger, null, [ 'batchEnabled' => true, 'batchRunner' => $this->runner->reveal(), - 'metadataProvider' => new $metadataProviderClass($server) + 'metadataProvider' => new $metadataProviderClass($server, $metadata) ] ); @@ -201,7 +211,13 @@ public function traceIdProvider() ], [ str_repeat('x', 32), - CloudRunMetadataProvider::class, + CloudRunServiceMetadataProvider::class, + [], + ['run.googleapis.com/trace_id' => str_repeat('x', 32)] + ], + [ + str_repeat('x', 32), + CloudRunJobMetadataProvider::class, [], ['run.googleapis.com/trace_id' => str_repeat('x', 32)] ], From 08f6599f9b7cd836d0cc7248b91f7c3b18c2e128 Mon Sep 17 00:00:00 2001 From: Matt Kynaston Date: Wed, 4 Sep 2024 18:55:42 +0100 Subject: [PATCH 2/4] Keep CloudRunMetadataProvider as alias for CloudRunServiceMetadataProvider --- Core/src/Report/CloudRunMetadataProvider.php | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Core/src/Report/CloudRunMetadataProvider.php diff --git a/Core/src/Report/CloudRunMetadataProvider.php b/Core/src/Report/CloudRunMetadataProvider.php new file mode 100644 index 000000000000..00b6b9fbd978 --- /dev/null +++ b/Core/src/Report/CloudRunMetadataProvider.php @@ -0,0 +1,25 @@ + Date: Thu, 5 Sep 2024 09:50:13 +0100 Subject: [PATCH 3/4] Bump google/core dep to next release --- Logging/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Logging/composer.json b/Logging/composer.json index 8757234410e6..53c116ca27c2 100644 --- a/Logging/composer.json +++ b/Logging/composer.json @@ -5,7 +5,7 @@ "minimum-stability": "stable", "require": { "php": "^8.0", - "google/cloud-core": "^1.58", + "google/cloud-core": "^1.60", "google/gax": "^1.34.0" }, "require-dev": { From 89e60a4e4548cdaaabeafe55afb95184785f3510 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Fri, 27 Dec 2024 12:53:07 -0800 Subject: [PATCH 4/4] Apply suggestions from code review --- Core/src/Report/CloudRunJobMetadataProvider.php | 2 +- Core/src/Report/CloudRunServiceMetadataProvider.php | 2 +- Core/tests/Unit/Report/CloudRunJobMetadataProviderTest.php | 2 +- Core/tests/Unit/Report/CloudRunServiceMetadataProviderTest.php | 2 +- Logging/composer.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Core/src/Report/CloudRunJobMetadataProvider.php b/Core/src/Report/CloudRunJobMetadataProvider.php index ca5bd591cbde..fe8d94ef4cbd 100644 --- a/Core/src/Report/CloudRunJobMetadataProvider.php +++ b/Core/src/Report/CloudRunJobMetadataProvider.php @@ -1,6 +1,6 @@