Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OTEL batch processor instead of ES bulk processor #2162

Closed
pavolloffay opened this issue Feb 19, 2020 · 5 comments
Closed

Use OTEL batch processor instead of ES bulk processor #2162

pavolloffay opened this issue Feb 19, 2020 · 5 comments

Comments

@pavolloffay
Copy link
Member

Created from jaegertracing/jaeger-opentelemetry-collector#14 (comment)

Elasticsearch bulk processor batches multiple requests into a single request which is sent to ES bulk API.

We could leverage OTEL batch processor https://github.com/open-telemetry/opentelemetry-collector/tree/master/processor/batchprocessor which has similar behavior. The batch processor batches TraceData from the same service and passes it forward once size or timeout criteria are satisfied.

To use the batch processor we will have to refactor Jaeger span writer.

@objectiser
Copy link
Contributor

objectiser commented Mar 6, 2020

To use the batch processor we will have to refactor Jaeger span writer.

If we considered these OTel exporters (for storage) replacements for the Jaeger span writers, then we may avoid such issues, and also be able to directly support the OTel proto model -> storage, related to #2117 and jaegertracing/jaeger-opentelemetry-collector#6 .

It may require duplication of code during the transition phase, but once the OTC is being used in place of the Jaeger Collector, this should result in the current Jaeger span writers being redundant anyway.

@pavolloffay
Copy link
Member Author

The prerequisite is to switch Jaeger to OTEL. Then the storage writer will have to be refactored to work on the batch/spanData.

@pavolloffay
Copy link
Member Author

#2117 moved to #2117

Move to OTEL model in Jager core does not solve the problem with reusing OTEL batch processor. To support OTEL batch processor we will have to redesign writer interface to work on list of spans and appropriately change writer implementations.

@objectiser
Copy link
Contributor

Still not clear to me why we need to continue with using the current writer interfaces, rather than just porting the implementations to become exporter impls directly?

@pavolloffay pavolloffay transferred this issue from jaegertracing/jaeger-opentelemetry-collector Apr 6, 2020
@ghost ghost added the needs-triage label Apr 6, 2020
@pavolloffay
Copy link
Member Author

Still not clear to me why we need to continue with using the current writer interfaces, rather than just porting the implementations to become exporter impls directly?

Eventually we will have to do that. My objective was to avoid having two implementations at the same time.

I am closing this and I will open a new issue for re-implementing all storage writers to use OTEL model directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants