-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[exporter/awss3] feat!: partition format #37954
Conversation
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.
Changes make sense, it's an alpha exporter, maybe we can afford this breaking change.
Validated against my personal S3 bucket & fixed the link the workflows were complaining about, all working as expected |
CI fails:
|
I am kicking the build, please hold off merging main, I have to kick the build off every time you do :) |
// Valid values are: [hour,minute] | ||
S3Partition string `mapstructure:"s3_partition"` | ||
// S3PartitionFormat is used to provide the rollup on how data is written. Uses [strftime](https://www.man7.org/linux/man-pages/man3/strftime.3.html) formatting. | ||
S3PartitionFormat string `mapstructure:"s3_partition_format"` |
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.
It looks like this change could be done in a less disruptive manner for users, by deprecating the s3_partition
field first and removing it in a future version.
The component is in alpha
, so I suppose this is acceptable.
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.
I see now that there is also a receiver awss3receiver
with the same s3_partition:
input, do these need to be kept in line?
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.
it's alpha
and I think we need time to get things right. Yes, we might set the same approach for the s3 receiver.
Some conflicts to resolve @kevthompson |
Description
Replacing the limited
s3_partition
option with a dynamics3_partition_format
option to allow custom filenames. Allows users to define their own partition formats (e.g. removing the "year=" strings or partitioning weekly).This is a breaking change, I created this as a new input and removed the
s3_partition
so anyone that was onhourly
partitions won't accidentally upgrade and change their file formats. The default format still points to the same location.Also fixed a bug where not specifying a prefix would create an empty directory in the bucket.
Link to tracking issue
Fixes #37915
Fixes #37503
Testing
Test cases all passing
Tested against personal S3 bucket
s3://test-otel-s3-exporter/year=2025/month=02/day=23/hour=22/minute=34/logs_235895800.json
'%Y/%m/%d/%H/%M'
=>s3://test-otel-s3-exporter/2025/02/23/22/34/logs_216971670.json
s3://test-otel-s3-exporter/baz/2025/02/23/22/33/logs_134704167.json
Documentation
Updated README with new
s3_partition_format
input & removed thes3_partition
entry. Added an extra example config with a custom partition format.