opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

Make `WITH_ASYNC_EXPORT_PREVIEW` mainstream

Open marcalff opened this issue 1 year ago • 12 comments

To discuss.

Asynchronous export has been stable for a while.

The preview flag can be removed, and the code be mainstream (without ifdef).

This will decrease the overall complexity, and simplify CI builds.

marcalff avatar Oct 12 '23 14:10 marcalff

If I remember correctly, Async export was put under feature flag as it is not specs compliant. It calls Export::export simultaneously without waiting for previous call to end.

lalitb avatar Oct 12 '23 15:10 lalitb

If I remember correctly, Async export was put under feature flag as it is not specs compliant. It calls Export::export simultaneously without waiting for previous call to end.

Good to know.

In that case, we should investigate which parts are still missing and not compliant, so it gets resolved over time, instead of keeping a feature flag forever.

marcalff avatar Oct 12 '23 16:10 marcalff

This was a feature provided for those looking for high throughput at the expense of deviating from the specifications. I don't think the plan was ever to make this compliant.

lalitb avatar Oct 12 '23 17:10 lalitb

Is it possible to move the async exporter to contrib repo then, if it is not in the spec?

ThomsonTan avatar Oct 12 '23 18:10 ThomsonTan

Thats a good idea. Though won’t be easy as I think this preview flag is both in batch processor and exporter.

lalitb avatar Oct 12 '23 18:10 lalitb

Is it possible to move the async exporter to contrib repo then, if it is not in the spec?

It's in spec but optional.

https://github.com/open-telemetry/opentelemetry-proto/blob/main/docs/specification.md#otlpgrpc-concurrent-requests https://github.com/open-telemetry/opentelemetry-proto/blob/main/docs/specification.md#otlphttp-concurrent-requests

owent avatar Oct 13 '23 11:10 owent

Aync exporting for OTLP gRPC exporter do not implemented yet.

owent avatar Oct 13 '23 11:10 owent

To summarize:

  • HTTP async is in the spec, and implemented
  • GRPC async is in the spec, but not implemented
  • the name WITH_ASYNC_EXPORT_PREVIEW is not specific to HTTP or GRPC

To discuss, possible options:

  • wait for the GRPC async implementation, and put both mainstream much later
  • make HTTP async mainstream now, and use a different feature flag later for GRPC (WITH_GRPC_ASYNC_EXPORT_PREVIEW)

marcalff avatar Oct 18 '23 19:10 marcalff

To summarize:

  • HTTP async is in the spec, and implemented
  • GRPC async is in the spec, but not implemented
  • the name WITH_ASYNC_EXPORT_PREVIEW is not specific to HTTP or GRPC

To discuss, possible options:

  • wait for the GRPC async implementation, and put both mainstream much later
  • make HTTP async mainstream now, and use a different feature flag later for GRPC (WITH_GRPC_ASYNC_EXPORT_PREVIEW)

Maybe I will have time to implement gRPC async exporting one or two weeks later. Personally I prefer to keep WITH_ASYNC_EXPORT_PREVIEW now.

owent avatar Oct 19 '23 10:10 owent

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

github-actions[bot] avatar Dec 19 '23 01:12 github-actions[bot]

Maybe WITH_GRPC_ASYNC_EXPORT_PREVIEW can be removed after #2407 is merged and test it for some time.

owent avatar Dec 19 '23 02:12 owent

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

github-actions[bot] avatar Feb 21 '24 01:02 github-actions[bot]