export opentelemetry spans with batch processor #6733
Conversation
| @@ -48,6 +48,9 @@ namespace otel_resource = opentelemetry::sdk::resource; | |||
|
|
|||
| namespace triton { namespace server { | |||
|
|
|||
| static const int EXPORT_QUEUE_SIZE = 5000; | |||
There was a problem hiding this comment.
Does it make sense to make these options configurable through cmdline args?
| @@ -40,6 +40,8 @@ | |||
| #include "opentelemetry/sdk/resource/resource.h" | |||
| #include "opentelemetry/sdk/trace/processor.h" | |||
| #include "opentelemetry/sdk/trace/simple_processor_factory.h" | |||
There was a problem hiding this comment.
If we are removing SimpleProcessFactory, then it makes sense to remove this input
There was a problem hiding this comment.
Hi @theoclark , thank you for this PR! Left some comments. May I also ask you to write some tests for your code? Open telemetry tests are located here: https://github.com/triton-inference-server/server/tree/main/qa/L0_trace. Could you please also send us a signed CLA?
Thanks for the review, will make some changes. I work for Speechmatics and I believe we've already signed the CLA? Any specific tests you had in mind? |
Got it, thanks for letting me know about CLA. Regarding tests. Basically, it would be nice to add the following tests:
|
|
Hi @theoclark , I've pushed a PR, which has a major refactor for tests: #6785 If you are interested, I can pick up this PR as well and add tests myself. |
|
Hi @oandreeva-nv, thanks that would be great. Hadn't got around to writing the tests myself. |
|
@oandreeva-nv we've also noticed in other parts of our code that exporting traces across HTTP can have more of an impact on performance than gRPC. Is there a reason tritonserver uses HTTP over gRPC that I'm not aware of? |
|
@theoclark There was a limited scope of what we could implement initially. gRPC is on our roadmap for implementation. I'll try to make it happen in 24.02 release, since there is one more ask for gRPC exporter |
|
Hi @theoclark , I'll close this PR since #6842 was merged, and it includes your commits from this PR and mentions you as a co-author |
|
Thanks @oandreeva-nv for your work on this. Great that it's been merged. |
We have observed slowdowns of up to 50% when running tritonserver with opentelemetry. We have traced this to the SimpleSpan Exporter and have observed these slowdowns go away when switching to the BatchSpanProcessor.