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

Introduces SpanRecord type and clarifies a PendingSpan concept #736

Merged
merged 1 commit into from
Jul 27, 2018

Conversation

codefromthecrypt
Copy link
Member

Before, we had some elements that weren't well organized about not-yet-
reported spans. Formerly, there was MutableSpan, MutableSpanMap, and
Recorder. These were internal and replaced by still internal types of:

  • SpanRecord: mutable type which can become a Zipkin span.
  • PendingSpan: SpanRecord and a clock appropriate for a trace context.
  • PendingSpans: repository allowing lookup and removal by trace context.

These are clarifications and optimizations of code that existed before.
Notably, this removes the need to re-build a zipkin span to customize it
before reporting, or to need a zipkin span at all when reporting to
metrics or other aggregations. Future work on that will happen in later
pull requests. There is no public api change involved as all work is
currently internal.

See #557

Before, we had some elements that weren't well organized about not-yet-
reported spans. Formerly, there was `MutableSpan`, `MutableSpanMap`, and
`Recorder`. These were internal and replaced by still internal types of:

* SpanRecord: mutable type which can become a Zipkin span.
* PendingSpan: SpanRecord and a clock appropriate for a trace context.
* PendingSpans: repository allowing lookup and removal by trace context.

These are clarifications and optimizations of code that existed before.
Notably, this removes the need to re-build a zipkin span to customize it
before reporting, or to need a zipkin span at all when reporting to
metrics or other aggregations. Future work on that will happen in later
pull requests. There is no public api change involved as all work is
currently internal.
@codefromthecrypt codefromthecrypt merged commit 328985c into master Jul 27, 2018
@codefromthecrypt codefromthecrypt deleted the efficiency branch July 27, 2018 04:12
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.

1 participant