opentelemetry-cpp
opentelemetry-cpp copied to clipboard
OTLP file exporter.
Fixes #2482
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
- [x]
CHANGELOG.mdupdated for non-trivial changes - [x] Unit tests have been added
- [ ] Changes in public API reviewed
This PR is ready to be reviewed now. @open-telemetry/cpp-approvers @perhapsmaple
Hi @owent, thanks for doing this. This is a very nice and well done PR. I have a couple of questions for now:
-
I'm not very familiar with this rolling strategy. Most log tailers like Promtail and Fluentbit recommend the Rename and Create rotation strategy. Has this rotation strategy been tested with any existing log tailer? See here for more information: https://grafana.com/docs/loki/latest/send-data/promtail/logrotation/
-
I know this is probably a bit out of scope of the opentelemetry specification, but is it possible to make a base class like
FileBackendso that users can implement their own rotation strategies if required. TheOtlpFileBackendis very well designed and configurable, but might not fit everyone's use case.
We will have to test it with the otlpjsonfilereceiver from opentelemetry-collector-contrib to make sure that everything works properly. I'm a little busy for the next couple days but I can help you with this after.
Thanks
Hi @owent, thanks for doing this. This is a very nice and well done PR. I have a couple of questions for now:
- I'm not very familiar with this rolling strategy. Most log tailers like Promtail and Fluentbit recommend the Rename and Create rotation strategy. Has this rotation strategy been tested with any existing log tailer? See here for more information: https://grafana.com/docs/loki/latest/send-data/promtail/logrotation/
- I know this is probably a bit out of scope of the opentelemetry specification, but is it possible to make a base class like
FileBackendso that users can implement their own rotation strategies if required. TheOtlpFileBackendis very well designed and configurable, but might not fit everyone's use case.We will have to test it with the
otlpjsonfilereceiverfromopentelemetry-collector-contribto make sure that everything works properly. I'm a little busy for the next couple days but I can help you with this after.Thanks
Thanks.
- I want to keep the default implementation simple. This rotation strategy is tested in our internal collector agent which is also based on otel-collector.
- It's a good idea to leave a base class to let users to use their own log sink, maybe spdlog, boost.log or other components.I will add it later. You can review it anytime when you have time. Thanks.
Need to resolve my 3 comments, and need a second reviewer: adding the please-review label.
Need to resolve my 3 comments, and need a second reviewer: adding the please-review label.
Done
Nice work. Can we document this exporter as an section in README.md ?
Nice work. Can we document this exporter as an section in README.md ?
Sorry for the missing. I will add documents ~~tomorrow~~ several days later. Sorry, I'm too busy recently.I will commit and push documents together with codes if there are more feedbacks about this PR.
Nice work. Can we document this exporter as an section in README.md ?
Sorry for the missing. I will add documents ~tomorrow~ several days later. Sorry, I'm too busy recently.I will commit and push documents together with codes if there are more feedbacks about this PR.
Merging this PR now, please open a PR later for some doc when you have time.
Thanks for this work.