Clarification for exporter vs processor chaining

lmolkova commented 5 years ago

Spec suggests that exporters are chainable

Allow implementing helpers as composable components that use the same chainable Exporter interface. SDK authors are encouraged to implement common functionality such as queuing, batching, tagging, etc. as helpers.

What are the reasons for making exporter (not processor) chainable?

Processor

runs synchronously with span and possibilities to tag things on spans are much wider than at exporter time.
Readable Span may have no APIs for tagging/modification of spans
Processor happens earlier in the pipeline and it's more efficient to filter there
if you filter spans in filtering exporter, you'd do it after batching, i.e. this may change how many spans exporter is called with
if you don't have exporter (e.g. you implement zpages processor or some kind of debugging processor) - how would you do filtering/tagging?

Processors seem like a better place for chaining (which does not imply exporters cannot be chained).

tsloughter commented 5 years ago

Seconded. The spec also says processors are "invoked in the same order as they were registered", if they aren't chained then why require an order?

Oberon00 commented 5 years ago

Why do you think that SpanProcessors wouldn't be chainable? The spec always talks about them in plural and the sentence that @tsloughter quoted makes it very clear to me that the spec says we should be able to chain multiple span processors.

EDIT: It seems this issue is more about "why are span processors not chainable" rather than "why are exporters chainable", am I correct?

tsloughter commented 5 years ago

@Oberon00 because they return void and not the span.

Oberon00 commented 5 years ago

Why should they return the Span? I think they are expected to just do their work with the Span, possibly modifying it in-place.

tsloughter commented 5 years ago

Ah, sorry, that is my misreading because of being used to immutable variables.

If that is the case then it could use some clarifying to say it runs them in order and the span can be modified. And better if it didn't say it must return "void" even if that would be the case in certain implementations. I opened #323 to discuss how the spec could be written in a clearer, language agnostic, way.

But maybe @lmolkova was thinking of something else?

Oberon00 commented 5 years ago

True, the Return type: Void part of the spec is superfluous at best.

lmolkova commented 5 years ago

The spec seems to imply that there are multiple processor->may-be-exporter chains so that you can send spans to different places (zpages and Jaeger).

  +-----+---------------+   +---------------------+   +-------------------+
  |     |               |   |                     |   |                   |
  |     |               |   | BatchProcessor      |   |    SpanExporter   |
  |     |               +---> SimpleProcessor     +--->  (JaegerExporter) |
  | SDK | SpanProcessor |   |                     |   |                   |
  |     |               |   +---------------------+   +-------------------+
  |     |               |
  |     |               |   +---------------------+
  |     |               |   |                     |
  |     |               +---> ZPagesProcessor     |
  |     |               |   |                     |
  +-----+---------------+   +---------------------+

I believe it also implies that you may want to filter span differently in different chains.

I.e. it seems real-world case might look like

span -> processor1 (filtering) -> processor2 (batching) -> zipkin
     -> processor3 (zpages)

I.e. span events are processed by each processor sequentially (not chained). And then each first processor might be a start of a new chain of processors that may end up with exporter.

So my questions about spec are

plural processors intention is to have multiple possible destinations?
plural processors invoke each other (i.e. filtering could be done) or they all invoked by span.Start/End sequentially?
or both: there could be multiple pipelines of processors each of which is a chain of processors ending up with exporter. And pipelines are executed sequentially on the Span Start/End

My guess that spec implies 3, but suggests chaining with the exporter. I argue that should be done in processor. I'm happy to update spec to make it more clear, but looking for someone to confirm the intent

open-telemetry / opentelemetry-specification

Clarification for exporter vs processor chaining #316