Skip to content
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

feat: partition format #37954

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

kevthompson
Copy link

@kevthompson kevthompson commented Feb 15, 2025

Description

Replacing the limited s3_partition option with a dynamic s3_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 on hourly 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

  • when no value is given it matches the current default => s3://test-otel-s3-exporter/year=2025/month=02/day=23/hour=22/minute=34/logs_235895800.json
  • custom path '%Y/%m/%d/%H/%M' => s3://test-otel-s3-exporter/2025/02/23/22/34/logs_216971670.json
  • custom path with prefix => s3://test-otel-s3-exporter/baz/2025/02/23/22/33/logs_134704167.json

Documentation

Updated README with new s3_partition_format input & removed the s3_partition entry. Added an extra example config with a custom partition format.

Copy link
Contributor

@atoulme atoulme left a 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.

@kevthompson
Copy link
Author

Validated against my personal S3 bucket & fixed the link the workflows were complaining about, all working as expected

@atoulme atoulme added ready to merge Code review completed; ready to merge by maintainers and removed ready to merge Code review completed; ready to merge by maintainers labels Feb 26, 2025
@atoulme
Copy link
Contributor

atoulme commented Feb 26, 2025

CI fails:

go.mod/go.sum deps changes detected, please run "make gotidy" and commit the changes in this PR.

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Feb 28, 2025
@atoulme
Copy link
Contributor

atoulme commented Feb 28, 2025

I am kicking the build, please hold off merging main, I have to kick the build off every time you do :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/awss3 ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Request) [AWS S3 Exporter] Custom File Paths [BUG][exporter/awss3] Newer versions path has extra "/" prefix
3 participants