From e74bf8909c1cd7e9a4b3b76f011c4a3e43a6c8d8 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:49:00 +0000 Subject: [PATCH 01/19] feat: Initate Metrics Trait * Add getVersion functionality. * Add unit test for it. --- src/MetricsTrait.php | 54 ++++++++++++++++++++++++++++++++++++++ tests/MetricsTraitTest.php | 43 ++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 src/MetricsTrait.php create mode 100644 tests/MetricsTraitTest.php diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php new file mode 100644 index 000000000..276dcc356 --- /dev/null +++ b/src/MetricsTrait.php @@ -0,0 +1,54 @@ + 'auth-request-type/at', + 'idToken' => 'auth-request-type/idt', + 'mdsPing' => 'auth-request-type/mds' + ]; + + private static array $credType = [ + 'user' => 'cred-type/u', + 'sa' => 'cred-type/sa', + 'sa-jwt' => 'cred-type/jwt', + 'gce' => 'cred-type/mds', + 'impersonate' => 'cred-type/imp' + ]; + + private function getVersion(): string + { + if (is_null(self::$version)) { + $versionFilePath = implode(DIRECTORY_SEPARATOR, [__DIR__, '..', 'VERSION']); + self::$version = trim((string) file_get_contents($versionFilePath)); + } + return self::$version; + } + + +} diff --git a/tests/MetricsTraitTest.php b/tests/MetricsTraitTest.php new file mode 100644 index 000000000..cf12a4b92 --- /dev/null +++ b/tests/MetricsTraitTest.php @@ -0,0 +1,43 @@ +impl = new class {use MetricsTrait { + getVersion as public; + }}; + } + + public function testGetVersion() + { + $actualVersion = $this->impl->getVersion(); + $this->assertStringMatchesFormat("%d.%d.%d", $actualVersion); + } +} From b01646a3ae2aeeecc0acb95b814dc6bd8ed75064 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:42:42 +0000 Subject: [PATCH 02/19] feat: Enable Service Api Metrics * With empty metrics headers --- src/CredentialsLoader.php | 1 + src/MetricsTrait.php | 46 +++++++++++++++++++++++++++++++++++++ src/UpdateMetadataTrait.php | 5 ++++ 3 files changed, 52 insertions(+) diff --git a/src/CredentialsLoader.php b/src/CredentialsLoader.php index 746b957a9..cef46a4e0 100644 --- a/src/CredentialsLoader.php +++ b/src/CredentialsLoader.php @@ -34,6 +34,7 @@ abstract class CredentialsLoader implements FetchAuthTokenInterface, UpdateMetadataInterface { + use MetricsTrait; use UpdateMetadataTrait; const TOKEN_CREDENTIAL_URI = 'https://oauth2.googleapis.com/token'; diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 276dcc356..8c75786c8 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -27,6 +27,8 @@ trait MetricsTrait { private static $version = null; + private static $headerKey = 'x-goog-api-client'; + private static array $requestType = [ 'accessToken' => 'auth-request-type/at', 'idToken' => 'auth-request-type/idt', @@ -50,5 +52,49 @@ private function getVersion(): string return self::$version; } + private function getServiceApiMetricsHeaderValue() + { + switch (get_class($this)) { + case 'Google\Auth\Credentials\UserRefreshCredentials': + return ''; + case 'Google\Auth\Credentials\ServiceAccountCredentials': + return ''; + case 'Google\Auth\Credentials\ServiceAccountJwtAccessCredentials': + return ''; + case 'Google\Auth\Credentials\GCECredentials': + return ''; + case 'Google\Auth\Credentials\ImpersonatedServiceAccountCredentials': + return ''; + } + return ''; + } + private function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest) + { + switch (get_class($this)) { + case 'Google\Auth\Credentials\UserRefreshCredentials': + return ''; + case 'Google\Auth\Credentials\ServiceAccountCredentials': + return ''; + case 'Google\Auth\Credentials\ServiceAccountJwtAccessCredentials': + return ''; + case 'Google\Auth\Credentials\GCECredentials': + return ''; + case 'Google\Auth\Credentials\ImpersonatedServiceAccountCredentials': + return ''; + } + return ''; + } + + private function applyMetricsHeader($metadata, $headerValue) + { + if (empty($headerValue)) { + return $metadata; + } elseif (isset($metadata[self::$headerKey])) { + $metadata[self::$headerKey][0] .= ' ' . $headerValue; + } else { + $metadata[self::$headerKey] = [$headerValue]; + } + return $metadata; + } } diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index fd33e0dca..ad3b7f2a9 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -61,6 +61,11 @@ public function updateMetadata( } elseif (isset($result['id_token'])) { $metadata_copy[self::AUTH_METADATA_KEY] = ['Bearer ' . $result['id_token']]; } + + $metadata_copy = $this->applyMetricsHeaders( + $metadata_copy, + $this->getServiceApiMetricsHeaderValue() + ); return $metadata_copy; } } From 73e9ee30aaf98a988cd3c915e1a91293b5fb0a47 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:45:33 +0000 Subject: [PATCH 03/19] feat: Enable Token Endpoint Metrics --- src/Credentials/GCECredentials.php | 18 +++++++++++++++--- .../ImpersonatedServiceAccountCredentials.php | 8 +++++++- src/Credentials/ServiceAccountCredentials.php | 11 ++++++++++- src/Credentials/UserRefreshCredentials.php | 16 ++++++++++++++-- src/OAuth2.php | 10 ++++++---- 5 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index 991589b52..c3b878ea8 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -395,7 +395,16 @@ public function fetchAuthToken(callable $httpHandler = null) return []; // return an empty array with no access token } - $response = $this->getFromMetadata($httpHandler, $this->tokenUri); + $isAccessTokenRequest = true; + if (isset($this->targetAudience)) { + $isAccessTokenRequest = false; + } + + $metricsHeader = $this->applyMetricsHeader( + [], + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + ); + $response = $this->getFromMetadata($httpHandler, $this->tokenUri, $metricsHeader); if ($this->targetAudience) { return ['id_token' => $response]; @@ -505,15 +514,18 @@ public function getProjectId(callable $httpHandler = null) * * @param callable $httpHandler An HTTP Handler to deliver PSR7 requests. * @param string $uri The metadata URI. + * @param array $metricsHeaders [optional] If present, add these headers to the token + * endpoint request. + * * @return string */ - private function getFromMetadata(callable $httpHandler, $uri) + private function getFromMetadata(callable $httpHandler, $uri, array $metricsHeaders = []) { $resp = $httpHandler( new Request( 'GET', $uri, - [self::FLAVOR_HEADER => 'Google'] + [self::FLAVOR_HEADER => 'Google'] + $metricsHeaders ) ); diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 1b4e46eaf..55ad4387f 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -121,7 +121,13 @@ public function getClientName(callable $unusedHttpHandler = null) */ public function fetchAuthToken(callable $httpHandler = null) { - return $this->sourceCredentials->fetchAuthToken($httpHandler); + // We don't support id token endpoint requests as of now for Impersonated Cred + $isAccessTokenRequest = true; + $metricsHeaders = $this->applyMetricsHeader( + [], + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest = true) + ); + return $this->sourceCredentials->fetchAuthToken($httpHandler, $metricsHeaders); } /** diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 8b6b79a6e..4d3b6290f 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -206,7 +206,16 @@ public function fetchAuthToken(callable $httpHandler = null) return $accessToken; } - return $this->auth->fetchAuthToken($httpHandler); + $isAccessTokenRequest = true; + if (isset($this->auth->getAdditionalClaims()['target_audience'])) { + $isAccessTokenRequest = false; + } + + $metricsHeader = $this->applyMetricsHeader( + [], + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + ); + return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); } /** diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index e2f32d87f..588fb24e6 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -98,6 +98,10 @@ public function __construct( /** * @param callable $httpHandler + * @param array $metricsHeaders [optional] Metrics headers to be inserted + * into the token endpoint request present. + * This is passed from ImersonatedServiceAccountCredentials as it uses + * UserRefreshCredentials as source credentials. * * @return array { * A set of auth related metadata, containing the following @@ -109,9 +113,17 @@ public function __construct( * @type string $id_token * } */ - public function fetchAuthToken(callable $httpHandler = null) + public function fetchAuthToken(callable $httpHandler = null, array $metricsHeader = []) { - return $this->auth->fetchAuthToken($httpHandler); + if (empty($metricsHeaders)) { + // We don't support id token endpoint requests as of now for User Cred + $isAccessTokenRequest = true; + $metricsHeaders = $this->applyMetricsHeader( + [], + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + ); + } + return $this->auth->fetchAuthToken($httpHandler, $metricsHeaders); } /** diff --git a/src/OAuth2.php b/src/OAuth2.php index 2e5adcdcf..2a683ec5b 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -574,7 +574,7 @@ public function toJwt(array $config = []) * @param callable $httpHandler callback which delivers psr7 request * @return RequestInterface the authorization Url. */ - public function generateCredentialsRequest(callable $httpHandler = null) + public function generateCredentialsRequest(callable $httpHandler = null, $metricsHeaders = []) { $uri = $this->getTokenCredentialUri(); if (is_null($uri)) { @@ -633,7 +633,7 @@ public function generateCredentialsRequest(callable $httpHandler = null) $headers = [ 'Cache-Control' => 'no-store', 'Content-Type' => 'application/x-www-form-urlencoded', - ]; + ] + $metricsHeaders; return new Request( 'POST', @@ -647,15 +647,17 @@ public function generateCredentialsRequest(callable $httpHandler = null) * Fetches the auth tokens based on the current state. * * @param callable $httpHandler callback which delivers psr7 request + * @param array $metricsHeaders [optional] If present, add these headers to the token + * endpoint request. * @return array the response */ - public function fetchAuthToken(callable $httpHandler = null) + public function fetchAuthToken(callable $httpHandler = null, $metricsHeaders = []) { if (is_null($httpHandler)) { $httpHandler = HttpHandlerFactory::build(HttpClientCache::getHttpClient()); } - $response = $httpHandler($this->generateCredentialsRequest($httpHandler)); + $response = $httpHandler($this->generateCredentialsRequest($httpHandler, $metricsHeaders)); $credentials = $this->parseTokenResponse($response); $this->updateToken($credentials); if (isset($credentials['scope'])) { From e1f1e082ad303f1d709f04cf008dfdeda9714512 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Fri, 15 Dec 2023 09:53:14 +0000 Subject: [PATCH 04/19] fix: Remove the anit pattern --- src/Credentials/GCECredentials.php | 24 +++++++++++-- .../ImpersonatedServiceAccountCredentials.php | 22 +++++++++++- src/Credentials/ServiceAccountCredentials.php | 10 ++++-- .../ServiceAccountJwtAccessCredentials.php | 9 +++-- src/Credentials/UserRefreshCredentials.php | 29 ++++++++++++++- src/MetricsTrait.php | 36 +++++++++++-------- src/UpdateMetadataTrait.php | 7 ++-- 7 files changed, 110 insertions(+), 27 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index c3b878ea8..bf50a07b1 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -333,7 +333,7 @@ public static function onGce(callable $httpHandler = null) new Request( 'GET', $checkUri, - [self::FLAVOR_HEADER => 'Google'] + [self::FLAVOR_HEADER => 'Google'] + self::getMdsPingHeader() ), ['timeout' => self::COMPUTE_PING_CONNECTION_TIMEOUT_S] ); @@ -402,7 +402,7 @@ public function fetchAuthToken(callable $httpHandler = null) $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue('gce', $isAccessTokenRequest) ); $response = $this->getFromMetadata($httpHandler, $this->tokenUri, $metricsHeader); @@ -422,6 +422,26 @@ public function fetchAuthToken(callable $httpHandler = null) return $json; } + /** + * Updates metadata with the authorization token. + * + * @param array $metadata metadata hashmap + * @param string $authUri optional auth uri + * @param callable $httpHandler callback which delivers psr7 request + * @param string $_metricsHeaderValue [INTERNAL] The observability metrics + * header value to be set on the request. + * @return array updated metadata hashmap + */ + public function updateMetadata( + $metadata, + $authUri = null, + callable $httpHandler = null, + string $_metricsHeaderValue = '' + ): array { + $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('gce'); + return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + } + /** * @return string */ diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 55ad4387f..8d5dd5641 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -125,11 +125,31 @@ public function fetchAuthToken(callable $httpHandler = null) $isAccessTokenRequest = true; $metricsHeaders = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest = true) + $this->getTokenEndpointMetricsHeaderValue('impersonated', $isAccessTokenRequest = true) ); return $this->sourceCredentials->fetchAuthToken($httpHandler, $metricsHeaders); } + /** + * Updates metadata with the authorization token. + * + * @param array $metadata metadata hashmap + * @param string $authUri optional auth uri + * @param callable $httpHandler callback which delivers psr7 request + * @param string $_metricsHeaderValue [INTERNAL] The observability metrics + * header value to be set on the request. + * @return array updated metadata hashmap + */ + public function updateMetadata( + $metadata, + $authUri = null, + callable $httpHandler = null, + string $_metricsHeaderValue = '' + ): array { + $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('impersonated'); + return $this->sourceCredentials->updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + } + /** * @return string */ diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 4d3b6290f..4ce90ad92 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -213,7 +213,7 @@ public function fetchAuthToken(callable $httpHandler = null) $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue('sa', $isAccessTokenRequest) ); return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); } @@ -262,16 +262,20 @@ public function getProjectId(callable $httpHandler = null) * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request + * @param string $_metricsHeaderValue [INTERNAL] The observability metrics + * header value to be set on the request. * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null + callable $httpHandler = null, + string $_metricsHeaderValue = '' ) { // scope exists. use oauth implementation if (!$this->useSelfSignedJwt()) { - return parent::updateMetadata($metadata, $authUri, $httpHandler); + $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('sa'); + return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); } $jwtCreds = $this->createJwtAccessCredentials(); diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 16c7d59ca..81aeff7ba 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -108,12 +108,15 @@ public function __construct($jsonKey, $scope = null) * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request + * @param string $_metricsHeaderValue [INTERNAL] The observability metrics + * header value to be set on the request. It's set internally and is not propagated * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null + callable $httpHandler = null, + $_metricsHeaderValue = '' ) { $scope = $this->auth->getScope(); if (empty($authUri) && empty($scope)) { @@ -121,8 +124,8 @@ public function updateMetadata( } $this->auth->setAudience($authUri); - - return parent::updateMetadata($metadata, $authUri, $httpHandler); + $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('jwt'); + return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); } /** diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 588fb24e6..52e78b536 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -115,17 +115,44 @@ public function __construct( */ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeader = []) { + // ImersonatedServiceAccountCredentials can propagate it's own header value, hence + // the check for empty metricsHeaders if (empty($metricsHeaders)) { // We don't support id token endpoint requests as of now for User Cred $isAccessTokenRequest = true; $metricsHeaders = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue('user', $isAccessTokenRequest) ); } return $this->auth->fetchAuthToken($httpHandler, $metricsHeaders); } + /** + * Updates metadata with the authorization token. + * + * @param array $metadata metadata hashmap + * @param string $authUri optional auth uri + * @param callable $httpHandler callback which delivers psr7 request + * @param string $_metricsHeaderValue [INTERNAL] The observability metrics + * header value to be set on the request. + * @return array updated metadata hashmap + */ + public function updateMetadata( + $metadata, + $authUri = null, + callable $httpHandler = null, + string $_metricsHeaderValue = '' + ): array { + // ImersonatedServiceAccountCredentials can propagate it's own header value, hence the check for empty $_metricsHeaderValue + if (empty($_metricsHeaderValue)) { + $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('user'); + } else { + $metricsHeaderValue = $_metricsHeaderValue; + } + return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + } + /** * @return string */ diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 8c75786c8..dc5040f53 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -52,41 +52,47 @@ private function getVersion(): string return self::$version; } - private function getServiceApiMetricsHeaderValue() + protected function getServiceApiMetricsHeaderValue(string $credType) { - switch (get_class($this)) { - case 'Google\Auth\Credentials\UserRefreshCredentials': + switch ($credType) { + case 'user': return ''; - case 'Google\Auth\Credentials\ServiceAccountCredentials': + case 'sa': return ''; - case 'Google\Auth\Credentials\ServiceAccountJwtAccessCredentials': + case 'jwt': return ''; - case 'Google\Auth\Credentials\GCECredentials': + case 'gce': return ''; - case 'Google\Auth\Credentials\ImpersonatedServiceAccountCredentials': + case 'impersonated': return ''; } return ''; } - private function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest) + protected function getTokenEndpointMetricsHeaderValue(string $credType, bool $isAccessTokenRequest) { - switch (get_class($this)) { - case 'Google\Auth\Credentials\UserRefreshCredentials': + switch ($credType) { + case 'user': return ''; - case 'Google\Auth\Credentials\ServiceAccountCredentials': + case 'sa': return ''; - case 'Google\Auth\Credentials\ServiceAccountJwtAccessCredentials': + case 'jwt': return ''; - case 'Google\Auth\Credentials\GCECredentials': + case 'gce': return ''; - case 'Google\Auth\Credentials\ImpersonatedServiceAccountCredentials': + case 'impersonated': return ''; } return ''; } - private function applyMetricsHeader($metadata, $headerValue) + protected static function getMdsPingHeader() + { + return []; + } + + + protected function applyMetricsHeader($metadata, $headerValue) { if (empty($headerValue)) { return $metadata; diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index ad3b7f2a9..8082ffc7c 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -43,12 +43,15 @@ public function getUpdateMetadataFunc() * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request + * @param string $metricsHeaderValue [optional] The observability metrics + * header value to be set on the request. * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null + callable $httpHandler = null, + string $metricsHeaderValue = '' ) { if (isset($metadata[self::AUTH_METADATA_KEY])) { // Auth metadata has already been set @@ -64,7 +67,7 @@ public function updateMetadata( $metadata_copy = $this->applyMetricsHeaders( $metadata_copy, - $this->getServiceApiMetricsHeaderValue() + $metricsHeaderValue ); return $metadata_copy; } From fa369060d5e213fa1ac9f3ce9371caa95c41de0e Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Fri, 15 Dec 2023 10:13:37 +0000 Subject: [PATCH 05/19] fix: Correct applyMetricHeader method name --- src/Credentials/GCECredentials.php | 2 +- src/Credentials/ImpersonatedServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountCredentials.php | 2 +- src/Credentials/UserRefreshCredentials.php | 2 +- src/MetricsTrait.php | 2 +- src/UpdateMetadataTrait.php | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index bf50a07b1..05ea8e9c6 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -400,7 +400,7 @@ public function fetchAuthToken(callable $httpHandler = null) $isAccessTokenRequest = false; } - $metricsHeader = $this->applyMetricsHeader( + $metricsHeader = $this->applyMetricHeaders( [], $this->getTokenEndpointMetricsHeaderValue('gce', $isAccessTokenRequest) ); diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 8d5dd5641..05e9e3dd6 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -123,7 +123,7 @@ public function fetchAuthToken(callable $httpHandler = null) { // We don't support id token endpoint requests as of now for Impersonated Cred $isAccessTokenRequest = true; - $metricsHeaders = $this->applyMetricsHeader( + $metricsHeaders = $this->applyMetricHeaders( [], $this->getTokenEndpointMetricsHeaderValue('impersonated', $isAccessTokenRequest = true) ); diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 4ce90ad92..ec3ca33d5 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -211,7 +211,7 @@ public function fetchAuthToken(callable $httpHandler = null) $isAccessTokenRequest = false; } - $metricsHeader = $this->applyMetricsHeader( + $metricsHeader = $this->applyMetricHeaders( [], $this->getTokenEndpointMetricsHeaderValue('sa', $isAccessTokenRequest) ); diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 52e78b536..7ebb739b7 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -120,7 +120,7 @@ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeade if (empty($metricsHeaders)) { // We don't support id token endpoint requests as of now for User Cred $isAccessTokenRequest = true; - $metricsHeaders = $this->applyMetricsHeader( + $metricsHeaders = $this->applyMetricHeaders( [], $this->getTokenEndpointMetricsHeaderValue('user', $isAccessTokenRequest) ); diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index dc5040f53..111463e27 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -92,7 +92,7 @@ protected static function getMdsPingHeader() } - protected function applyMetricsHeader($metadata, $headerValue) + protected function applyMetricHeaders($metadata, $headerValue) { if (empty($headerValue)) { return $metadata; diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 8082ffc7c..9db6453fa 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -65,7 +65,7 @@ public function updateMetadata( $metadata_copy[self::AUTH_METADATA_KEY] = ['Bearer ' . $result['id_token']]; } - $metadata_copy = $this->applyMetricsHeaders( + $metadata_copy = $this->applyMetricHeaders( $metadata_copy, $metricsHeaderValue ); From 7c2b6053b600b73505b81a61cefcbc43cabcf040 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 04:53:26 +0000 Subject: [PATCH 06/19] Finalise PR --- src/Credentials/GCECredentials.php | 48 +++-- .../ImpersonatedServiceAccountCredentials.php | 33 ++-- src/Credentials/ServiceAccountCredentials.php | 17 +- .../ServiceAccountJwtAccessCredentials.php | 13 +- src/Credentials/UserRefreshCredentials.php | 28 ++- src/MetricsTrait.php | 87 +++++---- src/OAuth2.php | 10 +- src/UpdateMetadataTrait.php | 7 +- tests/Credentials/GCECredentialsTest.php | 18 ++ tests/MetricsTraitTest.php | 166 +++++++++++++++++- 10 files changed, 294 insertions(+), 133 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index d394b2840..317c14180 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -144,6 +144,13 @@ class GCECredentials extends CredentialsLoader implements */ protected $lastReceivedToken; + /** + * User in observability metric headers + * + * @var string + */ + protected string $credType = 'cred-type/mds'; + /** * @var string|null */ @@ -308,6 +315,17 @@ private static function getProjectIdUri() return $base . self::PROJECT_ID_URI_PATH; } + private static function getMdsPingHeader() + { + return [ + self::$metricsHeaderKey => [sprintf( + 'gl-php/%s auth/%s auth-request-type/mds', + PHP_VERSION, + self::getVersion() + )] + ]; + } + /** * The full uri for accessing the default universe domain. * @@ -426,9 +444,9 @@ public function fetchAuthToken(callable $httpHandler = null) $isAccessTokenRequest = false; } - $metricsHeader = $this->applyMetricHeaders( + $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue('gce', $isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) ); $response = $this->getFromMetadata($httpHandler, $this->tokenUri, $metricsHeader); @@ -448,26 +466,6 @@ public function fetchAuthToken(callable $httpHandler = null) return $json; } - /** - * Updates metadata with the authorization token. - * - * @param array $metadata metadata hashmap - * @param string $authUri optional auth uri - * @param callable $httpHandler callback which delivers psr7 request - * @param string $_metricsHeaderValue [INTERNAL] The observability metrics - * header value to be set on the request. - * @return array updated metadata hashmap - */ - public function updateMetadata( - $metadata, - $authUri = null, - callable $httpHandler = null, - string $_metricsHeaderValue = '' - ): array { - $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('gce'); - return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); - } - /** * @return string */ @@ -610,18 +608,18 @@ public function getUniverseDomain(callable $httpHandler = null): string * * @param callable $httpHandler An HTTP Handler to deliver PSR7 requests. * @param string $uri The metadata URI. - * @param array $metricsHeaders [optional] If present, add these headers to the token + * @param array $metricsHeader [optional] If present, add these headers to the token * endpoint request. * * @return string */ - private function getFromMetadata(callable $httpHandler, $uri, array $metricsHeaders = []) + private function getFromMetadata(callable $httpHandler, $uri, array $metricsHeader = []) { $resp = $httpHandler( new Request( 'GET', $uri, - [self::FLAVOR_HEADER => 'Google'] + $metricsHeaders + [self::FLAVOR_HEADER => 'Google'] + $metricsHeader ) ); diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 05e9e3dd6..cce1664f0 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -36,6 +36,13 @@ class ImpersonatedServiceAccountCredentials extends CredentialsLoader implements */ protected $sourceCredentials; + /** + * User in observability metric headers + * + * @var string + */ + protected string $credType = 'cred-type/imp'; + /** * Instantiate an instance of ImpersonatedServiceAccountCredentials from a credentials file that * has be created with the --impersonated-service-account flag. @@ -123,31 +130,11 @@ public function fetchAuthToken(callable $httpHandler = null) { // We don't support id token endpoint requests as of now for Impersonated Cred $isAccessTokenRequest = true; - $metricsHeaders = $this->applyMetricHeaders( + $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue('impersonated', $isAccessTokenRequest = true) + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) ); - return $this->sourceCredentials->fetchAuthToken($httpHandler, $metricsHeaders); - } - - /** - * Updates metadata with the authorization token. - * - * @param array $metadata metadata hashmap - * @param string $authUri optional auth uri - * @param callable $httpHandler callback which delivers psr7 request - * @param string $_metricsHeaderValue [INTERNAL] The observability metrics - * header value to be set on the request. - * @return array updated metadata hashmap - */ - public function updateMetadata( - $metadata, - $authUri = null, - callable $httpHandler = null, - string $_metricsHeaderValue = '' - ): array { - $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('impersonated'); - return $this->sourceCredentials->updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + return $this->sourceCredentials->fetchAuthToken($httpHandler, $metricsHeader); } /** diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 19fddeb86..17b9ca94b 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -84,6 +84,13 @@ class ServiceAccountCredentials extends CredentialsLoader implements */ protected $projectId; + /** + * User in observability metric headers + * + * @var string + */ + protected string $credType = 'cred-type/sa'; + /** * @var array|null */ @@ -211,9 +218,9 @@ public function fetchAuthToken(callable $httpHandler = null) $isAccessTokenRequest = false; } - $metricsHeader = $this->applyMetricHeaders( + $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue('sa', $isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) ); return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); } @@ -262,20 +269,16 @@ public function getProjectId(callable $httpHandler = null) * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request - * @param string $_metricsHeaderValue [INTERNAL] The observability metrics - * header value to be set on the request. * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, callable $httpHandler = null, - string $_metricsHeaderValue = '' ) { // scope exists. use oauth implementation if (!$this->useSelfSignedJwt()) { - $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('sa'); - return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + return parent::updateMetadata($metadata, $authUri, $httpHandler); } $jwtCreds = $this->createJwtAccessCredentials(); diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 81aeff7ba..39097ef1e 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -54,6 +54,13 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements */ protected $quotaProject; + /** + * User in observability metric headers + * + * @var string + */ + protected string $credType = 'cred-type/jwt'; + /** * @var string */ @@ -108,15 +115,12 @@ public function __construct($jsonKey, $scope = null) * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request - * @param string $_metricsHeaderValue [INTERNAL] The observability metrics - * header value to be set on the request. It's set internally and is not propagated * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, callable $httpHandler = null, - $_metricsHeaderValue = '' ) { $scope = $this->auth->getScope(); if (empty($authUri) && empty($scope)) { @@ -124,8 +128,7 @@ public function updateMetadata( } $this->auth->setAudience($authUri); - $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('jwt'); - return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + return parent::updateMetadata($metadata, $authUri, $httpHandler); } /** diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 7ebb739b7..935f0bd3d 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -48,6 +48,13 @@ class UserRefreshCredentials extends CredentialsLoader implements GetQuotaProjec */ protected $quotaProject; + /** + * User in observability metric headers + * + * @var string + */ + protected string $credType = 'cred-type/u'; + /** * Create a new UserRefreshCredentials. * @@ -98,7 +105,7 @@ public function __construct( /** * @param callable $httpHandler - * @param array $metricsHeaders [optional] Metrics headers to be inserted + * @param array $metricsHeader [optional] Metrics headers to be inserted * into the token endpoint request present. * This is passed from ImersonatedServiceAccountCredentials as it uses * UserRefreshCredentials as source credentials. @@ -117,15 +124,15 @@ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeade { // ImersonatedServiceAccountCredentials can propagate it's own header value, hence // the check for empty metricsHeaders - if (empty($metricsHeaders)) { + if (empty($metricsHeader)) { // We don't support id token endpoint requests as of now for User Cred $isAccessTokenRequest = true; - $metricsHeaders = $this->applyMetricHeaders( + $metricsHeader = $this->applyMetricsHeader( [], - $this->getTokenEndpointMetricsHeaderValue('user', $isAccessTokenRequest) + $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) ); } - return $this->auth->fetchAuthToken($httpHandler, $metricsHeaders); + return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); } /** @@ -134,23 +141,14 @@ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeade * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request - * @param string $_metricsHeaderValue [INTERNAL] The observability metrics - * header value to be set on the request. * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, callable $httpHandler = null, - string $_metricsHeaderValue = '' ): array { - // ImersonatedServiceAccountCredentials can propagate it's own header value, hence the check for empty $_metricsHeaderValue - if (empty($_metricsHeaderValue)) { - $metricsHeaderValue = $this->getServiceApiMetricsHeaderValue('user'); - } else { - $metricsHeaderValue = $_metricsHeaderValue; - } - return parent::updateMetadata($metadata, $authUri, $httpHandler, $metricsHeaderValue); + return parent::updateMetadata($metadata, $authUri, $httpHandler); } /** diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 111463e27..8beea220a 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -27,80 +27,73 @@ trait MetricsTrait { private static $version = null; - private static $headerKey = 'x-goog-api-client'; + protected static $metricsHeaderKey = 'x-goog-api-client'; private static array $requestType = [ 'accessToken' => 'auth-request-type/at', - 'idToken' => 'auth-request-type/idt', - 'mdsPing' => 'auth-request-type/mds' + 'idToken' => 'auth-request-type/it', ]; - private static array $credType = [ + private static array $credTypes = [ 'user' => 'cred-type/u', 'sa' => 'cred-type/sa', - 'sa-jwt' => 'cred-type/jwt', + 'jwt' => 'cred-type/jwt', 'gce' => 'cred-type/mds', 'impersonate' => 'cred-type/imp' ]; - private function getVersion(): string + protected string $credType = ''; + protected function getServiceApiMetricsHeaderValue(): string { - if (is_null(self::$version)) { - $versionFilePath = implode(DIRECTORY_SEPARATOR, [__DIR__, '..', 'VERSION']); - self::$version = trim((string) file_get_contents($versionFilePath)); + if (!empty($this->credType)) { + return $this->langAndVersion() . ' ' . $this->credType; } - return self::$version; + return ''; } - protected function getServiceApiMetricsHeaderValue(string $credType) + protected function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest): string { - switch ($credType) { - case 'user': - return ''; - case 'sa': - return ''; - case 'jwt': - return ''; - case 'gce': - return ''; - case 'impersonated': - return ''; + $value = $this->langAndVersion(); + if ($isAccessTokenRequest) { + $value .= ' ' . self::$requestType['accessToken']; + } else { + $value .= ' ' . self::$requestType['idToken']; } - return ''; - } - protected function getTokenEndpointMetricsHeaderValue(string $credType, bool $isAccessTokenRequest) - { - switch ($credType) { - case 'user': - return ''; - case 'sa': - return ''; - case 'jwt': - return ''; - case 'gce': - return ''; - case 'impersonated': - return ''; + if (!empty($this->credType)) { + return $value . ' ' . $this->credType; } - return ''; - } - protected static function getMdsPingHeader() - { - return []; + return ''; } - - protected function applyMetricHeaders($metadata, $headerValue) + protected function applyMetricsHeader($metadata, $headerValue): array { if (empty($headerValue)) { return $metadata; - } elseif (isset($metadata[self::$headerKey])) { - $metadata[self::$headerKey][0] .= ' ' . $headerValue; + } elseif (!isset($metadata[self::$metricsHeaderKey]) || empty($metadata[self::$metricsHeaderKey])) { + $metadata[self::$metricsHeaderKey] = [$headerValue]; + } elseif (is_array($metadata[self::$metricsHeaderKey])) { + $metadata[self::$metricsHeaderKey][0] .= ' ' . $headerValue; } else { - $metadata[self::$headerKey] = [$headerValue]; + // It's a string instead of array + $metadata[self::$metricsHeaderKey] .= ' ' . $headerValue; } + return $metadata; } + + protected static function getVersion(): string + { + if (is_null(self::$version)) { + $versionFilePath = implode(DIRECTORY_SEPARATOR, [__DIR__, '..', 'VERSION']); + self::$version = trim((string) file_get_contents($versionFilePath)); + } + return self::$version; + } + + private function langAndVersion(): string + { + return 'gl-php/' . PHP_VERSION . ' auth/' . self::getVersion(); + } } diff --git a/src/OAuth2.php b/src/OAuth2.php index 2a683ec5b..199b780e2 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -574,7 +574,7 @@ public function toJwt(array $config = []) * @param callable $httpHandler callback which delivers psr7 request * @return RequestInterface the authorization Url. */ - public function generateCredentialsRequest(callable $httpHandler = null, $metricsHeaders = []) + public function generateCredentialsRequest(callable $httpHandler = null, $metricsHeader = []) { $uri = $this->getTokenCredentialUri(); if (is_null($uri)) { @@ -633,7 +633,7 @@ public function generateCredentialsRequest(callable $httpHandler = null, $metric $headers = [ 'Cache-Control' => 'no-store', 'Content-Type' => 'application/x-www-form-urlencoded', - ] + $metricsHeaders; + ] + $metricsHeader; return new Request( 'POST', @@ -647,17 +647,17 @@ public function generateCredentialsRequest(callable $httpHandler = null, $metric * Fetches the auth tokens based on the current state. * * @param callable $httpHandler callback which delivers psr7 request - * @param array $metricsHeaders [optional] If present, add these headers to the token + * @param array $metricsHeader [optional] If present, add these headers to the token * endpoint request. * @return array the response */ - public function fetchAuthToken(callable $httpHandler = null, $metricsHeaders = []) + public function fetchAuthToken(callable $httpHandler = null, $metricsHeader = []) { if (is_null($httpHandler)) { $httpHandler = HttpHandlerFactory::build(HttpClientCache::getHttpClient()); } - $response = $httpHandler($this->generateCredentialsRequest($httpHandler, $metricsHeaders)); + $response = $httpHandler($this->generateCredentialsRequest($httpHandler, $metricsHeader)); $credentials = $this->parseTokenResponse($response); $this->updateToken($credentials); if (isset($credentials['scope'])) { diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 9db6453fa..8d9cbc7d2 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -43,15 +43,12 @@ public function getUpdateMetadataFunc() * @param array $metadata metadata hashmap * @param string $authUri optional auth uri * @param callable $httpHandler callback which delivers psr7 request - * @param string $metricsHeaderValue [optional] The observability metrics - * header value to be set on the request. * @return array updated metadata hashmap */ public function updateMetadata( $metadata, $authUri = null, callable $httpHandler = null, - string $metricsHeaderValue = '' ) { if (isset($metadata[self::AUTH_METADATA_KEY])) { // Auth metadata has already been set @@ -65,9 +62,9 @@ public function updateMetadata( $metadata_copy[self::AUTH_METADATA_KEY] = ['Bearer ' . $result['id_token']]; } - $metadata_copy = $this->applyMetricHeaders( + $metadata_copy = $this->applyMetricsHeader( $metadata_copy, - $metricsHeaderValue + $this->getServiceApiMetricsHeaderValue() ); return $metadata_copy; } diff --git a/tests/Credentials/GCECredentialsTest.php b/tests/Credentials/GCECredentialsTest.php index 695ba0195..72face85e 100644 --- a/tests/Credentials/GCECredentialsTest.php +++ b/tests/Credentials/GCECredentialsTest.php @@ -51,6 +51,24 @@ public function testOnGceMetadataFlavorHeader() $this->assertTrue($onGce); } + public function testOnGceMetricsHeader() + { + $handerInvoked = false; + $dummyHandler = function ($request) use (&$handerInvoked) { + $header = $request->getHeaderLine('x-goog-api-client'); + $handerInvoked = true; + $this->assertStringMatchesFormat( + 'gl-php/%s auth/%s auth-request-type/mds', + $header + ); + + return new Psr7\Response(200, [GCECredentials::FLAVOR_HEADER => 'Google']); + }; + + GCECredentials::onGce($dummyHandler); + $this->assertTrue($handerInvoked); + } + public function testOnGCEIsFalseOnClientErrorStatus() { // simulate retry attempts by returning multiple 400s diff --git a/tests/MetricsTraitTest.php b/tests/MetricsTraitTest.php index cf12a4b92..6dbcd0650 100644 --- a/tests/MetricsTraitTest.php +++ b/tests/MetricsTraitTest.php @@ -17,7 +17,15 @@ namespace Google\Auth\Tests; +use Google\Auth\Credentials\GCECredentials; +use Google\Auth\Credentials\ImpersonatedServiceAccountCredentials; +use Google\Auth\Credentials\ServiceAccountCredentials; +use Google\Auth\Credentials\ServiceAccountJwtAccessCredentials; +use Google\Auth\Credentials\UserRefreshCredentials; use Google\Auth\MetricsTrait; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\Psr7\Response; +use GuzzleHttp\Psr7\Utils; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -28,16 +36,172 @@ class MetricsTraitTest extends TestCase private $impl; + private static $headerKey = 'x-goog-api-client'; + + private $langAndVersion; + + private $jsonTokens; + public function setUp(): void { $this->impl = new class {use MetricsTrait { getVersion as public; + applyMetricsHeader as public; }}; + $this->langAndVersion = sprintf( + 'gl-php/%s auth/%s', + PHP_VERSION, + $this->impl::getVersion() + ); + $this->jsonTokens = json_encode(['access_token' => '1/abdef1234567890', 'expires_in' => '57']); } public function testGetVersion() { - $actualVersion = $this->impl->getVersion(); + $actualVersion = $this->impl::getVersion(); $this->assertStringMatchesFormat("%d.%d.%d", $actualVersion); } + + /** + * @dataProvider tokenRequestType + */ + public function testGCECredentials($scope, $targetAudience, $requestTypeHeaderValue) + { + $handlerCalled = false; + $jsonTokens = $this->jsonTokens; + $handler = getHandler([ + new Response(200, [GCECredentials::FLAVOR_HEADER => 'Google']), + function ($request, $options) use ( + $jsonTokens, + &$handlerCalled, + $requestTypeHeaderValue + ) { + $handlerCalled = true; + // This confirms that token endpoint requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s %s cred-type/mds', $this->langAndVersion, $requestTypeHeaderValue), + $request->getHeaderLine(self::$headerKey) + ); + return new Response(200, [], Utils::streamFor($jsonTokens)); + } + ]); + + $gceCred = new GCECredentials(null, $scope, $targetAudience); + $this->assertUpdateMetadata($gceCred, $handler, 'mds', $handlerCalled); + } + + /** + * @dataProvider tokenRequestType + */ + public function testServiceAccountCredentials($scope, $targetAudience, $requestTypeHeaderValue) + { + $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('sa', $requestTypeHeaderValue, $handlerCalled); + + $sa = new ServiceAccountCredentials( + $scope, + $keyFile, + null, + $targetAudience + ); + $this->assertUpdateMetadata($sa, $handler, 'sa', $handlerCalled); + } + + public function testServiceAccountJwtAccessCredentials() + { + $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; + $saJwt = new ServiceAccountJwtAccessCredentials($keyFile, 'exampleScope'); + $metadata = $saJwt->updateMetadata([], null, null); + $this->assertArrayHasKey(self::$headerKey, $metadata); + + // This confirms that service usage requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s cred-type/jwt', $this->langAndVersion), + $metadata[self::$headerKey][0] + ); + } + + public function testImpersonatedServiceAccountCredentials() + { + $keyFile = __DIR__ . '/fixtures5/.config/gcloud/application_default_credentials.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('imp', 'auth-request-type/at', $handlerCalled); + + $impersonatedCred = new ImpersonatedServiceAccountCredentials('exampleScope', $keyFile); + $this->assertUpdateMetadata($impersonatedCred, $handler, 'imp', $handlerCalled); + } + + public function testUserRefreshCredentials() + { + $keyFile = __DIR__ . '/fixtures2/gcloud.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('u', 'auth-request-type/at', $handlerCalled); + + $userRefreshCred = new UserRefreshCredentials('exampleScope', $keyFile); + $this->assertUpdateMetadata($userRefreshCred, $handler, 'u', $handlerCalled); + } + + private function assertUpdateMetadata($cred, $handler, $credShortform, &$handlerCalled) + { + $metadata = $cred->updateMetadata([], null, $handler); + $this->assertArrayHasKey(self::$headerKey, $metadata); + + // This confirms that service usage requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s cred-type/%s', $this->langAndVersion, $credShortform), + $metadata[self::$headerKey][0] + ); + + $this->assertTrue($handlerCalled); + } + + private function getCustomHandler($credShortform, $requestTypeHeaderValue, &$handlerCalled) + { + $jsonTokens = $this->jsonTokens; + return getHandler([ + function ($request, $options) use ( + $jsonTokens, + &$handlerCalled, + $requestTypeHeaderValue, + $credShortform + ) { + $handlerCalled = true; + // This confirms that token endpoint requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s %s cred-type/%s', $this->langAndVersion, $requestTypeHeaderValue, $credShortform), + $request->getHeaderLine(self::$headerKey) + ); + return new Response(200, [], Utils::streamFor($jsonTokens)); + } + ]); + } + + /** + * @dataProvider headerCases + */ + public function testApplyMetricsHeader($existingValue, $expected) + { + $metadata = [self::$headerKey => $existingValue]; + $metadata = $this->impl->applyMetricsHeader($metadata, 'bar'); + $this->assertEquals($expected, $metadata[self::$headerKey]); + } + + public function tokenRequestType() + { + return [ + ['someScope', null, 'auth-request-type/at'], + [null, 'someTargetAudience', 'auth-request-type/it'], + ]; + } + + public function headerCases() + { + return [ + ['', ['bar']], + ['foo', 'foo bar'], + [[], ['bar']], + [['foo'], ['foo bar']], + ]; + } } From bfe2cfed0895ddb501e90994be78056d8c7a8ca5 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 06:19:10 +0000 Subject: [PATCH 07/19] nit fix --- src/UpdateMetadataTrait.php | 2 +- tests/MetricsTraitTest.php | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 8d9cbc7d2..4186d313d 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -48,7 +48,7 @@ public function getUpdateMetadataFunc() public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null, + callable $httpHandler = null ) { if (isset($metadata[self::AUTH_METADATA_KEY])) { // Auth metadata has already been set diff --git a/tests/MetricsTraitTest.php b/tests/MetricsTraitTest.php index 6dbcd0650..d25c37d35 100644 --- a/tests/MetricsTraitTest.php +++ b/tests/MetricsTraitTest.php @@ -15,7 +15,7 @@ * limitations under the License. */ - namespace Google\Auth\Tests; +namespace Google\Auth\Tests; use Google\Auth\Credentials\GCECredentials; use Google\Auth\Credentials\ImpersonatedServiceAccountCredentials; @@ -23,11 +23,9 @@ use Google\Auth\Credentials\ServiceAccountJwtAccessCredentials; use Google\Auth\Credentials\UserRefreshCredentials; use Google\Auth\MetricsTrait; -use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\Utils; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; class MetricsTraitTest extends TestCase @@ -44,10 +42,12 @@ class MetricsTraitTest extends TestCase public function setUp(): void { - $this->impl = new class {use MetricsTrait { - getVersion as public; - applyMetricsHeader as public; - }}; + $this->impl = new class() { + use MetricsTrait { + getVersion as public; + applyMetricsHeader as public; + } + }; $this->langAndVersion = sprintf( 'gl-php/%s auth/%s', PHP_VERSION, @@ -59,7 +59,7 @@ public function setUp(): void public function testGetVersion() { $actualVersion = $this->impl::getVersion(); - $this->assertStringMatchesFormat("%d.%d.%d", $actualVersion); + $this->assertStringMatchesFormat('%d.%d.%d', $actualVersion); } /** From 3fc4be937febe542b58076330e49253145882e15 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 06:20:11 +0000 Subject: [PATCH 08/19] fix: Nit fix on docs --- src/Credentials/GCECredentials.php | 2 +- src/Credentials/ImpersonatedServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountJwtAccessCredentials.php | 2 +- src/Credentials/UserRefreshCredentials.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index 317c14180..af820f9a1 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -145,7 +145,7 @@ class GCECredentials extends CredentialsLoader implements protected $lastReceivedToken; /** - * User in observability metric headers + * Used in observability metric headers * * @var string */ diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index cce1664f0..5346b03a6 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -37,7 +37,7 @@ class ImpersonatedServiceAccountCredentials extends CredentialsLoader implements protected $sourceCredentials; /** - * User in observability metric headers + * Used in observability metric headers * * @var string */ diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 17b9ca94b..acfc129d1 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -85,7 +85,7 @@ class ServiceAccountCredentials extends CredentialsLoader implements protected $projectId; /** - * User in observability metric headers + * Used in observability metric headers * * @var string */ diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 39097ef1e..b6eef721e 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -55,7 +55,7 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements protected $quotaProject; /** - * User in observability metric headers + * Used in observability metric headers * * @var string */ diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 935f0bd3d..6d2ff07fd 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -49,7 +49,7 @@ class UserRefreshCredentials extends CredentialsLoader implements GetQuotaProjec protected $quotaProject; /** - * User in observability metric headers + * Used in observability metric headers * * @var string */ From 1c383b045b3ef845dd9545802480772e55631225 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 06:27:42 +0000 Subject: [PATCH 09/19] PR self review --- src/Credentials/ServiceAccountCredentials.php | 2 +- .../ServiceAccountJwtAccessCredentials.php | 3 ++- src/Credentials/UserRefreshCredentials.php | 20 ++----------------- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index acfc129d1..91126acde 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -274,7 +274,7 @@ public function getProjectId(callable $httpHandler = null) public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null, + callable $httpHandler = null ) { // scope exists. use oauth implementation if (!$this->useSelfSignedJwt()) { diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index b6eef721e..0ebb85548 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -120,7 +120,7 @@ public function __construct($jsonKey, $scope = null) public function updateMetadata( $metadata, $authUri = null, - callable $httpHandler = null, + callable $httpHandler = null ) { $scope = $this->auth->getScope(); if (empty($authUri) && empty($scope)) { @@ -128,6 +128,7 @@ public function updateMetadata( } $this->auth->setAudience($authUri); + return parent::updateMetadata($metadata, $authUri, $httpHandler); } diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 6d2ff07fd..b91d6dfad 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -107,7 +107,7 @@ public function __construct( * @param callable $httpHandler * @param array $metricsHeader [optional] Metrics headers to be inserted * into the token endpoint request present. - * This is passed from ImersonatedServiceAccountCredentials as it uses + * This could be passed from ImersonatedServiceAccountCredentials as it uses * UserRefreshCredentials as source credentials. * * @return array { @@ -123,7 +123,7 @@ public function __construct( public function fetchAuthToken(callable $httpHandler = null, array $metricsHeader = []) { // ImersonatedServiceAccountCredentials can propagate it's own header value, hence - // the check for empty metricsHeaders + // we'll pass them if present. if (empty($metricsHeader)) { // We don't support id token endpoint requests as of now for User Cred $isAccessTokenRequest = true; @@ -135,22 +135,6 @@ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeade return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); } - /** - * Updates metadata with the authorization token. - * - * @param array $metadata metadata hashmap - * @param string $authUri optional auth uri - * @param callable $httpHandler callback which delivers psr7 request - * @return array updated metadata hashmap - */ - public function updateMetadata( - $metadata, - $authUri = null, - callable $httpHandler = null, - ): array { - return parent::updateMetadata($metadata, $authUri, $httpHandler); - } - /** * @return string */ From aca91087439cb15bdfa79cca47bb954f9ea65c5a Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 10:12:49 +0000 Subject: [PATCH 10/19] fix: Static analysis fix and updatemetadata logic correction --- .../ExternalAccountCredentials.php | 2 ++ src/Credentials/GCECredentials.php | 9 +++-- .../ImpersonatedServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountCredentials.php | 2 +- .../ServiceAccountJwtAccessCredentials.php | 2 +- src/Credentials/UserRefreshCredentials.php | 4 +-- src/MetricsTrait.php | 35 ++++++++++++++++--- src/OAuth2.php | 4 ++- src/UpdateMetadataTrait.php | 16 ++++----- 9 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/Credentials/ExternalAccountCredentials.php b/src/Credentials/ExternalAccountCredentials.php index b2716bfaa..ed7f80c7b 100644 --- a/src/Credentials/ExternalAccountCredentials.php +++ b/src/Credentials/ExternalAccountCredentials.php @@ -25,6 +25,7 @@ use Google\Auth\GetQuotaProjectInterface; use Google\Auth\HttpHandler\HttpClientCache; use Google\Auth\HttpHandler\HttpHandlerFactory; +use Google\Auth\MetricsTrait; use Google\Auth\OAuth2; use Google\Auth\UpdateMetadataInterface; use Google\Auth\UpdateMetadataTrait; @@ -34,6 +35,7 @@ class ExternalAccountCredentials implements FetchAuthTokenInterface, UpdateMetadataInterface, GetQuotaProjectInterface { use UpdateMetadataTrait; + use MetricsTrait; private const EXTERNAL_ACCOUNT_TYPE = 'external_account'; diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index af820f9a1..e1e376c77 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -149,7 +149,7 @@ class GCECredentials extends CredentialsLoader implements * * @var string */ - protected string $credType = 'cred-type/mds'; + protected $credType = 'cred-type/mds'; /** * @var string|null @@ -315,6 +315,9 @@ private static function getProjectIdUri() return $base . self::PROJECT_ID_URI_PATH; } + /** + * @return array + */ private static function getMdsPingHeader() { return [ @@ -440,7 +443,7 @@ public function fetchAuthToken(callable $httpHandler = null) } $isAccessTokenRequest = true; - if (isset($this->targetAudience)) { + if (!is_null($this->targetAudience)) { $isAccessTokenRequest = false; } @@ -608,7 +611,7 @@ public function getUniverseDomain(callable $httpHandler = null): string * * @param callable $httpHandler An HTTP Handler to deliver PSR7 requests. * @param string $uri The metadata URI. - * @param array $metricsHeader [optional] If present, add these headers to the token + * @param array $metricsHeader [optional] If present, add these headers to the token * endpoint request. * * @return string diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 5346b03a6..67b1debc6 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -41,7 +41,7 @@ class ImpersonatedServiceAccountCredentials extends CredentialsLoader implements * * @var string */ - protected string $credType = 'cred-type/imp'; + protected $credType = 'cred-type/imp'; /** * Instantiate an instance of ImpersonatedServiceAccountCredentials from a credentials file that diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 91126acde..0affd69ea 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -89,7 +89,7 @@ class ServiceAccountCredentials extends CredentialsLoader implements * * @var string */ - protected string $credType = 'cred-type/sa'; + protected $credType = 'cred-type/sa'; /** * @var array|null diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 0ebb85548..dc5966e9c 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -59,7 +59,7 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements * * @var string */ - protected string $credType = 'cred-type/jwt'; + protected $credType = 'cred-type/jwt'; /** * @var string diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index b91d6dfad..66f95fee4 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -53,7 +53,7 @@ class UserRefreshCredentials extends CredentialsLoader implements GetQuotaProjec * * @var string */ - protected string $credType = 'cred-type/u'; + protected $credType = 'cred-type/u'; /** * Create a new UserRefreshCredentials. @@ -105,7 +105,7 @@ public function __construct( /** * @param callable $httpHandler - * @param array $metricsHeader [optional] Metrics headers to be inserted + * @param array $metricsHeader [optional] Metrics headers to be inserted * into the token endpoint request present. * This could be passed from ImersonatedServiceAccountCredentials as it uses * UserRefreshCredentials as source credentials. diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 8beea220a..13f937b7d 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -25,16 +25,30 @@ */ trait MetricsTrait { - private static $version = null; + /** + * @var string The version of the auth library php. + */ + private static $version; + /** + * @var string The header key for the observability metrics. + */ protected static $metricsHeaderKey = 'x-goog-api-client'; - private static array $requestType = [ + /** + * @var array The request type header values + * for the observability metrics. + */ + private static $requestType = [ 'accessToken' => 'auth-request-type/at', 'idToken' => 'auth-request-type/it', ]; - private static array $credTypes = [ + /** + * @var array The credential type headervalues + * for the observability metrics. + */ + private static $credTypes = [ 'user' => 'cred-type/u', 'sa' => 'cred-type/sa', 'jwt' => 'cred-type/jwt', @@ -42,7 +56,12 @@ trait MetricsTrait 'impersonate' => 'cred-type/imp' ]; - protected string $credType = ''; + /** + * @var string The credential type for the observability metrics. + * This would be overridden by the credential class if applicable. + */ + protected $credType = ''; + protected function getServiceApiMetricsHeaderValue(): string { if (!empty($this->credType)) { @@ -67,7 +86,13 @@ protected function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest return ''; } - protected function applyMetricsHeader($metadata, $headerValue): array + /** + * @param array $metadata The metadata to update and return. + * @param string $headerValue The header value to add to the metadata for + * observability metrics. + * @return array The updated metadata. + */ + protected function applyMetricsHeader($metadata, $headerValue) { if (empty($headerValue)) { return $metadata; diff --git a/src/OAuth2.php b/src/OAuth2.php index 199b780e2..b160bd223 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -572,6 +572,8 @@ public function toJwt(array $config = []) * Generates a request for token credentials. * * @param callable $httpHandler callback which delivers psr7 request + * @param array $metricsHeader [optional] Metrics headers to be inserted + * into the token endpoint request present. * @return RequestInterface the authorization Url. */ public function generateCredentialsRequest(callable $httpHandler = null, $metricsHeader = []) @@ -647,7 +649,7 @@ public function generateCredentialsRequest(callable $httpHandler = null, $metric * Fetches the auth tokens based on the current state. * * @param callable $httpHandler callback which delivers psr7 request - * @param array $metricsHeader [optional] If present, add these headers to the token + * @param array $metricsHeader [optional] If present, add these headers to the token * endpoint request. * @return array the response */ diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 4186d313d..97945778d 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -50,22 +50,22 @@ public function updateMetadata( $authUri = null, callable $httpHandler = null ) { - if (isset($metadata[self::AUTH_METADATA_KEY])) { + $metadata_copy = $metadata; + $metadata_copy = $this->applyMetricsHeader( + $metadata_copy, + $this->getServiceApiMetricsHeaderValue() + ); + + if (isset($metadata_copy[self::AUTH_METADATA_KEY])) { // Auth metadata has already been set - return $metadata; + return $metadata_copy; } $result = $this->fetchAuthToken($httpHandler); - $metadata_copy = $metadata; if (isset($result['access_token'])) { $metadata_copy[self::AUTH_METADATA_KEY] = ['Bearer ' . $result['access_token']]; } elseif (isset($result['id_token'])) { $metadata_copy[self::AUTH_METADATA_KEY] = ['Bearer ' . $result['id_token']]; } - - $metadata_copy = $this->applyMetricsHeader( - $metadata_copy, - $this->getServiceApiMetricsHeaderValue() - ); return $metadata_copy; } } From 2640b978b6bf9e50c232f5078441416b9623fea4 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 19 Dec 2023 13:11:29 +0000 Subject: [PATCH 11/19] Self review: 1) Simplify token endpoint metrics header getter logic 2) Update documentation 3) Add explaination to the tests. --- src/Credentials/UserRefreshCredentials.php | 4 +- src/MetricsTrait.php | 6 +-- tests/MetricsTraitTest.php | 58 +++++++++++++++------- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 66f95fee4..440e73b95 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -122,8 +122,8 @@ public function __construct( */ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeader = []) { - // ImersonatedServiceAccountCredentials can propagate it's own header value, hence - // we'll pass them if present. + // ImersonatedServiceAccountCredentials will propagate it's own header value, hence + // we'll pass them if present, else create headers for UserCred and pass along. if (empty($metricsHeader)) { // We don't support id token endpoint requests as of now for User Cred $isAccessTokenRequest = true; diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 13f937b7d..0611a6100 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -73,11 +73,7 @@ protected function getServiceApiMetricsHeaderValue(): string protected function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest): string { $value = $this->langAndVersion(); - if ($isAccessTokenRequest) { - $value .= ' ' . self::$requestType['accessToken']; - } else { - $value .= ' ' . self::$requestType['idToken']; - } + $value .= ' ' . self::$requestType[($isAccessTokenRequest ? 'accessToken' : 'idToken')]; if (!empty($this->credType)) { return $value . ' ' . $this->credType; diff --git a/tests/MetricsTraitTest.php b/tests/MetricsTraitTest.php index d25c37d35..31a5a100e 100644 --- a/tests/MetricsTraitTest.php +++ b/tests/MetricsTraitTest.php @@ -108,6 +108,10 @@ public function testServiceAccountCredentials($scope, $targetAudience, $requestT $this->assertUpdateMetadata($sa, $handler, 'sa', $handlerCalled); } + /** + * ServiceAccountJwtAccessCredentials creates the jwt token within library hence + * they don't have any observability metrics header check for token endpoint requests. + */ public function testServiceAccountJwtAccessCredentials() { $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; @@ -122,6 +126,9 @@ public function testServiceAccountJwtAccessCredentials() ); } + /** + * ImpersonatedServiceAccountCredentials haven't enabled identity token support hence + * they don't have 'auth-request-type/it' observability metric header check. public function testImpersonatedServiceAccountCredentials() { $keyFile = __DIR__ . '/fixtures5/.config/gcloud/application_default_credentials.json'; @@ -132,6 +139,10 @@ public function testImpersonatedServiceAccountCredentials() $this->assertUpdateMetadata($impersonatedCred, $handler, 'imp', $handlerCalled); } + /** + * UserRefreshCredentials haven't enabled identity token support hence + * they don't have 'auth-request-type/it' observability metric header check. + */ public function testUserRefreshCredentials() { $keyFile = __DIR__ . '/fixtures2/gcloud.json'; @@ -142,6 +153,20 @@ public function testUserRefreshCredentials() $this->assertUpdateMetadata($userRefreshCred, $handler, 'u', $handlerCalled); } + /** + * @dataProvider headerCases + */ + public function testApplyMetricsHeader($existingValue, $expected) + { + $metadata = [self::$headerKey => $existingValue]; + $metadata = $this->impl->applyMetricsHeader($metadata, 'bar'); + $this->assertEquals($expected, $metadata[self::$headerKey]); + } + + /** + * Invokes the 'updateMetadata' method of cred fetcher with empty metadata argument + * and asserts for proper service api usage observability metrics header. + */ private function assertUpdateMetadata($cred, $handler, $credShortform, &$handlerCalled) { $metadata = $cred->updateMetadata([], null, $handler); @@ -156,6 +181,15 @@ private function assertUpdateMetadata($cred, $handler, $credShortform, &$handler $this->assertTrue($handlerCalled); } + /** + * @param string $credShortform The short form of the credential type + * used in observability metric header value. + * @param string $requestTypeHeaderValue Expected header value of the form + * 'auth-request-type/<>' + * @param bool $handlerCalled Reference to the handlerCalled flag asserted later + * in the test. + * @return callable + */ private function getCustomHandler($credShortform, $requestTypeHeaderValue, &$handlerCalled) { $jsonTokens = $this->jsonTokens; @@ -177,14 +211,14 @@ function ($request, $options) use ( ]); } - /** - * @dataProvider headerCases - */ - public function testApplyMetricsHeader($existingValue, $expected) + public function headerCases() { - $metadata = [self::$headerKey => $existingValue]; - $metadata = $this->impl->applyMetricsHeader($metadata, 'bar'); - $this->assertEquals($expected, $metadata[self::$headerKey]); + return [ + ['', ['bar']], + ['foo', 'foo bar'], + [[], ['bar']], + [['foo'], ['foo bar']], + ]; } public function tokenRequestType() @@ -194,14 +228,4 @@ public function tokenRequestType() [null, 'someTargetAudience', 'auth-request-type/it'], ]; } - - public function headerCases() - { - return [ - ['', ['bar']], - ['foo', 'foo bar'], - [[], ['bar']], - [['foo'], ['foo bar']], - ]; - } } From 0293833af0c36cf0fc7fe52db3ccd53b07bccbd9 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:26:58 +0000 Subject: [PATCH 12/19] PR Iteration --- .../ExternalAccountCredentials.php | 2 - src/Credentials/GCECredentials.php | 52 ++--- .../ImpersonatedServiceAccountCredentials.php | 22 +- src/Credentials/ServiceAccountCredentials.php | 32 ++- .../ServiceAccountJwtAccessCredentials.php | 19 +- src/Credentials/UserRefreshCredentials.php | 35 ++- src/CredentialsLoader.php | 1 - src/MetricsTrait.php | 99 +++++---- src/OAuth2.php | 14 +- src/UpdateMetadataTrait.php | 8 +- tests/MetricsTraitTest.php | 189 +--------------- tests/ObservabilityMetricsTest.php | 203 ++++++++++++++++++ 12 files changed, 345 insertions(+), 331 deletions(-) create mode 100644 tests/ObservabilityMetricsTest.php diff --git a/src/Credentials/ExternalAccountCredentials.php b/src/Credentials/ExternalAccountCredentials.php index ed7f80c7b..b2716bfaa 100644 --- a/src/Credentials/ExternalAccountCredentials.php +++ b/src/Credentials/ExternalAccountCredentials.php @@ -25,7 +25,6 @@ use Google\Auth\GetQuotaProjectInterface; use Google\Auth\HttpHandler\HttpClientCache; use Google\Auth\HttpHandler\HttpHandlerFactory; -use Google\Auth\MetricsTrait; use Google\Auth\OAuth2; use Google\Auth\UpdateMetadataInterface; use Google\Auth\UpdateMetadataTrait; @@ -35,7 +34,6 @@ class ExternalAccountCredentials implements FetchAuthTokenInterface, UpdateMetadataInterface, GetQuotaProjectInterface { use UpdateMetadataTrait; - use MetricsTrait; private const EXTERNAL_ACCOUNT_TYPE = 'external_account'; diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index e1e376c77..f7eaa0c68 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -110,6 +110,8 @@ class GCECredentials extends CredentialsLoader implements */ private const GKE_PRODUCT_NAME_FILE = '/sys/class/dmi/id/product_name'; + private const CRED_TYPE = 'mds'; + /** * Note: the explicit `timeout` and `tries` below is a workaround. The underlying * issue is that resolving an unknown host on some networks will take @@ -144,13 +146,6 @@ class GCECredentials extends CredentialsLoader implements */ protected $lastReceivedToken; - /** - * Used in observability metric headers - * - * @var string - */ - protected $credType = 'cred-type/mds'; - /** * @var string|null */ @@ -315,20 +310,6 @@ private static function getProjectIdUri() return $base . self::PROJECT_ID_URI_PATH; } - /** - * @return array - */ - private static function getMdsPingHeader() - { - return [ - self::$metricsHeaderKey => [sprintf( - 'gl-php/%s auth/%s auth-request-type/mds', - PHP_VERSION, - self::getVersion() - )] - ]; - } - /** * The full uri for accessing the default universe domain. * @@ -380,7 +361,10 @@ public static function onGce(callable $httpHandler = null) new Request( 'GET', $checkUri, - [self::FLAVOR_HEADER => 'Google'] + self::getMdsPingHeader() + [ + self::FLAVOR_HEADER => 'Google', + self::$metricMetadataKey => self::getMetricsHeader('', 'mds') + ] ), ['timeout' => self::COMPUTE_PING_CONNECTION_TIMEOUT_S] ); @@ -442,16 +426,11 @@ public function fetchAuthToken(callable $httpHandler = null) return []; // return an empty array with no access token } - $isAccessTokenRequest = true; - if (!is_null($this->targetAudience)) { - $isAccessTokenRequest = false; - } - - $metricsHeader = $this->applyMetricsHeader( - [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + $response = $this->getFromMetadata( + $httpHandler, + $this->tokenUri, + $this->applyTokenEndpointMetrics([], $this->targetAudience ? 'it' : 'at') ); - $response = $this->getFromMetadata($httpHandler, $this->tokenUri, $metricsHeader); if ($this->targetAudience) { return ['id_token' => $response]; @@ -611,18 +590,18 @@ public function getUniverseDomain(callable $httpHandler = null): string * * @param callable $httpHandler An HTTP Handler to deliver PSR7 requests. * @param string $uri The metadata URI. - * @param array $metricsHeader [optional] If present, add these headers to the token + * @param array $headers [optional] If present, add these headers to the token * endpoint request. * * @return string */ - private function getFromMetadata(callable $httpHandler, $uri, array $metricsHeader = []) + private function getFromMetadata(callable $httpHandler, $uri, array $headers = []) { $resp = $httpHandler( new Request( 'GET', $uri, - [self::FLAVOR_HEADER => 'Google'] + $metricsHeader + [self::FLAVOR_HEADER => 'Google'] + $headers ) ); @@ -654,4 +633,9 @@ public function setIsOnGce($isOnGce) // Set isOnGce $this->isOnGce = $isOnGce; } + + public function getCredType(): string + { + return self::CRED_TYPE; + } } diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 67b1debc6..1222b0fc2 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -26,6 +26,8 @@ class ImpersonatedServiceAccountCredentials extends CredentialsLoader implements { use IamSignerTrait; + private const CRED_TYPE = 'imp'; + /** * @var string */ @@ -36,13 +38,6 @@ class ImpersonatedServiceAccountCredentials extends CredentialsLoader implements */ protected $sourceCredentials; - /** - * Used in observability metric headers - * - * @var string - */ - protected $credType = 'cred-type/imp'; - /** * Instantiate an instance of ImpersonatedServiceAccountCredentials from a credentials file that * has be created with the --impersonated-service-account flag. @@ -129,12 +124,10 @@ public function getClientName(callable $unusedHttpHandler = null) public function fetchAuthToken(callable $httpHandler = null) { // We don't support id token endpoint requests as of now for Impersonated Cred - $isAccessTokenRequest = true; - $metricsHeader = $this->applyMetricsHeader( - [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) + return $this->sourceCredentials->fetchAuthToken( + $httpHandler, + $this->applyTokenEndpointMetrics([], 'at') ); - return $this->sourceCredentials->fetchAuthToken($httpHandler, $metricsHeader); } /** @@ -152,4 +145,9 @@ public function getLastReceivedToken() { return $this->sourceCredentials->getLastReceivedToken(); } + + public function getCredType(): string + { + return self::CRED_TYPE; + } } diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 0affd69ea..f20fab6e6 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -65,6 +65,13 @@ class ServiceAccountCredentials extends CredentialsLoader implements { use ServiceAccountSignerTrait; + /** + * Used in observability metric headers + * + * @var string + */ + private const CRED_TYPE = 'sa'; + /** * The OAuth2 instance used to conduct authorization. * @@ -84,13 +91,6 @@ class ServiceAccountCredentials extends CredentialsLoader implements */ protected $projectId; - /** - * Used in observability metric headers - * - * @var string - */ - protected $credType = 'cred-type/sa'; - /** * @var array|null */ @@ -213,16 +213,9 @@ public function fetchAuthToken(callable $httpHandler = null) return $accessToken; } - $isAccessTokenRequest = true; - if (isset($this->auth->getAdditionalClaims()['target_audience'])) { - $isAccessTokenRequest = false; - } - - $metricsHeader = $this->applyMetricsHeader( - [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) - ); - return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); + $authRequestType = empty($this->auth->getAdditionalClaims()['target_audience']) + ? 'at' : 'it'; + return $this->auth->fetchAuthToken($httpHandler, $this->applyTokenEndpointMetrics([], $authRequestType)); } /** @@ -360,6 +353,11 @@ public function getUniverseDomain(): string return $this->universeDomain; } + public function getCredType(): string + { + return self::CRED_TYPE; + } + /** * @return bool */ diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index dc5966e9c..ed86545cf 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -40,6 +40,13 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements { use ServiceAccountSignerTrait; + /** + * Used in observability metric headers + * + * @var string + */ + private const CRED_TYPE = 'jwt'; + /** * The OAuth2 instance used to conduct authorization. * @@ -54,13 +61,6 @@ class ServiceAccountJwtAccessCredentials extends CredentialsLoader implements */ protected $quotaProject; - /** - * Used in observability metric headers - * - * @var string - */ - protected $credType = 'cred-type/jwt'; - /** * @var string */ @@ -212,4 +212,9 @@ public function getQuotaProject() { return $this->quotaProject; } + + public function getCredType(): string + { + return self::CRED_TYPE; + } } diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 440e73b95..154df2fea 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -34,6 +34,13 @@ */ class UserRefreshCredentials extends CredentialsLoader implements GetQuotaProjectInterface { + /** + * Used in observability metric headers + * + * @var string + */ + private const CRED_TYPE = 'u'; + /** * The OAuth2 instance used to conduct authorization. * @@ -48,13 +55,6 @@ class UserRefreshCredentials extends CredentialsLoader implements GetQuotaProjec */ protected $quotaProject; - /** - * Used in observability metric headers - * - * @var string - */ - protected $credType = 'cred-type/u'; - /** * Create a new UserRefreshCredentials. * @@ -122,17 +122,11 @@ public function __construct( */ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeader = []) { - // ImersonatedServiceAccountCredentials will propagate it's own header value, hence - // we'll pass them if present, else create headers for UserCred and pass along. - if (empty($metricsHeader)) { - // We don't support id token endpoint requests as of now for User Cred - $isAccessTokenRequest = true; - $metricsHeader = $this->applyMetricsHeader( - [], - $this->getTokenEndpointMetricsHeaderValue($isAccessTokenRequest) - ); - } - return $this->auth->fetchAuthToken($httpHandler, $metricsHeader); + // We don't support id token endpoint requests as of now for User Cred + return $this->auth->fetchAuthToken( + $httpHandler, + $this->applyTokenEndpointMetrics($metricsHeader, 'at') + ); } /** @@ -170,4 +164,9 @@ public function getGrantedScope() { return $this->auth->getGrantedScope(); } + + public function getCredType(): string + { + return self::CRED_TYPE; + } } diff --git a/src/CredentialsLoader.php b/src/CredentialsLoader.php index cef46a4e0..746b957a9 100644 --- a/src/CredentialsLoader.php +++ b/src/CredentialsLoader.php @@ -34,7 +34,6 @@ abstract class CredentialsLoader implements FetchAuthTokenInterface, UpdateMetadataInterface { - use MetricsTrait; use UpdateMetadataTrait; const TOKEN_CREDENTIAL_URI = 'https://oauth2.googleapis.com/token'; diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 0611a6100..2b0c5c599 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -33,74 +33,71 @@ trait MetricsTrait /** * @var string The header key for the observability metrics. */ - protected static $metricsHeaderKey = 'x-goog-api-client'; + protected static $metricMetadataKey = 'x-goog-api-client'; /** - * @var array The request type header values - * for the observability metrics. + * @param string $credType [Optional] The credential type. + * Empty value will not add any credential type to the header. + * Should be one of `'sa'`, `'jwt'`, `'imp'`, `'mds'`, `'u'`. + * @param string $authRequestType [Optional] The auth request type. + * Empty value will not add any auth request type to the header. + * Should be one of `'at'`, `'it'`, `'mds'`. + * @return string The header value for the observability metrics. */ - private static $requestType = [ - 'accessToken' => 'auth-request-type/at', - 'idToken' => 'auth-request-type/it', - ]; + protected static function getMetricsHeader( + $credType = '', + $authRequestType = '' + ): string { + $value = sprintf( + 'gl-php/%s auth/%s', + PHP_VERSION, + self::getVersion() + ); - /** - * @var array The credential type headervalues - * for the observability metrics. - */ - private static $credTypes = [ - 'user' => 'cred-type/u', - 'sa' => 'cred-type/sa', - 'jwt' => 'cred-type/jwt', - 'gce' => 'cred-type/mds', - 'impersonate' => 'cred-type/imp' - ]; - - /** - * @var string The credential type for the observability metrics. - * This would be overridden by the credential class if applicable. - */ - protected $credType = ''; + if (!empty($authRequestType)) { + $value .= ' auth-request-type/' . $authRequestType; + } - protected function getServiceApiMetricsHeaderValue(): string - { - if (!empty($this->credType)) { - return $this->langAndVersion() . ' ' . $this->credType; + if (!empty($credType)) { + $value .= ' cred-type/' . $credType; } - return ''; + + return $value; } - protected function getTokenEndpointMetricsHeaderValue(bool $isAccessTokenRequest): string + /** + * @param array $metadata The metadata to update and return. + * @return array The updated metadata. + */ + protected function applyServiceApiUsageMetrics($metadata) { - $value = $this->langAndVersion(); - $value .= ' ' . self::$requestType[($isAccessTokenRequest ? 'accessToken' : 'idToken')]; - - if (!empty($this->credType)) { - return $value . ' ' . $this->credType; + if ($credType = $this->getCredType()) { + // Add service api usage observability metrics info to metadata + $metricsHeader = self::getMetricsHeader($credType); + if (!isset($metadata[self::$metricMetadataKey])) { + $metadata[self::$metricMetadataKey] = [$metricsHeader]; + } elseif (is_array($metadata[self::$metricMetadataKey])) { + $metadata[self::$metricMetadataKey][0] .= ' ' . $metricsHeader; + } else { + $metadata[self::$metricMetadataKey] .= ' ' . $metricsHeader; + } } - return ''; + return $metadata; } /** * @param array $metadata The metadata to update and return. - * @param string $headerValue The header value to add to the metadata for - * observability metrics. + * @param string $authRequestType The auth request type. Possible values are + * `'at'`, `'it'`, `'mds'`. * @return array The updated metadata. */ - protected function applyMetricsHeader($metadata, $headerValue) + protected function applyTokenEndpointMetrics($metadata, $authRequestType) { - if (empty($headerValue)) { - return $metadata; - } elseif (!isset($metadata[self::$metricsHeaderKey]) || empty($metadata[self::$metricsHeaderKey])) { - $metadata[self::$metricsHeaderKey] = [$headerValue]; - } elseif (is_array($metadata[self::$metricsHeaderKey])) { - $metadata[self::$metricsHeaderKey][0] .= ' ' . $headerValue; - } else { - // It's a string instead of array - $metadata[self::$metricsHeaderKey] .= ' ' . $headerValue; + $metricsHeader = $this->getMetricsHeader($this->getCredType(), $authRequestType); + if (!isset($metadata[self::$metricMetadataKey])) { + $metadata[self::$metricMetadataKey] = $metricsHeader; } - return $metadata; } @@ -113,8 +110,8 @@ protected static function getVersion(): string return self::$version; } - private function langAndVersion(): string + public function getCredType(): string { - return 'gl-php/' . PHP_VERSION . ' auth/' . self::getVersion(); + return ''; } } diff --git a/src/OAuth2.php b/src/OAuth2.php index b160bd223..c964b9a8c 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -572,11 +572,11 @@ public function toJwt(array $config = []) * Generates a request for token credentials. * * @param callable $httpHandler callback which delivers psr7 request - * @param array $metricsHeader [optional] Metrics headers to be inserted - * into the token endpoint request present. + * @param array $headers [optional] Additional headers to pass to + * the token endpoint request. * @return RequestInterface the authorization Url. */ - public function generateCredentialsRequest(callable $httpHandler = null, $metricsHeader = []) + public function generateCredentialsRequest(callable $httpHandler = null, $headers = []) { $uri = $this->getTokenCredentialUri(); if (is_null($uri)) { @@ -635,7 +635,7 @@ public function generateCredentialsRequest(callable $httpHandler = null, $metric $headers = [ 'Cache-Control' => 'no-store', 'Content-Type' => 'application/x-www-form-urlencoded', - ] + $metricsHeader; + ] + $headers; return new Request( 'POST', @@ -649,17 +649,17 @@ public function generateCredentialsRequest(callable $httpHandler = null, $metric * Fetches the auth tokens based on the current state. * * @param callable $httpHandler callback which delivers psr7 request - * @param array $metricsHeader [optional] If present, add these headers to the token + * @param array $headers [optional] If present, add these headers to the token * endpoint request. * @return array the response */ - public function fetchAuthToken(callable $httpHandler = null, $metricsHeader = []) + public function fetchAuthToken(callable $httpHandler = null, $headers = []) { if (is_null($httpHandler)) { $httpHandler = HttpHandlerFactory::build(HttpClientCache::getHttpClient()); } - $response = $httpHandler($this->generateCredentialsRequest($httpHandler, $metricsHeader)); + $response = $httpHandler($this->generateCredentialsRequest($httpHandler, $headers)); $credentials = $this->parseTokenResponse($response); $this->updateToken($credentials); if (isset($credentials['scope'])) { diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 97945778d..2f7592846 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -26,6 +26,8 @@ */ trait UpdateMetadataTrait { + use MetricsTrait; + /** * export a callback function which updates runtime metadata. * @@ -51,10 +53,8 @@ public function updateMetadata( callable $httpHandler = null ) { $metadata_copy = $metadata; - $metadata_copy = $this->applyMetricsHeader( - $metadata_copy, - $this->getServiceApiMetricsHeaderValue() - ); + + $metadata_copy = $this->applyServiceApiUsageMetrics($metadata_copy); if (isset($metadata_copy[self::AUTH_METADATA_KEY])) { // Auth metadata has already been set diff --git a/tests/MetricsTraitTest.php b/tests/MetricsTraitTest.php index 31a5a100e..c16211507 100644 --- a/tests/MetricsTraitTest.php +++ b/tests/MetricsTraitTest.php @@ -17,14 +17,7 @@ namespace Google\Auth\Tests; -use Google\Auth\Credentials\GCECredentials; -use Google\Auth\Credentials\ImpersonatedServiceAccountCredentials; -use Google\Auth\Credentials\ServiceAccountCredentials; -use Google\Auth\Credentials\ServiceAccountJwtAccessCredentials; -use Google\Auth\Credentials\UserRefreshCredentials; use Google\Auth\MetricsTrait; -use GuzzleHttp\Psr7\Response; -use GuzzleHttp\Psr7\Utils; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; @@ -34,26 +27,15 @@ class MetricsTraitTest extends TestCase private $impl; - private static $headerKey = 'x-goog-api-client'; - - private $langAndVersion; - - private $jsonTokens; - public function setUp(): void { $this->impl = new class() { - use MetricsTrait { + use MetricsTrait{ getVersion as public; - applyMetricsHeader as public; + getMetricsHeader as public; } }; - $this->langAndVersion = sprintf( - 'gl-php/%s auth/%s', - PHP_VERSION, - $this->impl::getVersion() - ); - $this->jsonTokens = json_encode(['access_token' => '1/abdef1234567890', 'expires_in' => '57']); + } public function testGetVersion() @@ -63,169 +45,20 @@ public function testGetVersion() } /** - * @dataProvider tokenRequestType - */ - public function testGCECredentials($scope, $targetAudience, $requestTypeHeaderValue) - { - $handlerCalled = false; - $jsonTokens = $this->jsonTokens; - $handler = getHandler([ - new Response(200, [GCECredentials::FLAVOR_HEADER => 'Google']), - function ($request, $options) use ( - $jsonTokens, - &$handlerCalled, - $requestTypeHeaderValue - ) { - $handlerCalled = true; - // This confirms that token endpoint requests have proper observability metric headers - $this->assertStringContainsString( - sprintf('%s %s cred-type/mds', $this->langAndVersion, $requestTypeHeaderValue), - $request->getHeaderLine(self::$headerKey) - ); - return new Response(200, [], Utils::streamFor($jsonTokens)); - } - ]); - - $gceCred = new GCECredentials(null, $scope, $targetAudience); - $this->assertUpdateMetadata($gceCred, $handler, 'mds', $handlerCalled); - } - - /** - * @dataProvider tokenRequestType - */ - public function testServiceAccountCredentials($scope, $targetAudience, $requestTypeHeaderValue) - { - $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; - $handlerCalled = false; - $handler = $this->getCustomHandler('sa', $requestTypeHeaderValue, $handlerCalled); - - $sa = new ServiceAccountCredentials( - $scope, - $keyFile, - null, - $targetAudience - ); - $this->assertUpdateMetadata($sa, $handler, 'sa', $handlerCalled); - } - - /** - * ServiceAccountJwtAccessCredentials creates the jwt token within library hence - * they don't have any observability metrics header check for token endpoint requests. - */ - public function testServiceAccountJwtAccessCredentials() - { - $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; - $saJwt = new ServiceAccountJwtAccessCredentials($keyFile, 'exampleScope'); - $metadata = $saJwt->updateMetadata([], null, null); - $this->assertArrayHasKey(self::$headerKey, $metadata); - - // This confirms that service usage requests have proper observability metric headers - $this->assertStringContainsString( - sprintf('%s cred-type/jwt', $this->langAndVersion), - $metadata[self::$headerKey][0] - ); - } - - /** - * ImpersonatedServiceAccountCredentials haven't enabled identity token support hence - * they don't have 'auth-request-type/it' observability metric header check. - public function testImpersonatedServiceAccountCredentials() - { - $keyFile = __DIR__ . '/fixtures5/.config/gcloud/application_default_credentials.json'; - $handlerCalled = false; - $handler = $this->getCustomHandler('imp', 'auth-request-type/at', $handlerCalled); - - $impersonatedCred = new ImpersonatedServiceAccountCredentials('exampleScope', $keyFile); - $this->assertUpdateMetadata($impersonatedCred, $handler, 'imp', $handlerCalled); - } - - /** - * UserRefreshCredentials haven't enabled identity token support hence - * they don't have 'auth-request-type/it' observability metric header check. - */ - public function testUserRefreshCredentials() - { - $keyFile = __DIR__ . '/fixtures2/gcloud.json'; - $handlerCalled = false; - $handler = $this->getCustomHandler('u', 'auth-request-type/at', $handlerCalled); - - $userRefreshCred = new UserRefreshCredentials('exampleScope', $keyFile); - $this->assertUpdateMetadata($userRefreshCred, $handler, 'u', $handlerCalled); - } - - /** - * @dataProvider headerCases - */ - public function testApplyMetricsHeader($existingValue, $expected) - { - $metadata = [self::$headerKey => $existingValue]; - $metadata = $this->impl->applyMetricsHeader($metadata, 'bar'); - $this->assertEquals($expected, $metadata[self::$headerKey]); - } - - /** - * Invokes the 'updateMetadata' method of cred fetcher with empty metadata argument - * and asserts for proper service api usage observability metrics header. + * @dataProvider metricsHeaderCases */ - private function assertUpdateMetadata($cred, $handler, $credShortform, &$handlerCalled) + public function testGetMetricsHeader($credType, $authRequestType, $expected) { - $metadata = $cred->updateMetadata([], null, $handler); - $this->assertArrayHasKey(self::$headerKey, $metadata); - - // This confirms that service usage requests have proper observability metric headers - $this->assertStringContainsString( - sprintf('%s cred-type/%s', $this->langAndVersion, $credShortform), - $metadata[self::$headerKey][0] - ); - - $this->assertTrue($handlerCalled); - } - - /** - * @param string $credShortform The short form of the credential type - * used in observability metric header value. - * @param string $requestTypeHeaderValue Expected header value of the form - * 'auth-request-type/<>' - * @param bool $handlerCalled Reference to the handlerCalled flag asserted later - * in the test. - * @return callable - */ - private function getCustomHandler($credShortform, $requestTypeHeaderValue, &$handlerCalled) - { - $jsonTokens = $this->jsonTokens; - return getHandler([ - function ($request, $options) use ( - $jsonTokens, - &$handlerCalled, - $requestTypeHeaderValue, - $credShortform - ) { - $handlerCalled = true; - // This confirms that token endpoint requests have proper observability metric headers - $this->assertStringContainsString( - sprintf('%s %s cred-type/%s', $this->langAndVersion, $requestTypeHeaderValue, $credShortform), - $request->getHeaderLine(self::$headerKey) - ); - return new Response(200, [], Utils::streamFor($jsonTokens)); - } - ]); - } - - public function headerCases() - { - return [ - ['', ['bar']], - ['foo', 'foo bar'], - [[], ['bar']], - [['foo'], ['foo bar']], - ]; + $headerValue = $this->impl::getMetricsHeader($credType, $authRequestType); + $this->assertStringMatchesFormat('gl-php/%s auth/%s ' . $expected, $headerValue); } - public function tokenRequestType() + public function metricsHeaderCases() { return [ - ['someScope', null, 'auth-request-type/at'], - [null, 'someTargetAudience', 'auth-request-type/it'], + ['foo', '', 'cred-type/foo'], + ['', 'bar', 'auth-request-type/bar'], + ['foo', 'bar', 'auth-request-type/bar cred-type/foo'] ]; } } diff --git a/tests/ObservabilityMetricsTest.php b/tests/ObservabilityMetricsTest.php new file mode 100644 index 000000000..a50a4731a --- /dev/null +++ b/tests/ObservabilityMetricsTest.php @@ -0,0 +1,203 @@ +langAndVersion = sprintf( + 'gl-php/%s auth/%s', + PHP_VERSION, + $updateMetadataTraitImpl::getVersion() + ); + $this->jsonTokens = json_encode(['access_token' => '1/abdef1234567890', 'expires_in' => '57']); + } + + /** + * @dataProvider tokenRequestType + */ + public function testGCECredentials($scope, $targetAudience, $requestTypeHeaderValue) + { + $handlerCalled = false; + $jsonTokens = $this->jsonTokens; + $handler = getHandler([ + new Response(200, [GCECredentials::FLAVOR_HEADER => 'Google']), + function ($request, $options) use ( + $jsonTokens, + &$handlerCalled, + $requestTypeHeaderValue + ) { + $handlerCalled = true; + // This confirms that token endpoint requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s %s cred-type/mds', $this->langAndVersion, $requestTypeHeaderValue), + $request->getHeaderLine(self::$headerKey) + ); + return new Response(200, [], Utils::streamFor($jsonTokens)); + } + ]); + + $gceCred = new GCECredentials(null, $scope, $targetAudience); + $this->assertUpdateMetadata($gceCred, $handler, 'mds', $handlerCalled); + } + + /** + * @dataProvider tokenRequestType + */ + public function testServiceAccountCredentials($scope, $targetAudience, $requestTypeHeaderValue) + { + $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('sa', $requestTypeHeaderValue, $handlerCalled); + + $sa = new ServiceAccountCredentials( + $scope, + $keyFile, + null, + $targetAudience + ); + $this->assertUpdateMetadata($sa, $handler, 'sa', $handlerCalled); + } + + /** + * ServiceAccountJwtAccessCredentials creates the jwt token within library hence + * they don't have any observability metrics header check for token endpoint requests. + */ + public function testServiceAccountJwtAccessCredentials() + { + $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; + $saJwt = new ServiceAccountJwtAccessCredentials($keyFile, 'exampleScope'); + $metadata = $saJwt->updateMetadata([], null, null); + $this->assertArrayHasKey(self::$headerKey, $metadata); + + // This confirms that service usage requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s cred-type/jwt', $this->langAndVersion), + $metadata[self::$headerKey][0] + ); + } + + /** + * ImpersonatedServiceAccountCredentials haven't enabled identity token support hence + * they don't have 'auth-request-type/it' observability metric header check. + */ + public function testImpersonatedServiceAccountCredentials() + { + $keyFile = __DIR__ . '/fixtures5/.config/gcloud/application_default_credentials.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('imp', 'auth-request-type/at', $handlerCalled); + + $impersonatedCred = new ImpersonatedServiceAccountCredentials('exampleScope', $keyFile); + $this->assertUpdateMetadata($impersonatedCred, $handler, 'imp', $handlerCalled); + } + + /** + * UserRefreshCredentials haven't enabled identity token support hence + * they don't have 'auth-request-type/it' observability metric header check. + */ + public function testUserRefreshCredentials() + { + $keyFile = __DIR__ . '/fixtures2/gcloud.json'; + $handlerCalled = false; + $handler = $this->getCustomHandler('u', 'auth-request-type/at', $handlerCalled); + + $userRefreshCred = new UserRefreshCredentials('exampleScope', $keyFile); + $this->assertUpdateMetadata($userRefreshCred, $handler, 'u', $handlerCalled); + } + + /** + * Invokes the 'updateMetadata' method of cred fetcher with empty metadata argument + * and asserts for proper service api usage observability metrics header. + */ + private function assertUpdateMetadata($cred, $handler, $credShortform, &$handlerCalled) + { + $metadata = $cred->updateMetadata([], null, $handler); + $this->assertArrayHasKey(self::$headerKey, $metadata); + + // This confirms that service usage requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s cred-type/%s', $this->langAndVersion, $credShortform), + $metadata[self::$headerKey][0] + ); + + $this->assertTrue($handlerCalled); + } + + /** + * @param string $credShortform The short form of the credential type + * used in observability metric header value. + * @param string $requestTypeHeaderValue Expected header value of the form + * 'auth-request-type/<>' + * @param bool $handlerCalled Reference to the handlerCalled flag asserted later + * in the test. + * @return callable + */ + private function getCustomHandler($credShortform, $requestTypeHeaderValue, &$handlerCalled) + { + $jsonTokens = $this->jsonTokens; + return getHandler([ + function ($request, $options) use ( + $jsonTokens, + &$handlerCalled, + $requestTypeHeaderValue, + $credShortform + ) { + $handlerCalled = true; + // This confirms that token endpoint requests have proper observability metric headers + $this->assertStringContainsString( + sprintf('%s %s cred-type/%s', $this->langAndVersion, $requestTypeHeaderValue, $credShortform), + $request->getHeaderLine(self::$headerKey) + ); + return new Response(200, [], Utils::streamFor($jsonTokens)); + } + ]); + } + + public function tokenRequestType() + { + return [ + ['someScope', null, 'auth-request-type/at'], + [null, 'someTargetAudience', 'auth-request-type/it'], + ]; + } +} From 06b368f51c0194a59393bb6925363fd43d7cecdc Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 4 Jan 2024 06:07:03 +0000 Subject: [PATCH 13/19] Fix the service api usage metrics logic --- src/MetricsTrait.php | 16 +++++++++++----- tests/ObservabilityMetricsTest.php | 8 ++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 2b0c5c599..31414071c 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -72,14 +72,20 @@ protected static function getMetricsHeader( protected function applyServiceApiUsageMetrics($metadata) { if ($credType = $this->getCredType()) { - // Add service api usage observability metrics info to metadata - $metricsHeader = self::getMetricsHeader($credType); + // Add service api usage observability metrics info into metadata + // We expect upstream libries to have the metadata key populated already + $value = 'cred-type/' . $credType; if (!isset($metadata[self::$metricMetadataKey])) { - $metadata[self::$metricMetadataKey] = [$metricsHeader]; + // Unlikely case if upstream google php libraries are using + // this auth library. Still someone can invoke updateMetadata + // directly, that's why just adding the cred-type header. + $metadata[self::$metricMetadataKey] = [$value]; } elseif (is_array($metadata[self::$metricMetadataKey])) { - $metadata[self::$metricMetadataKey][0] .= ' ' . $metricsHeader; + // Append to the existing value. + $metadata[self::$metricMetadataKey][0] .= ' ' . $value; } else { - $metadata[self::$metricMetadataKey] .= ' ' . $metricsHeader; + // Append to the existing value. + $metadata[self::$metricMetadataKey] .= ' ' . $value; } } diff --git a/tests/ObservabilityMetricsTest.php b/tests/ObservabilityMetricsTest.php index a50a4731a..374b6a5d0 100644 --- a/tests/ObservabilityMetricsTest.php +++ b/tests/ObservabilityMetricsTest.php @@ -107,12 +107,12 @@ public function testServiceAccountJwtAccessCredentials() { $keyFile = __DIR__ . '/fixtures3/service_account_credentials.json'; $saJwt = new ServiceAccountJwtAccessCredentials($keyFile, 'exampleScope'); - $metadata = $saJwt->updateMetadata([], null, null); + $metadata = $saJwt->updateMetadata([self::$headerKey => ['foo']], null, null); $this->assertArrayHasKey(self::$headerKey, $metadata); // This confirms that service usage requests have proper observability metric headers $this->assertStringContainsString( - sprintf('%s cred-type/jwt', $this->langAndVersion), + sprintf('foo cred-type/jwt'), $metadata[self::$headerKey][0] ); } @@ -151,12 +151,12 @@ public function testUserRefreshCredentials() */ private function assertUpdateMetadata($cred, $handler, $credShortform, &$handlerCalled) { - $metadata = $cred->updateMetadata([], null, $handler); + $metadata = $cred->updateMetadata([self::$headerKey => ['foo']], null, $handler); $this->assertArrayHasKey(self::$headerKey, $metadata); // This confirms that service usage requests have proper observability metric headers $this->assertStringContainsString( - sprintf('%s cred-type/%s', $this->langAndVersion, $credShortform), + sprintf('foo cred-type/%s', $credShortform), $metadata[self::$headerKey][0] ); From fce1cea16d164ff013efd9aea3e16a15f4bd7f7c Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:30:29 +0000 Subject: [PATCH 14/19] Update documentation --- src/MetricsTrait.php | 7 ++----- src/UpdateMetadataTrait.php | 3 +++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 31414071c..cd08045e2 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -76,15 +76,12 @@ protected function applyServiceApiUsageMetrics($metadata) // We expect upstream libries to have the metadata key populated already $value = 'cred-type/' . $credType; if (!isset($metadata[self::$metricMetadataKey])) { - // Unlikely case if upstream google php libraries are using - // this auth library. Still someone can invoke updateMetadata - // directly, that's why just adding the cred-type header. + // This case will happen only when someone invokes the updateMetadata + // method on the credentials fetcher themselves. $metadata[self::$metricMetadataKey] = [$value]; } elseif (is_array($metadata[self::$metricMetadataKey])) { - // Append to the existing value. $metadata[self::$metricMetadataKey][0] .= ' ' . $value; } else { - // Append to the existing value. $metadata[self::$metricMetadataKey] .= ' ' . $value; } } diff --git a/src/UpdateMetadataTrait.php b/src/UpdateMetadataTrait.php index 2f7592846..30d4060cf 100644 --- a/src/UpdateMetadataTrait.php +++ b/src/UpdateMetadataTrait.php @@ -54,6 +54,9 @@ public function updateMetadata( ) { $metadata_copy = $metadata; + // We do need to set the service api usage metrics irrespective even if + // the auth token is set because invoking this method with auth tokens + // would mean the intention is to just explicitly set the metrics metadata. $metadata_copy = $this->applyServiceApiUsageMetrics($metadata_copy); if (isset($metadata_copy[self::AUTH_METADATA_KEY])) { From 43c525a41a388fc74b77c377dca055c840ac6a10 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:36:53 +0000 Subject: [PATCH 15/19] Remove directory separator usage --- src/MetricsTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index cd08045e2..3b06cb6a5 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -107,7 +107,7 @@ protected function applyTokenEndpointMetrics($metadata, $authRequestType) protected static function getVersion(): string { if (is_null(self::$version)) { - $versionFilePath = implode(DIRECTORY_SEPARATOR, [__DIR__, '..', 'VERSION']); + $versionFilePath = __DIR__ .'/../VERSION'; self::$version = trim((string) file_get_contents($versionFilePath)); } return self::$version; From c70ff2e67a28253dc2e0867bc1715bfe3221fe14 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Fri, 5 Jan 2024 07:02:31 +0000 Subject: [PATCH 16/19] Nit fix --- src/MetricsTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 3b06cb6a5..d2fe58cf2 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -107,7 +107,7 @@ protected function applyTokenEndpointMetrics($metadata, $authRequestType) protected static function getVersion(): string { if (is_null(self::$version)) { - $versionFilePath = __DIR__ .'/../VERSION'; + $versionFilePath = __DIR__ . '/../VERSION'; self::$version = trim((string) file_get_contents($versionFilePath)); } return self::$version; From d4cc704c6701ce69e0b1c816fb5ce9209be3ff23 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 30 Apr 2024 15:44:36 +0000 Subject: [PATCH 17/19] iteration --- src/Credentials/GCECredentials.php | 2 +- src/Credentials/ImpersonatedServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountCredentials.php | 2 +- src/Credentials/ServiceAccountJwtAccessCredentials.php | 2 +- src/Credentials/UserRefreshCredentials.php | 2 +- src/MetricsTrait.php | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index f1879ef00..7ef8f7045 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -632,7 +632,7 @@ public function setIsOnGce($isOnGce) $this->isOnGce = $isOnGce; } - public function getCredType(): string + protected function getCredType(): string { return self::CRED_TYPE; } diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 1222b0fc2..791fe985a 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -146,7 +146,7 @@ public function getLastReceivedToken() return $this->sourceCredentials->getLastReceivedToken(); } - public function getCredType(): string + protected function getCredType(): string { return self::CRED_TYPE; } diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index f20fab6e6..91238029d 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -353,7 +353,7 @@ public function getUniverseDomain(): string return $this->universeDomain; } - public function getCredType(): string + protected function getCredType(): string { return self::CRED_TYPE; } diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 0e456f947..87baa7500 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -217,7 +217,7 @@ public function getQuotaProject() return $this->quotaProject; } - public function getCredType(): string + protected function getCredType(): string { return self::CRED_TYPE; } diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 154df2fea..69778f7c8 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -165,7 +165,7 @@ public function getGrantedScope() return $this->auth->getGrantedScope(); } - public function getCredType(): string + protected function getCredType(): string { return self::CRED_TYPE; } diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index d2fe58cf2..9e15add23 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -113,7 +113,7 @@ protected static function getVersion(): string return self::$version; } - public function getCredType(): string + protected function getCredType(): string { return ''; } From e71c48e65254a592c930571e78f695d7fdf34739 Mon Sep 17 00:00:00 2001 From: yash30201 <54198301+yash30201@users.noreply.github.com> Date: Tue, 30 Apr 2024 16:10:10 +0000 Subject: [PATCH 18/19] Update static reference --- src/MetricsTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 9e15add23..3950f7af5 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -97,7 +97,7 @@ protected function applyServiceApiUsageMetrics($metadata) */ protected function applyTokenEndpointMetrics($metadata, $authRequestType) { - $metricsHeader = $this->getMetricsHeader($this->getCredType(), $authRequestType); + $metricsHeader = self::getMetricsHeader($this->getCredType(), $authRequestType); if (!isset($metadata[self::$metricMetadataKey])) { $metadata[self::$metricMetadataKey] = $metricsHeader; } From 16e0b720cb7ce91a03c864a40151504e2fea6943 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Thu, 2 May 2024 08:06:21 -0700 Subject: [PATCH 19/19] Apply suggestions from code review --- src/MetricsTrait.php | 2 +- tests/MetricsTraitTest.php | 3 +-- tests/ObservabilityMetricsTest.php | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/MetricsTrait.php b/src/MetricsTrait.php index 3950f7af5..8d5c03cf8 100644 --- a/src/MetricsTrait.php +++ b/src/MetricsTrait.php @@ -1,6 +1,6 @@