Problem
Currently, the OTLP exporter is passed Recordable objects at export time. The span protobuf in the Recordable needs to be moved into the request that will be exported (see otlp_exporter.cc).
Although we use std::move to move the span protobuf from the Recordable to the request, benchmark tests indicate that the protobuf is likely being copied, not moved. This is due to the way std::move handles const lvalue references (see this link for more information).
To see the difference between move and copy, I did the following test:
Current code in otlp_exporter.cc (probably copying, even though we're using std::move):
*instrumentation_lib->add_spans() = std::move(rec->span());
Current benchmarks:
Modified code (swap, effectively moving):
instrumentation_lib->add_spans()->Swap(
const_cast<opentelemetry::proto::trace::v1::Span *>(&rec->span()));
Modified benchmarks:
We can see that the benchmarks are roughly half for the modified code (500 ns vs. 1000 ns for dense spans).
Solution
The modified code above is unsafe, since it uses a const_cast.
Another solution is to modify the Recordable class to store a pointer to a span protobuf, instead of a span protobuf object.
The exporter could maintain a collection of the actual span protobufs. When it becomes time to export, the exporter could associate each given Recordable with its span protobuf using the span id. Storing span protobufs in the exporter, rather than in Recordable, would likely allow the protobufs to be moved into the requests, rather than copied.
Problem
Currently, the OTLP exporter is passed
Recordableobjects at export time. The span protobuf in theRecordableneeds to be moved into the request that will be exported (seeotlp_exporter.cc).Although we use
std::moveto move the span protobuf from theRecordableto the request, benchmark tests indicate that the protobuf is likely being copied, not moved. This is due to the waystd::movehandles const lvalue references (see this link for more information).To see the difference between move and copy, I did the following test:
Current code in
otlp_exporter.cc(probably copying, even though we're usingstd::move):Current benchmarks:
Modified code (swap, effectively moving):
Modified benchmarks:
We can see that the benchmarks are roughly half for the modified code (500 ns vs. 1000 ns for dense spans).
Solution
The modified code above is unsafe, since it uses a
const_cast.Another solution is to modify the
Recordableclass to store a pointer to a span protobuf, instead of a span protobuf object.The exporter could maintain a collection of the actual span protobufs. When it becomes time to export, the exporter could associate each given
Recordablewith its span protobuf using the span id. Storing span protobufs in the exporter, rather than inRecordable, would likely allow the protobufs to be moved into the requests, rather than copied.