refactor otlp exporter #838
Conversation
continuing on from Nevay's work, this removes the individual OtlpHttp and OtlpGrpc exporters, and replaces with Otlp\Exporter using a transport to manage grpc and http/protobuf complexity.
verified that the call attributes have the same values when using TraceServiceClient
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #838 +/- ##
============================================
- Coverage 80.90% 80.80% -0.10%
+ Complexity 1973 1951 -22
============================================
Files 246 248 +2
Lines 5173 5122 -51
============================================
- Hits 4185 4139 -46
+ Misses 988 983 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This reverts commit bbb685b. Mocking proto-generated files segfaults with ext-protobuf (which is harder to diagnose when running in a child process, which was required for fancy mockery mocking of "new". So, back to the prototype approach, and skip the tests if ext-protobuf enabled.
|
LGTM sans the conflict. |
src/SDK/Trace/ExporterFactory.php
Outdated
| { | ||
| $factoryClass = self::KNOWN_TRANSPORT_FACTORIES[self::normalizeProtocol($protocol)]; | ||
| self::validateClass($factoryClass); | ||
| $factory = new $factoryClass(); |
There was a problem hiding this comment.
Users should be able to provide custom transport factory implementations.
There was a problem hiding this comment.
I think we would need to invent a new env var for that, something like OTEL_PHP_TRANSPORT_FACTORY_CLASS...
There was a problem hiding this comment.
I'm thinking of a follow-up PR which allows registering exporter and transport factories, and using composer autoload->files to register our initial set.
This reverts commit 350d40c.
Co-authored-by: Tobias Bachert <[email protected]>
…etry-php into refactor-otlp-exporter
|
Great improvement. Thanks for the effort. I want to try it on a small project to see it in a real-world environment. Do you plan to create a new tag (0.0.16) with the changes? |
|
Hi @erdemkose good idea, we'll get one out soon. |
Remove OtlpHttp and OtlpGrpc exporters, and replace with Otlp\SpanExporter and Otlp\MetricExporter using a transport to manage grpc and http/protobuf complexity.
Allow setting signal and protocol on transport factories (particularly for grpc, which uses signal to look up grpc method)
Adding BC for OtlpHttp and OtlpGrpc, which will still fail but with a more helpful message.
Fix failing examples.
Send user-agent header with otlp exports.
Allow otlp http/json