-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix awss3reader prefix trim #43587
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
Fix awss3reader prefix trim #43587
Conversation
|
|
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
codeboten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @anhtaw! please ensure a changelog entry is added https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-a-changelog-entry
Hi @codeboten Changelog entry has been added. Thanks for the reminder! |
adcharre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me and seems to follow similar behaviour as to the S3 Exporter.
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
|
Thank you for your contribution @anhtaw! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. If you are getting started contributing, you can also join the CNCF Slack channel #opentelemetry-new-contributors to ask for guidance and get help. |
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixing a bug in the `awss3receiver` key generation logic where object keys could contain an unexpected leading slash (`/year=...`) when `s3_prefix` was empty and `s3_partition_format` was not explicitly set. This change makes the behavior consistent between default and custom `s3_partition_format` by ensuring keys do not start with a slash unless explicitly configured. Additional logic was added to handle cases where `prefix` equals `/` or `//`, preserving the user-defined format without stripping or duplicating slashes. --- <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#43522 --- <!--Describe what testing was performed and which tests were added.--> #### Testing - Added new unit tests in `s3reader_test.go` covering: - Default `s3_partition_format` behavior with empty prefix. - Explicit `s3_partition_format` override. - Custom prefix values `/` and `//`. - Verified generated S3 keys match expected format: - `year=2025/month=10/day=15/...` (no leading slash) - `/year=2025/...` when prefix = `/`. - All existing tests pass (`make test`). --- <!--Describe the documentation added.--> #### Documentation No external documentation changes required. Internal behavior change documented in code comments for `getObjectPrefixForTime()`.
Description
Fixing a bug in the
awss3receiverkey generation logic where object keys could containan unexpected leading slash (
/year=...) whens3_prefixwas empty ands3_partition_formatwas not explicitly set.
This change makes the behavior consistent between default and custom
s3_partition_formatby ensuring keys do not start with a slash unless explicitly configured.
Additional logic was added to handle cases where
prefixequals/or//, preserving theuser-defined format without stripping or duplicating slashes.
Link to tracking issue
Fixes #43522
Testing
s3reader_test.gocovering:s3_partition_formatbehavior with empty prefix.s3_partition_formatoverride./and//.year=2025/month=10/day=15/...(no leading slash)/year=2025/...when prefix =/.make test).Documentation
No external documentation changes required.
Internal behavior change documented in code comments for
getObjectPrefixForTime().