Skip to content

feat: add Diffgram user agent to S3 storage clients - #1634

Draft
goanpeca wants to merge 1 commit into
diffgram:masterfrom
goanpeca:s3-client-user-agent
Draft

feat: add Diffgram user agent to S3 storage clients#1634
goanpeca wants to merge 1 commit into
diffgram:masterfrom
goanpeca:s3-client-user-agent

Conversation

@goanpeca

Copy link
Copy Markdown

Description

Related issue: #1633

boto3 clients built by DataToolsS3 and DataToolsMinio send the stock botocore user agent, so Diffgram traffic is indistinguishable from any other boto3 caller in bucket access logs and storage side metrics. This appends a diffgram/<version> token to the existing agent string using Config.user_agent_extra, which is the supported botocore way to add to the user agent rather than replace it.

The version is read from the existing DIFFGRAM_VERSION_TAG setting and falls back to dev when it is not set, so there is no new configuration to manage and the module level constant cannot raise on import. Config(user_agent_extra=...).merge(config or Config()) keeps every caller supplied option, so the signature_version='s3v4' config used by IS_DIFFGRAM_S3_V4_SIGNATURE and by the MinIO connector still applies.

Both S3 client factories are covered: the AWS one in shared/data_tools_core_s3.py and the endpoint_url one in shared/data_tools_core_minio.py, which is also the path used for other S3 compatible endpoints.

Resulting agent string, for example: Boto3/1.18.1 md/Botocore#1.21.1 ua/2.0 os/linux lang/python#3.11.1 Botocore/1.21.1 diffgram/dev.

No new settings, no new dependencies, and no change to endpoints, signing, verification, or request behavior.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.

I have...

  • added ! to the type prefix if Breaking Changes. (not applicable, this only adds a token to an outgoing header)
  • considered the impact to the SDK. (none, this is server side client construction only)
  • included the correct type prefix in the PR title
  • provided a link to the relevant issue in JIRA or Github Link in PR. (not applicable, no tracking issue)
  • included the unit and integration tests (not applicable, there are no existing tests around these two factories. Verified locally that the token is appended to the default agent rather than replacing it, and that a caller supplied signature_version survives the merge)
  • included comments & docs for new API Endpoints (not applicable, no API endpoints)
  • updated the relevant documentation or specification in readme.io (not applicable, no user facing configuration change)
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed (will confirm once CI runs here)

Breaking Changes including adding required params: none.

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the Author checklist was followed
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

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.

1 participant