SpanData should define virtual explicit operator SpanData *() const { .... }

[GeorgViehoever]

...because base class trace_sdk::Recordable defines it. Currently, the conversion operator always produces nullptr even if the real class pointed to is SpanData, SpanData does not override the operator, but it should. https://github.com/open-telemetry/opentelemetry-cpp/blob/054b0dc207c1f58e290d78cdaac5f314bc328b31/sdk/include/opentelemetry/sdk/trace/recordable.h#L174C3-L174C67

Workaround for the moment: Use of dynamic_cast(recordablePtr)

[lalitb]

This conversion operator is supposed to be used only within the SDK, for the purpose of span-filtering, and not in application. Just curious, how are you using it?

[GeorgViehoever]

If the call is internal, you should mark it as such (comment, naming convention such as leading _, or something like this).

I am trying to filter Spans based on their duration before they reach a SpanExporter - to avoid the overhead of exporting tiny spans. A possible place to do that would be SpanProcessor.OnEnd(), that receives Recordable. Recordable has a SetDuration(), but unfortunately no GetDuration(). SpanData (subclassed from Recordable) has the missing GetDuration(). Using the operator SpanData *() was my first attempt to get there, but a dynamic_cast also works for Recordables that are really SpanData, since SpanData is subclassed from Recordable.

Now here is an additional problem. SpanData is used, for instance, by OStreamSpanExporter - so we are fine with somehow getting SpanData from Recordable. However, OtlpGrpcExporter uses OtlpGrpcExporter, which is not derived from SpanData, and I see now way towards a GetDuration() there. So I am stuck again :-(, no way to get duration.

Any method that would allow me to filter Spans on duration before they reach any exporter would help. Maybe I am on the wrong track with subclassed SpanExporter.OnEnd()...

[lalitb]

Ok, I can see what you are trying to achieve. I don't think it would be easy to achieve filtering with otel-cpp. There have been some discussions here - #2108 (comment) if that helps to understand the problem.

[GeorgViehoever]

Thanks for providing the pointer to #2108 (comment) and the related #2389 and #2345. They help to understand the considerations.

The fact that OTel C++ SDK does not have Readable Spans limits the abilities of any SpanProcessor - even simple cases like the filtering I had in mind are not possible, just as the more complicated ones the other issues had in mind.

I do understand the performance argument for writeable only Recordable. But having to send out 90% unwanted spans also has an overhead. I will probably have to record span information until the program terminates, and then create filtered only OTel spans "post mortem". This does not give me the near-realtime visibility I otherwise have, and it probably does not link log entries to spans, both of which would be desirable.

Maybe the next gen OTel C++ SDK can consider supporting Readable Spans as an option, especially for making SpanProcessors more useful.

[marcalff]

opentelemetry-cpp needs to support readable / writable span data.

Need to investigate how to implement this.

Risk is a performance / flexibility tradeoff.

[marcalff]

@GeorgViehoever

If I understand correctly, you have a subclass of SpanProcessor, and want to:

• inspect the span data
• possibly modify it ?
• possibly discard the Span

when execution reaches the processor OnEnd().

Please indicate how the SpanProcessor::MakeRecordable() method is implemented for the subclass.

Assuming it calls the exporter MakeRecordable(), how about instead:

• Create a SpanData in CustomSpanProcessor::MakeRecordable()
• inspect and modify the span data
• In CustomSpanProcessor::OnEnd(), if the span is to be exported, only then invoke exporter->MakeRecordable() and copy everything.

Not tested, but this way the code creates a Readable and Writable SpanData in memory first, and at the end copies the data to the underlying exporter WriteOnly Recordable.

Trying the other way (first creating a Recordable in the exporter), and somehow attempting to get a R/W SpanData from it will not work, not to mention there is no way to cancel a span in the exporter if it needs to be discarded.

[GeorgViehoever]

@marcalff Currently I dont intend to change span data in the SpanProcessor. That would be simple since all types of Recordables have the SetXYZ() methods.
Regarding your proposal to always use SpanData, and convert to Exporter specific recordable only in OnEnd(): I was actually following a similar path already, and should be able to report back on results shortly. What would be measurements/results that you are iinterested in? I am also not sure what kind of side effects that would have.
My FilteringSpanProcessor looked something like this:

class FilteringSpanProcessor: public trace_sdk::BatchSpanProcessor
{
public:
  
   FilteringSpanProcessor(std::unique_ptr<trace_sdk::SpanExporter> && exporter,
                          std::chrono::nanoseconds minTimeNs_p,
                          const trace_sdk::BatchSpanProcessorOptions & options)
      : trace_sdk::BatchSpanProcessor(std::move(exporter), options)
      , minTimeNs(minTimeNs_p)
      {
      }

   
  void OnEnd(std::unique_ptr<trace_sdk::Recordable> &&span) noexcept override
  {
     std::chrono::nanoseconds duration;
    // ... Somehow get the duration, which works via SpanData* for ConsoleExporter,
    // but does not work for OTLP Exporter
    const auto spanData = dynamic_cast<trace_sdk::SpanData *>(span.get());
    duration = spanData->GetDuration(); //crashes for OTLP Exportewr due to nullPtr

     if (duration>=minTimeNs)
     {
        trace_sdk::BatchSpanProcessor::OnEnd(std::move(span));
     }
  };

private:
   const std::chrono::nanoseconds minTimeNs;
};

[github-actions]

This issue was marked as stale due to lack of activity.