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

feat: UserAgent v2.1 #1690

Merged
merged 9 commits into from
Aug 26, 2024
Merged

feat: UserAgent v2.1 #1690

merged 9 commits into from
Aug 26, 2024

Conversation

sichanyoo
Copy link
Contributor

Issue #

Description of changes

  • Refer to comments on GitHub diff for details

New/existing dependencies impact assessment, if applicable

Conventional Commits

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Sichan Yoo added 8 commits August 20, 2024 11:11
…puted property, utility function for grabbing flags from config / context and saving them into context.businessMetrics.
…tMiddlewrae initialization code, to the runtime UserAgentMiddleware code. This delays user agent struct initialization to right before the request is sent, and is done because we need access to the last-minute values in the context right before the request is sent, in order to grab all features used in request flow and put them in the business metrics section of the user agent.
…efactor fromConfig() initializer wrapper to fromConfigAndContext(), now that business metrics needs values saved in context as well.
… relevant values from config. Add unit tests for BusinessMetrics.
Copy link
Contributor Author

@sichanyoo sichanyoo left a comment

Choose a reason for hiding this comment

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

Comments for reviewers

@@ -15,24 +15,34 @@ public struct UserAgentMiddleware<OperationStackInput, OperationStackOutput> {
private let X_AMZ_USER_AGENT: String = "x-amz-user-agent"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delay when the AWSUserAgentMetadata is initialized, from middleware init to middleware execution. This ensures we have the latest and all values we need from context to populate the user agent header.

let awsUserAgentString = AWSUserAgentMetadata.fromConfigAndContext(
serviceID: serviceID,
version: version,
config: UserAgentValuesFromConfig(config: config),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UserAgentValuesFromConfig is a container for subset of config values in the original config. This makes testing much easier + small benefit of not passing entire config object again.

@@ -6,6 +6,7 @@
//
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replace configMetadata and featureMetadata with the new businessMetrics.

self.frameworkMetadata = frameworkMetadata
}

public static func fromConfig(
public static func fromConfigAndContext(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we need context as well, since business metrics get set in context by middlewares (interceptors).

frameworkMetadata: frameworkMetadata
)
}
}

public class UserAgentValuesFromConfig {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a container for config values that we need for user agent header.

Comment on lines +30 to +34
if commaSeparatedMetricValues.lengthOfBytes(using: .ascii) > 1024 {
while commaSeparatedMetricValues.lengthOfBytes(using: .ascii) > 1024 {
commaSeparatedMetricValues = commaSeparatedMetricValues.substringBeforeLast(",")
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SEP states that the string value of business metrics (comma-separated metric values without spaces) cannot exceed 1024 bytes, and last value in the string must be removed until size requirement is met.

config: UserAgentValuesFromConfig,
context: Context
) {
setFlagsIntoContext(config: config, context: context)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks at values in config and context, and saves flags to context.businessMetrics.

context: Context
) {
setFlagsIntoContext(config: config, context: context)
self.features = context.businessMetrics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Takes the updated context.businessMetrics from line directly above this and sets it to features.

Comment on lines +62 to +84
/* List of readable "feature ID" to "metric value"; last updated on 08/19/2024
[Feature ID] [Metric Value] [Flag Supported]
"RESOURCE_MODEL" : "A" :
"WAITER" : "B" :
"PAGINATOR" : "C" :
"RETRY_MODE_LEGACY" : "D" : Y
"RETRY_MODE_STANDARD" : "E" : Y
"RETRY_MODE_ADAPTIVE" : "F" : Y
"S3_TRANSFER" : "G" :
"S3_CRYPTO_V1N" : "H" :
"S3_CRYPTO_V2" : "I" :
"S3_EXPRESS_BUCKET" : "J" :
"S3_ACCESS_GRANTS" : "K" :
"GZIP_REQUEST_COMPRESSION" : "L" :
"PROTOCOL_RPC_V2_CBOR" : "M" :
"ENDPOINT_OVERRIDE" : "N" : Y
"ACCOUNT_ID_ENDPOINT" : "O" :
"ACCOUNT_ID_MODE_PREFERRED" : "P" :
"ACCOUNT_ID_MODE_DISABLED" : "Q" :
"ACCOUNT_ID_MODE_REQUIRED" : "R" :
"SIGV4A_SIGNING" : "S" : Y
"RESOLVED_ACCOUNT_ID" : "T" :
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to be updated each time we support new flags

Comment on lines +53 to +55
var combined = businessMetrics
combined.merge(newPair) { (current, new) in new }
attributes.set(key: businessMetricsKey, value: combined)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adds a new entry to existing dictionary, overriding with new entry's value if existing dictionary already has an entry with the same key

@sichanyoo sichanyoo requested a review from jbelkins August 21, 2024 19:42
@sichanyoo sichanyoo merged commit ec7cee4 into main Aug 26, 2024
29 checks passed
@sichanyoo sichanyoo deleted the feat/useragent-v2.1-sep branch August 26, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants