Update user agent construction and deprecate old SdkUserAgent class #5596
Conversation
cenedhryn
force-pushed
the
salande/ua2_0-opt
branch
from
13d24dc
to
fcb336d
Compare
…esting, since client no longer depends on those values to be an empty string
...k-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java
Outdated
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/useragent/SdkClientUserAgentProperties.java
Outdated
Show resolved
Hide resolved
| * Replace any spaces, parentheses in the input with underscores. | ||
| */ | ||
| public static String sanitizeInput(String input) { | ||
| return input == null ? UNKNOWN : input.replaceAll(UA_DENYLIST_REGEX, "_"); |
There was a problem hiding this comment.
Is it worthwile to precompile this into a Pattern?
There was a problem hiding this comment.
The current usage doesn't require it, since it's mainly used in the singleton class, but I'm adding it still.
| * returns the first value. | ||
| */ | ||
| public static String concat(String prefix, String suffix, String separator) { | ||
| return suffix != null && !suffix.isEmpty() ? prefix + separator + suffix : prefix; |
There was a problem hiding this comment.
isEmpty() returns false for whitespace. Is that okay?
There was a problem hiding this comment.
It's fine; it's on a few different call paths but mainly it's legacy code. I don't think there is any case where there would only be a whitespace value - don't think that can happen with system setting values.
| } catch (ClassNotFoundException e) { | ||
| //Ignore | ||
| } catch (Exception e) { | ||
| if (log.isTraceEnabled()) { |
There was a problem hiding this comment.
nit: don't think this is necessary
There was a problem hiding this comment.
Kept previous code, but can remove.
There was a problem hiding this comment.
To clarify, this code is unchanged from before. I want to leave it as is to not risk any issues with errors.
| } catch (ClassNotFoundException e) { | ||
| //Ignore | ||
| } catch (Exception e) { | ||
| if (log.isTraceEnabled()) { |
...k-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java
Outdated
Show resolved
Hide resolved
|
Motivation and Context
New cross-SDK specifications changes the format of the Java user agent header. This change refactors Java SDK to conform to those specifications.
Modifications
Creates a SystemUserAgent and deprecates SdkUserAgent
The existing
software.amazon.awssdk.core.util.SdkUserAgentclass is used for two types of requests:The latter can only use system level information in the user agent, while the former also can include client- and request-level information.
The new
SystemUserAgentstores a general system user agent string to be used in both use cases. The old user agent class is deprecated, because the name is confusing.SystemUserAgenthas public methods to retrieve the individual system properties it supports, in addition to the system user agent string (which is a concatenated value).Minor changes
user.regionand addsuser.country. The JVM doesn't populate user.region anymore, and the lang/region value (likeen_US) has not been added to the user agent string for a while.SdkClientUserAgentProperties.javaas input to building the main part of the SDK user agent string. It is currently a grab bag because at another point in the implementation it was stored in execution attributes. It can be a POJO as well, but the upcoming business metrics refactor also gives ample opportunities to refactor the code again.USER_AGENT_PREFIXandUSER_AGENT_SUFFIX. It's unclear why the implementation was that way.Discussion topics
How to handle old SdkUserAgent
We can
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License