Draft to Discuss: Use native Rust support for async traits in LogExporter::export() method

[lalitb]

Changes

Continuation to #2143, more to show feasibility, performance improvements, and have discussion before raising the PR for eventual review, this PR demonstrate using async traits support in Rust exporter. The support was added in Rust v1.75, while the msrv as of now is v1.70 for otel-sdk

The change is:
Existing:

#[async_trait]
pub trait LogExporter: Send + Sync + Debug {
  async fn export(&mut self, batch: LogBatch<'_>) -> LogResult<()>;
}

PR:

pub trait LogExporter: Send + Sync + Debug {
    fn export<'a>(
        &'a mut self,
        batch: &'a LogBatch<'a>,
    ) -> impl std::future::Future<Output = LogResult<()>> + Send + 'a;
}

Have modified the stress test to use LogExporter to demonstrate the improvement:
main with modified stress test:
main...lalitb:opentelemetry-rust:main-stress-with-exporter
6,348,766 iterations/sec
5,725,142 iterations/sec
6,020,324 iterations/sec
5,814,542 iterations/sec

this PR with modified stress test:
9,550,832 iterations/sec
11,045,635 iterations/sec
10,632,124 iterations/sec
10,212,681 iterations/sec
9,810,356 iterations/sec

There are improvements of ~71%, but it need to bump msrv to 1.75. Also, most of the improvement won't be the periodic

Thanks,
Lalit

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[cijothomas]

@lalitb I think it maybe better to first check how to avoid requiring mut reference for exporting, before evaluating the results of this perf test, as that requirement has forced the need of mutex and introduced contention and hence stress tests shows way low throughput than before.

[lalitb]

@cijothomas If we remove the mut self-reference from the LogExporter::export() method, it will force the exporter implementation to rely on interior mutability to modify its internal state. This would involve using synchronization mechanisms like locks or atomic operations, which add complexity and overhead. But in the case of SimpleLogProcessor and BatchLogProcessor, these methods are always called sequentially, so there’s no real benefit to introducing this extra layer of synchronization. It would just make the code more complicated and less performant without actually solving any problem.

The use-case for having a non-mutating export method would be in reentrant log processors, where LogProcessor::emit() -> LogProcessor::export() could be invoked concurrently from multiple user threads. In such scenarios, interior mutability would be necessary if the exporter implementation needs to manage internal state. However, this is not relevant for the current simple and batch processors, which operate in a strictly sequential manner.

Actually, there are 4 different exporter scenarios to think about -

• sync/concurrent:

    fn export<'a>(&'a self, batch: &'a LogBatch<'a>) -> LogResult<()>;

• sync/sequential,

   fn export<'a>(&'a mut self, batch: &'a LogBatch<'a>) -> LogResult<()>;

• async/concurrent

  fn export<'a>(&'a self, batch: &'a LogBatch<'a>) -> impl std::future::Future<Output = LogResult<()>> + Send + 'a;

• async/sequential

 fn export<'a>(&'a mut self, batch: &'a LogBatch<'a>) -> impl std::future::Future<Output = LogResult<()>> + Send + 'a;

Currently:

• The SimpleLogProcessor and BatchLogProcessor invoke the exporter in async/sequential mode.
• The ReentrantProcessor, used by ETW and user_events in the contrib repository, operates the exporter in sync/concurrent mode.

To ensure clarity and extensibility, we need to properly model these scenarios within the design.

[lalitb]

Just to add, we don't currently have specific use-case for sync/sequential, and async/concurrent, they are mentioned above for completeness of all the scenarios.

[cijothomas]

we don't need async-trait anymore?

[lalitb]

yes, this can be safely removed.