Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhancement: user agent 2.1 #3001

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/AwsClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ public function __construct(array $args)
if (isset($args['with_resolved'])) {
$args['with_resolved']($config);
}
MetricsBuilder::appendMetricsCaptureMiddleware(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: do we need to add this (or other static/guaranteed metrics) to our middleware stack?

$this->getHandlerList(),
MetricsBuilder::RESOURCE_MODEL
);
$this->addUserAgentMiddleware($config);
}

public function getHandlerList()
Expand Down Expand Up @@ -437,6 +442,7 @@ private function addSignatureMiddleware(array $args)
$name = $this->config['signing_name'];
$region = $this->config['signing_region'];
$signingRegionSet = $this->signingRegionSet;
$handlerList = $this->getHandlerList();

if (isset($args['signature_version'])
|| isset($this->config['configured_signature_version'])
Expand All @@ -455,7 +461,8 @@ private function addSignatureMiddleware(array $args)
$region,
$signatureVersion,
$configuredSignatureVersion,
$signingRegionSet
$signingRegionSet,
$handlerList
) {
if (!$configuredSignatureVersion) {
if (!empty($c['@context']['signing_region'])) {
Expand Down Expand Up @@ -490,6 +497,11 @@ private function addSignatureMiddleware(array $args)
$region = $signingRegionSet
?? $commandSigningRegionSet
?? $region;

MetricsBuilder::appendMetricsCaptureMiddleware(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: This function is invoked during the sign step. How is the signature version being passed to the metrics builder by appending middleware to the build step, which has already completed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Metrics here will not be collected. I will do a refactor for this type of cases.

$handlerList,
MetricsBuilder::SIGV4A_SIGNING
);
}

return SignatureProvider::resolve($provider, $signatureVersion, $name, $region);
Expand Down Expand Up @@ -595,6 +607,14 @@ private function addEndpointV2Middleware()
);
}

private function addUserAgentMiddleware($args)
{
$this->getHandlerList()->prependSign(
UserAgentMiddleware::wrap($args),
'user-agent'
);
}

/**
* Retrieves client context param definition from service model,
* creates mapping of client context param names with client-provided
Expand Down
84 changes: 1 addition & 83 deletions src/ClientResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -980,66 +980,8 @@ public static function _default_app_id(array $args)

public static function _apply_user_agent($inputUserAgent, array &$args, HandlerList $list)
{
// Add SDK version
$userAgent = ['aws-sdk-php/' . Sdk::VERSION];

// User Agent Metadata
$userAgent[] = 'ua/2.0';

// If on HHVM add the HHVM version
if (defined('HHVM_VERSION')) {
$userAgent []= 'HHVM/' . HHVM_VERSION;
}

// Add OS version
$disabledFunctions = explode(',', ini_get('disable_functions'));
if (function_exists('php_uname')
&& !in_array('php_uname', $disabledFunctions, true)
) {
$osName = "OS/" . php_uname('s') . '#' . php_uname('r');
if (!empty($osName)) {
$userAgent []= $osName;
}
}

// Add the language version
$userAgent []= 'lang/php#' . phpversion();

// Add exec environment if present
if ($executionEnvironment = getenv('AWS_EXECUTION_ENV')) {
$userAgent []= $executionEnvironment;
}

// Add endpoint discovery if set
if (isset($args['endpoint_discovery'])) {
if (($args['endpoint_discovery'] instanceof \Aws\EndpointDiscovery\Configuration
&& $args['endpoint_discovery']->isEnabled())
) {
$userAgent []= 'cfg/endpoint-discovery';
} elseif (is_array($args['endpoint_discovery'])
&& isset($args['endpoint_discovery']['enabled'])
&& $args['endpoint_discovery']['enabled']
) {
$userAgent []= 'cfg/endpoint-discovery';
}
}

// Add retry mode if set
if (isset($args['retries'])) {
if ($args['retries'] instanceof \Aws\Retry\Configuration) {
$userAgent []= 'cfg/retry-mode#' . $args["retries"]->getMode();
} elseif (is_array($args['retries'])
&& isset($args["retries"]["mode"])
) {
$userAgent []= 'cfg/retry-mode#' . $args["retries"]["mode"];
}
}

// AppID Metadata
if (!empty($args['app_id'])) {
$userAgent[] = 'app/' . $args['app_id'];
}

$userAgent = [];
// Add the input to the end
if ($inputUserAgent){
if (!is_array($inputUserAgent)) {
Expand All @@ -1050,30 +992,6 @@ public static function _apply_user_agent($inputUserAgent, array &$args, HandlerL
}

$args['ua_append'] = $userAgent;

$list->appendBuild(static function (callable $handler) use ($userAgent) {
return function (
CommandInterface $command,
RequestInterface $request
) use ($handler, $userAgent) {
return $handler(
$command,
$request->withHeader(
'X-Amz-User-Agent',
implode(' ', array_merge(
$userAgent,
$request->getHeader('X-Amz-User-Agent')
))
)->withHeader(
'User-Agent',
implode(' ', array_merge(
$userAgent,
$request->getHeader('User-Agent')
))
)
);
};
});
}

public static function _apply_endpoint($value, array &$args, HandlerList $list)
Expand Down
17 changes: 16 additions & 1 deletion src/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ class Command implements CommandInterface
/** @var HandlerList */
private $handlerList;

/** @var Array */
/** @var array */
private $authSchemes;

/** @var MetricsBuilder */
private $metricsBuilder;

/**
* Accepts an associative array of command options, including:
*
Expand All @@ -38,6 +41,7 @@ public function __construct($name, array $args = [], HandlerList $list = null)
if (!isset($this->data['@context'])) {
$this->data['@context'] = [];
}
$this->metricsBuilder = new MetricsBuilder();
}

public function __clone()
Expand Down Expand Up @@ -110,4 +114,15 @@ public function get($name)
{
return $this[$name];
}

/**
* @inheridoc
* @internal
*
* @return MetricsBuilder
*/
public function getMetricsBuilder(): MetricsBuilder
{
return $this->metricsBuilder;
}
}
7 changes: 7 additions & 0 deletions src/CommandInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,11 @@ public function hasParam($name);
* @return HandlerList
*/
public function getHandlerList();

/**
* Returns the metrics builder instance tied up to this command.
*
* @return MetricsBuilder
*/
public function getMetricsBuilder();
}
29 changes: 28 additions & 1 deletion src/EndpointV2/EndpointV2Middleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Aws\Api\Service;
use Aws\Auth\Exception\UnresolvedAuthSchemeException;
use Aws\CommandInterface;
use Aws\MetricsBuilder;
use Closure;
use GuzzleHttp\Promise\Promise;
use function JmesPath\search;
Expand Down Expand Up @@ -98,8 +99,15 @@ 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
);
$endpoint = $this->endpointProvider->resolveEndpoint($providerArgs);

$this->hookAccountIdEndpointMetric(
$endpoint,
$command
);
if (!empty($authSchemes = $endpoint->getProperty('authSchemes'))) {
$this->applyAuthScheme(
$authSchemes,
Expand Down Expand Up @@ -394,4 +402,23 @@ private function resolveAccountId(): ?string

return $identity->getAccountId();
}

private function hookAccountIdMetric($accountId, &$command)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: can we make a generic method on the MetricsBuilder class that will handle this and other things like it i.e. accountidendpointmetric?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can take a look and see if it fits better there.

{
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
);
}
}
}
Loading