-
Notifications
You must be signed in to change notification settings - Fork 765
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
Aligns exporting pipeline with SDK spec #227
Conversation
3751a86
to
1445643
Compare
{ | ||
this.worker.RegisterHandler(name, handler); | ||
/// <summary> | ||
/// Batch is successfully exported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why batch is in the name of an enum in the abstract exporter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exporters always export list of spans. Batch vs non-batch exists only in processor.
{ | ||
protected readonly SpanExporter Exporter; | ||
|
||
protected SpanProcessor(SpanExporter exporter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can processors be chained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tracer will have multiple processors, they are executed sequentially (not chained) per spec.
We'll handle processor registration in #226
// limitations under the License. | ||
// </copyright> | ||
|
||
namespace OpenTelemetry.Trace.Export.Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for adding a test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just couple of comments. Looks good otherwise
According to spec, Tracer should have processor(s): simple and batching.
Each processor has exporter which can export batch and shutdown
this change aligns implementation with spec (partially) and removes extra abstractions left from census.
Depends on #221, #222Fixes #120, #223, #225
More to do: #224 (batching processor), #226 (multi-processor registration)