OTLPLogExporter example to not use OTLP-Pipeline builders #1811
Conversation
| @@ -55,15 +56,15 @@ fn init_metrics() -> Result<opentelemetry_sdk::metrics::SdkMeterProvider, Metric | |||
| } | |||
|
|
|||
| fn init_logs() -> Result<opentelemetry_sdk::logs::LoggerProvider, LogError> { | |||
| opentelemetry_otlp::new_pipeline() | |||
| .logging() | |||
| let exporter = opentelemetry_otlp::new_exporter() | |||
There was a problem hiding this comment.
this does work, and is consistent with other exporter like stdout. However, the build_log_exporter() is called on something which is not advertised as a builder, which is weird!
There was a problem hiding this comment.
I don't want to merge this, without fixing the awkward API. Most likely this is something we'll have to address after the initial beta release.
There was a problem hiding this comment.
new_exporter comment reads:
/// Create a builder to build OTLP metrics exporter or tracing exporter.
We can just rename the struct/function if it feels werid. But I think conceptually it's a builder
There was a problem hiding this comment.
Actually, build_log_exporter() is not called on the exporter, but instead called on the TonicExporterBuilder. This makes more sense, and the name "TonicExporterBuilder" indicates it is a builder.. Even then, "build()
is more apt than "build_log_exporter", but this requires more refactoring to entire pipeline(s) structs in the OTLPExporter.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1811 +/- ##
=====================================
Coverage 73.7% 73.7%
=====================================
Files 123 123
Lines 19634 19634
=====================================
+ Hits 14481 14482 +1
+ Misses 5153 5152 -1 ☔ View full report in Codecov by Sentry. |
Opening a PR to show some issues related to : #1810