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

Add SpanExporter Interface #75

Closed
wants to merge 1 commit into from

Conversation

mayurkale22
Copy link
Member

This PR contains an interface (SpanExporter) that allows different tracing services to export recorded data for sampled spans in their own format.

* Called to export sampled {@link Span}s.
* @param spans the list of sampled Spans to be exported.
*/
export(spans: Span[]): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this called? We need spans to be grouped by trace, so unless this is called at the end of every trace it wouldn't work for us.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning to add a SpanEventListener interface to record start and end of the spans. Exporters would use these methods to collect and store spans locally and export method would run every few secs to export to backend.

interface SpanEventListener {
    onStartSpan(span: Span): void;
    onEndSpan(span: Span): void;
}

interface SpanExporter extends SpanEventListener {}

Let me know if this makes sense, otherwise please suggest an alternatives.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand how this addresses the issue. What we need is to be able to report traces to our backend as traces and not as individual spans. In OpenCensus for example, it's traces that were passed to this method and not individual spans.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume by traces here you mean all of the spans for a given trace that were generated by the process? (Since presumably the full distributed trace is being written by different microservices). Could we implement that behavior by having some kind of buffering adapter that listens for spans and groups them by trace ID and then flushes them once a certain period of time goes by? Or how might the system know that no more spans for a given trace ID within the process will be created?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume by traces here you mean all of the spans for a given trace that were generated by the process?

Yes, but given the recent discussions it seems that exporters will be responsible for handling the batching, in which case my initial concern is solved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, yeah, I could imagine some batching utilities that exporters could use for common batching strategies, one of which could be grouping by trace ID

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this need will naturally appear if code starts to be duplicated. In the meantime, I wouldn't try to predict how batching will be done. In our case, we batch in very specific ways to optimize calls as much as possible, so we wouldn't be able to use a generic batch utility.

@mayurkale22 mayurkale22 removed this from the API complete milestone Jul 8, 2019
@mayurkale22
Copy link
Member Author

Closed via #177

@mayurkale22 mayurkale22 deleted the SpanExporter branch August 12, 2019 20:42
dyladan pushed a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
dyladan pushed a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
…is-4.x

Update dependency @types/ioredis to v4.16.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants