Add detailed client information to User-Agent #25889
Open
+150
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nineinchnick
approved these changes
May 29, 2025
client/trino-client/src/main/java/io/trino/client/SourceBuilder.java
Outdated
Show resolved
Hide resolved
wendigo
force-pushed
the
serafin/cli-source
branch
5 times, most recently
from
0a48c6f to
c32f0dd
Compare
electrum
previously requested changes
May 29, 2025
There was a problem hiding this comment.
The “source” field is user visible and used by resource groups for matching. It’s often set by applications for queuing purposes. It is not intended to be used for client version information.
The standard User-Agent header is appropriate for this purpose. If the intent is to log this in query history, user agent could be included there.
|
@electrum we send source as User-Agent |
|
Source is sent as |
|
It's been that way since the beginning: 33032e7 |
wendigo
force-pushed
the
serafin/cli-source
branch
from
c32f0dd to
91e68bf
Compare
wendigo
changed the title
wendigo
force-pushed
the
serafin/cli-source
branch
from
91e68bf to
9830bbb
Compare
nineinchnick
approved these changes
Jun 2, 2025
losipiuk
reviewed
Jun 3, 2025
| @VisibleForTesting | ||
| static String sanitize(String value) | ||
| { | ||
| return value.replaceAll("[^a-zA-Z0-9_.-]", "_"); |
There was a problem hiding this comment.
nit: colapse mutiple unsane characters into single _
losipiuk
approved these changes
Jun 3, 2025
This sends more information about the client to the Trino cluster. Structure is similiar to other tools like AWS SDK: ``` aws-sdk-java/2.31.42 md/io#sync md/http#Apache ua/2.1 os/Linux#5.10.236-228.935.amzn2.aarch64 lang/java#24.0.1 md/OpenJDK_64-Bit_Server_VM#24.0.1+9 md/vendor#Eclipse_Adoptium md/en_US app/Trino cfg/auth-source#stsweb m/D ``` For our own clients it will be similiar to: ``` trino-cli/1.0.0 os=Mac_OS_X os/version=15.3.1 arch=aarch64 lang/java=24+36 java/vm=OpenJDK_64-Bit_Server_VM java/vendor=Eclipse_Adoptium locale=pl-PL md/lang=en-US md/feature=experimental ```
User-Agent header is already added when HttpClientFactory is used to construct OkHttpClient
wendigo
force-pushed
the
serafin/cli-source
branch
from
04d65f3 to
ee1b720
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This sends more information about the client to the Trino cluster.
Structure is similiar to other tools like AWS SDK:
For our own clients it will be similiar to:
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: