feat: user agent v2.1 second revision

- Move the user agent middleware after the signing step in order to gather signature metrics. - Add request compression metric gathering. - Add specific testing for signatures. - Add specific testing for request compression. - Add specific testing for s3 encryption clients.
aws · Nov 5, 2024 · 2ee9c21 · 2ee9c21
1 parent e3936e8
commit 2ee9c21
Show file tree

Hide file tree

Showing 7 changed files with 452 additions and 74 deletions.
diff --git a/src/AwsClient.php b/src/AwsClient.php
@@ -280,10 +280,6 @@ public function __construct(array $args)
         if (isset($args['with_resolved'])) {
             $args['with_resolved']($config);
         }
-        MetricsBuilder::appendMetricsCaptureMiddleware(
-            $this->getHandlerList(),
-            MetricsBuilder::RESOURCE_MODEL
-        );
         $this->addUserAgentMiddleware($config);
     }
 
@@ -453,7 +449,7 @@ private function addSignatureMiddleware(array $args)
         }
 
         $resolver = static function (
-            CommandInterface $c
+            CommandInterface $command
         ) use (
                 $api,
                 $provider,
@@ -465,17 +461,17 @@ private function addSignatureMiddleware(array $args)
             $handlerList
         ) {
             if (!$configuredSignatureVersion) {
-                if (!empty($c['@context']['signing_region'])) {
-                    $region = $c['@context']['signing_region'];
+                if (!empty($command['@context']['signing_region'])) {
+                    $region = $command['@context']['signing_region'];
                 }
-                if (!empty($c['@context']['signing_service'])) {
-                    $name = $c['@context']['signing_service'];
+                if (!empty($command['@context']['signing_service'])) {
+                    $name = $command['@context']['signing_service'];
                 }
-                if (!empty($c['@context']['signature_version'])) {
-                    $signatureVersion = $c['@context']['signature_version'];
+                if (!empty($command['@context']['signature_version'])) {
+                    $signatureVersion = $command['@context']['signature_version'];
                 }
 
-                $authType = $api->getOperation($c->getName())['authtype'];
+                $authType = $api->getOperation($command->getName())['authtype'];
                 switch ($authType){
                     case 'none':
                         $signatureVersion = 'anonymous';
@@ -490,20 +486,21 @@ private function addSignatureMiddleware(array $args)
             }
 
             if ($signatureVersion === 'v4a') {
-                $commandSigningRegionSet = !empty($c['@context']['signing_region_set'])
-                    ? implode(', ', $c['@context']['signing_region_set'])
+                $commandSigningRegionSet = !empty($command['@context']['signing_region_set'])
+                    ? implode(', ', $command['@context']['signing_region_set'])
                     : null;
 
                 $region = $signingRegionSet
                     ?? $commandSigningRegionSet
                     ?? $region;
-
-                MetricsBuilder::appendMetricsCaptureMiddleware(
-                    $handlerList,
-                    MetricsBuilder::SIGV4A_SIGNING
-                );
             }
 
+            // Capture signature metric
+            $command->getMetricsBuilder()->appendByIdentifyingFeature(
+                'signature',
+                $signatureVersion
+            );
+
             return SignatureProvider::resolve($provider, $signatureVersion, $name, $region);
         };
         $this->handlerList->appendSign(
@@ -607,9 +604,19 @@ private function addEndpointV2Middleware()
         );
     }
 
+    /**
+     * Appends the user agent middleware.
+     * This middleware MUST be appended after the
+     * signature middleware `addSignatureMiddleware`,
+     * so that metrics around signatures are properly
+     * captured.
+     *
+     * @param $args
+     * @return void
+     */
     private function addUserAgentMiddleware($args)
     {
-        $this->getHandlerList()->prependSign(
+        $this->getHandlerList()->appendSign(
             UserAgentMiddleware::wrap($args),
             'user-agent'
         );

diff --git a/src/EndpointV2/EndpointV2Middleware.php b/src/EndpointV2/EndpointV2Middleware.php
@@ -99,14 +99,14 @@ public function __invoke(CommandInterface $command)
         $operation = $this->api->getOperation($command->getName());
         $commandArgs = $command->toArray();
         $providerArgs = $this->resolveArgs($commandArgs, $operation);
-        $this->hookAccountIdMetric(
-            $providerArgs[self::ACCOUNT_ID_PARAM] ?? null,
-            $command
+        $command->getMetricsBuilder()->appendByIdentifyingFeature(
+            'account_id',
+            $providerArgs[self::ACCOUNT_ID_PARAM] ?? null
         );
         $endpoint = $this->endpointProvider->resolveEndpoint($providerArgs);
-        $this->hookAccountIdEndpointMetric(
-            $endpoint,
-            $command
+        $command->getMetricsBuilder()->appendByIdentifyingFeature(
+            'account_id_endpoint',
+            $endpoint->getUrl()
         );
         if (!empty($authSchemes = $endpoint->getProperty('authSchemes'))) {
             $this->applyAuthScheme(
@@ -402,23 +402,4 @@ private function resolveAccountId(): ?string
 
         return $identity->getAccountId();
     }
-
-    private function hookAccountIdMetric($accountId, &$command)
-    {
-        if (!empty($accountId)) {
-            MetricsBuilder::fromCommand($command)->append(
-                MetricsBuilder::RESOLVED_ACCOUNT_ID
-            );
-        }
-    }
-
-    private function hookAccountIdEndpointMetric($endpoint, $command)
-    {
-        $regex = "/^(https?:\/\/\d{12}\.[^\s\/$.?#].\S*)$/";
-        if (preg_match($regex, $endpoint->getUrl())) {
-            MetricsBuilder::fromCommand($command)->append(
-                MetricsBuilder::ACCOUNT_ID_ENDPOINT
-            );
-        }
-    }
 }
diff --git a/src/MetricsBuilder.php b/src/MetricsBuilder.php
@@ -7,7 +7,6 @@
  */
 final class MetricsBuilder
 {
-    const COMMAND_METRICS_BUILDER = "CommandMetricsBuilder";
     const RESOURCE_MODEL = "A";
     const WAITER = "B";
     const PAGINATOR = "C";
@@ -17,6 +16,8 @@ final class MetricsBuilder
     const S3_TRANSFER = "G";
     const S3_CRYPTO_V1N = "H";
     const S3_CRYPTO_V2 = "I";
+    const S3_EXPRESS_BUCKET = "J";
+    const GZIP_REQUEST_COMPRESSION = "L";
     const ENDPOINT_OVERRIDE = "N";
     const ACCOUNT_ID_ENDPOINT = "O";
     const ACCOUNT_ID_MODE_PREFERRED = "P";
@@ -96,6 +97,34 @@ public function append(string $metric): void
         $this->metricsSize += strlen($metric) + strlen(self::$METRIC_SEPARATOR);
     }
 
+    public function appendByIdentifyingFeature($featureGroup, $value)
+    {
+        if ($featureGroup === 'signature') {
+            if ($value === 'v4-s3express') {
+                $this->append(MetricsBuilder::S3_EXPRESS_BUCKET);
+            } elseif ($value === 'v4a') {
+                $this->append(MetricsBuilder::SIGV4A_SIGNING);
+            }
+        } elseif ($featureGroup === 'request_compression') {
+            if ($value === 'gzip') {
+                $this->append(MetricsBuilder::GZIP_REQUEST_COMPRESSION);
+            }
+        } elseif ($featureGroup === 'account_id') {
+            if (!empty($value)) {
+                $this->append(MetricsBuilder::RESOLVED_ACCOUNT_ID);
+            }
+        } elseif ($featureGroup === 'account_id_endpoint') {
+            if (empty($value)) {
+                return;
+            }
+
+            $regex = "/^(https?:\/\/\d{12}\.[^\s\/$.?#].\S*)$/";
+            if (preg_match($regex, $value)) {
+                $this->append(MetricsBuilder::ACCOUNT_ID_ENDPOINT);
+            }
+        }
+    }
+
     /**
      * Validates if a metric can be appended by verifying if the current
      * metrics size plus the new metric plus the length of the separator

diff --git a/src/RequestCompressionMiddleware.php b/src/RequestCompressionMiddleware.php
@@ -67,6 +67,12 @@ public function __invoke(CommandInterface $command, RequestInterface $request)
         $this->encodings = $compressionInfo['encodings'];
         $request = $this->compressRequestBody($request);
 
+        // Capture request compression metric
+        $command->getMetricsBuilder()->appendByIdentifyingFeature(
+            'request_compression',
+            $request->getHeaderLine('content-encoding')
+        );
+
         return $nextHandler($command, $request);
     }
 
@@ -161,4 +167,4 @@ private function isValidCompressionSize($compressionSize)
             . 'non-negative integer value between 0 and 10485760 bytes, inclusive.'
         );
     }
-}
+}
diff --git a/src/UserAgentMiddleware.php b/src/UserAgentMiddleware.php
@@ -6,6 +6,12 @@
 use Closure;
 use Psr\Http\Message\RequestInterface;
 
+/**
+ * Builds and injects the user agent header values.
+ * This middleware must be appended into step where all the
+ * metrics to be gathered are already resolved. As of now it should be
+ * after the signing step.
+ */
 class UserAgentMiddleware
 {
     const AGENT_VERSION = 2.1;

diff --git a/tests/AwsClientTest.php b/tests/AwsClientTest.php
@@ -986,25 +986,6 @@ public function testClientParameterOverridesDefaultAccountIdEndpointModeBuiltIns
         self::assertEquals($expectedAccountIdEndpointMode, $builtIns['AWS::Auth::AccountIdEndpointMode']);
     }
 
-    public function testAppendsC2jMetricsCaptureMiddleware()
-    {
-        $client = new S3Client([
-            'region' => 'us-east-2',
-            'http_handler' => function (RequestInterface $request) {
-                $this->assertTrue(
-                    in_array(
-                        MetricsBuilder::RESOURCE_MODEL,
-                        $this->getMetricsAsArray($request)
-                    )
-                );
-
-                return new Response();
-            }
-        ]);
-
-        $client->listBuckets();
-    }
-
     public function testAppendsUserAgentMiddleware()
     {
         $client = new S3Client([