[exporter/awss3] S3 Exporter #20912
[exporter/awss3] S3 Exporter #20912
Conversation
|
@atoulme LGTM. I think it would be good to do some e2e tests, however I don't want to block this PR. Let me know what do you think. |
|
May I know when this PR will be merged to the main? |
|
@pdelewski can you create an issue for e2e tests please? We can discuss it separately. |
Yes, sure |
|
Rebased off latest main. |
atoulme
added
the
ready to merge
|
@pdelewski I talked about this PR with @dmitryax and he asked me if we had any way to read otlp_proto data back. If we cannot, should we consider removing it for now while we figure a plan to use it? |
codeboten
removed
the
ready to merge
|
@atoulme i removed the ready to merge label until the pdata question is resolved |
@atoulme It's not clear why do we need to read otlp_proto data back? |
|
@pdelewski well, if you store that data in S3, do you want a way to make sense of it? How will this data be read after? It's ok if the collector is not involved, just looking for why. It's likely that if we offer this option users will store as proto data and come back to ask us what to do with it next. At least with JSON we have a couple ways to make sense of it. |
|
@atoulme ok, so it's about semantic meaning (schema). That's something I have not taken into account, yet. |
|
@pdelewski Can we remove it for now while we figure this out? |
|
No - I'll make a code change to show you, hold on. |
|
@pdelewski I have removed the binary protobuf marshaler, see 43ac4c9 |
Description:
This is a reprisal of #10000, rebased, and modified to match best practices. It also adds support for metrics.
Link to tracking Issue:
Fixes #2835
Testing:
Unit tests.
Documentation:
README