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.

Sign up for GitHub

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

Jump to bottom

tracing: make span a union type #54737

Merged
merged 20 commits into from
Oct 23, 2020
Merged

tracing: make span a union type #54737

merged 20 commits into from
Oct 23, 2020

Commits on Oct 23, 2020

  1. tracing: make span a union type 

    Clarify that a `span` is anywhere between zero to three subspans:

- net/trace,
- external opentracing Tracer, and
- crdb-internal trace span,

where zero subspans corresponds to what is today noopSpan (not
implemented yet, but TODO added).

Lots of threads to pull, but this is a good checkpoint since
everything compiles and the diff is mechanical and small enough
to actually review.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    8598646 View commit details
    Browse the repository at this point in the history

  2. tracing: remove noopSpan 

    Remove the noopSpan and noopSpanContext types. Instead, we use a `*span`
(and `*spanContext`) with a zero trace ID (and in fact, all zero except
for the tracer).

This allows a sleuth of further cleanups which are deferred to follow-up
commits.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    3e7d258 View commit details
    Browse the repository at this point in the history

  3. tracing: move tracer from crdbSpan to span 

    That's where it belongs.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    75dc94a View commit details
    Browse the repository at this point in the history

  4. tracing: improve comment on (*span).isNoop() 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    b9ae26d View commit details
    Browse the repository at this point in the history

  5. tracing: harmonize subspan field names 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    e676d03 View commit details
    Browse the repository at this point in the history

  6. tracing: un-embed crdbSpan 

    Lots of top-level logic reaches into the `crdbSpan`, which is not ideal.
We want top-level tracing methods to delegate to a method on each of the
subspans of our main `span` type.

This commit doesn't do anything about that, but it gets us one step
closer by mechanically unembedding the crdbSpan field. The diff
highlights a lot of code that needs to be moved into methods that sit on
`*crdbSpan`.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    9c02d12 View commit details
    Browse the repository at this point in the history

  7. tracingpb: establish package 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    5f47e41 View commit details
    Browse the repository at this point in the history

  8. tracing: move parts of recording, baggage, tag functionality to crdbSpan 

    This starts towards a picture in which operations on a `*span`
essentially turn into an opaque invocation of something on a `crdbSpan`
plus possibly additional logic for the net/trace and external span.

The hope is that by pulling further on this thread `crdbSpan` can move
into its own package and thus have a clean interaction with the tracer
and top-level methods.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    b251640 View commit details
    Browse the repository at this point in the history

  9. tracing: delegate to crdbSpan in ImportRemoteSpans 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    cf0a026 View commit details
    Browse the repository at this point in the history

  10. tracing: delegate to crdbSpan in LogFields 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    4f4c513 View commit details
    Browse the repository at this point in the history

  11. tracing: delegate to crdbSpan in BaggageItem 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    4e5b594 View commit details
    Browse the repository at this point in the history

  12. tracing: improve comments on recording modes 

    I had to understand the semantics by reading the code, and now that
I think I understand them, want to save the next person from having
to do the same.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    d5c6cf7 View commit details
    Browse the repository at this point in the history

  13. tracing: create *span in only one method 

    We were previously maintaining very similar code in three places:

1. `(*Tracer).StartSpan`
2. `StartRootSpan`
3. `StartChildSpan`

This commit creates a method `(*Tracer).startSpanGeneric` and makes
the above a thin wrapper around it.

This allows us to simplify just one version of the code.  Merging the
three also showed minor differences: tags didn't seem to propagate the
same in all cases, and a bug (that I just introduced) about what
constitutes a `noopSpanContext` was highlighted and fixed.

I still have plenty of questions about this code and far from good
confidence that I didn't introduce any bugs. I did pay some attention
to performance regressions, but no benchmarks are introduced at this
point. My interest is to simplify the package first, then make it
cheap (again). We can live with a temporary performance regression,
as long as it is limited to the case in which tracing is actually
enabled.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    d433733 View commit details
    Browse the repository at this point in the history

  14. tracing: assert that we never have >1 span reference 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    d1646dd View commit details
    Browse the repository at this point in the history

  15. tracing: explain blackhole spans 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    f55f719 View commit details
    Browse the repository at this point in the history

  16. tracing: propagate shadow tracers to children only when identical 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    cf48395 View commit details
    Browse the repository at this point in the history

  17. tracing: remove a few references to shadowTracer 

    Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    374e745 View commit details
    Browse the repository at this point in the history

  18. tracing: properly export Span and SpanContext 

    They will be used instead of `opentracing.Span{,Context}` in future
commits.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    1921231 View commit details
    Browse the repository at this point in the history

  19. tracing: don't ignore recordability in StartChildSpan 

    Improve commentary about recordability while I'm there.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    9f5a99e View commit details
    Browse the repository at this point in the history

  20. tracing: respect sso.Tags in StartSpan 

    This got lost during the refactors and caused some test failures.

Release note: None
    @tbg
    tbg committed Oct 23, 2020
    Configuration menu
    Copy the full SHA
    2ace2ab View commit details
    Browse the repository at this point in the history