Modify user agent header syntax and add support for app ID #2154
Conversation
| "id": "22cf4628-6095-4d35-a4f6-cf1ba88a4ea6", | ||
| "type": "feature", | ||
| "description": "Modify user agent syntax and introduce support for sdk app ID section in UA header", | ||
| "modules": [ |
There was a problem hiding this comment.
fix: This will cause issues with the release, work with Luc or Isaiah to generate a rollup changelog entry.
|
|
||
| // The AppID that could be retrieved from env var or shared config to be | ||
| // added to request's user agent header | ||
| AppID string |
There was a problem hiding this comment.
suggestion: We probably want to indicate this will end up in the user agent, we can also probably link to the relevant documentation from CLI if there is any.
aws/middleware/user_agent_test.go
Outdated
| @@ -15,7 +15,7 @@ import ( | |||
| "github.com/google/go-cmp/cmp/cmpopts" | |||
| ) | |||
|
|
|||
| var expectedAgent = aws.SDKName + "/" + aws.SDKVersion + " os/" + getNormalizedOSName() + " lang/go/" + languageVersion + " md/GOOS/" + runtime.GOOS + " md/GOARCH/" + runtime.GOARCH | |||
| var expectedAgent = aws.SDKName + "/" + aws.SDKVersion + " os/" + getNormalizedOSName() + " lang/go#" + languageVersion + " md/GOOS#" + runtime.GOOS + " md/GOARCH#" + runtime.GOARCH | |||
|
|
|||
| //var expectedSDKAgent = aws.SDKName + "/" + aws.SDKVersion + " os/" + getNormalizedOSName() + " lang/go/" + languageVersion + " md/GOOS/" + runtime.GOOS + " md/GOARCH/" + runtime.GOARCH | |||
aws/middleware/user_agent.go
Outdated
| @@ -74,7 +79,7 @@ type requestUserAgent struct { | |||
| // X-Amz-User-Agent example: | |||
| // | |||
| // aws-sdk-go-v2/1.2.3 md/GOOS/linux md/GOARCH/amd64 lang/go/1.15 | |||
| func newRequestUserAgent() *requestUserAgent { | |||
| func newRequestUserAgent(appID ...string) *requestUserAgent { | |||
There was a problem hiding this comment.
fix: appId is singular, also this is forcing us to know/care about appId when creating the user agent structure.
There is already a SDKAgentKeyType of type ApplicationIdentifier for this purpose. We should revert this back and just make an explicit call in setting up the middleware to add two keys. I'll expand elsewhere.
service/accessanalyzer/api_client.go
Outdated
| func addClientUserAgent(stack *middleware.Stack) error { | ||
| return awsmiddleware.AddSDKAgentKeyValue(awsmiddleware.APIMetadata, "accessanalyzer", goModuleVersion)(stack) | ||
| func addClientUserAgent(stack *middleware.Stack, options Options) error { | ||
| return awsmiddleware.AddSDKAgentKeyValue(awsmiddleware.APIMetadata, "accessanalyzer", goModuleVersion, options.AppID)(stack) |
There was a problem hiding this comment.
Rather than force AppId through this API and abuse variadic arguments we can just call it twice I believe.
e.g.
if err := awsmiddleware.AddSDKAgentKeyValue(awsmiddleware.APIMetadata, "accessanalyzer", goModuleVersion)(stack); err != nil {
return err
}
if options.AppID != nil {
return awsmiddleware.AddSDKAgentKey(awsmiddleware.ApplicationIdentifier, options.AppID)(stack)
}
return nilsync main branch into current branch
aws/config.go
Outdated
| @@ -132,6 +132,13 @@ type Config struct { | |||
| // `config.LoadDefaultConfig`. You should not populate this structure | |||
| // programmatically, or rely on the values here within your applications. | |||
| RuntimeEnvironment RuntimeEnvironment | |||
|
|
|||
| // The AppID could be retrieved from environment variable AWS_SDK_UA_APP_ID | |||
There was a problem hiding this comment.
AppId is an optional application specific identifier that can be set. When set it will be appended to the User-Agent header of every request in the form of
App/{AppId}. This variable is sourced from environment variableAWS_SDK_UA_APP_IDor the shared config profile attributesdk_ua_app_id. See https://docs.aws.amazon.com/sdkref/latest/guide/settings-reference.html for more information on environment variables and shared config settings.
| AwsConfigField.builder() | ||
| .name(SDK_APP_ID) | ||
| .type(getUniversalSymbol("string")) | ||
| .documentation("The aws sdk app ID that could be retrieved from env var or shared config file" + |
There was a problem hiding this comment.
This doesn't match the current generated setting:
The sdk user agent app id to be added to http request's User-Agent header
AppID string
Maybe need to regenerate?
suggestion:
The (optional) application specific identifier appended to the User-Agent header.
Modify http request User-Agent header syntax and add support for optional app ID section in the UA header.
Customer code could offer an App ID for identifying their applications, which can be obtained from following sources in descending priority:
Once a service operation is called with app ID set, it will be concatenated to UA header string in format
{app/appID}. One example UA string calling s3 service is shown here:User-Agent: aws-sdk-go-v2/1.18.1 os/macos lang/go#1.19.5 md/GOOS#darwin md/GOARCH#amd64 api/s3#1.36.0 app/123