[exporter/awss3] use encoding extensions #31801
Conversation
atoulme
force-pushed
the
add_encoding_s3exporter
branch
from
c92f056
to
417a175
Compare
| marshaler.logsMarshaler, _ = e.(plog.Marshaler) | ||
| marshaler.metricsMarshaler, _ = e.(pmetric.Marshaler) | ||
| marshaler.tracesMarshaler, _ = e.(ptrace.Marshaler) | ||
| marshaler.fileFormat = encoding.String() |
There was a problem hiding this comment.
@atoulme - what's the expected behaviour here, is this intended to be the ID as defined in the config file?
extension:
textencoding: # <- fileFormat == "" ?
textencoding/myspecialtextencoding: # <- fileFormat == "myspecialtextencoding"?There was a problem hiding this comment.
The whole ID: textencoding/myspecialtextencoding
There was a problem hiding this comment.
So you'd end up with an object key of something like:
mytrace/year=2024/month=03/day=18/hour=12/minute=02/myprefixtrace_12345.textencoding/myspecialtextencoding
(where keyprefix=mytrace, prefix=myprefix and metadata=trace)
thats not the prettiest key and there's a possibility that you could have problems with allowed characters or length (unlikely).
Might it be better to do one of the following:
- allow specifying the extension as part of the exporter config
- leave it empty
- modify the encoding extension interface to add a function to return the extension to use.
There was a problem hiding this comment.
Thanks for looking into this. 1 and 2 seem like the right idea to me, with the extension by default being an empty string. Do you want to file an issue to follow up?
Description:
Add encoding extension support to AWS S3 exporter.
Link to tracking Issue:
Fixes #30554
Testing:
Unit tests.
Documentation:
README.