Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Add support for StrictSpans mode in Tracing #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jhferris
Copy link
Contributor

Goal: StrictSpans mode is meant to enforce the invariant that a parent span cannot
end while it has children running. This can happen if your service has an internally-asynchronous architecture.

In this solution, if a parent span ends with active children, it is held open
and its end is triggered when the last child ends. This works even with a chain of spans, the last child
can trigger all parents up to the root to end.

Details:

  1. Switch running_span_store_impl to key its map on SpanId instead of the pointer
  2. Add FindSpanById method on RunningSpanStoreImpl
  3. Add reference count called active_child_count_ on SpanImpl to keep track of active children
  4. Add logic at span start and end time to update the reference count on parents,
  5. Add tests

@jhferris
Copy link
Contributor Author

I realize this is a nontrivial change. It started out as "I wonder how hard it would be to do this" and kind of grew from there. That said, I believe this solves an actual use case, and I've tried to make it so that the code changes/impacts are:

  1. minimal if you're not running in StrictSpans mode (not the best name, I'm open to suggestions)
  2. As clean/fitting into the existing structure of the code as possible
  3. Tested :)

I'm happy to split up the pull request into smaller pieces if that would be easier or make any other changes you feel are appropriate, if you think the core idea is viable.

Goal: StrictSpans mode is meant to enforce the invariant that a parent span cannot
end while it has children running. If a parent span ends with active children, it is held open
and its end is triggered when the last child ends. This works even with a chain of spans, the last child
can trigger all parents up to the root to end.

Details:
1. Switch running_span_store_impl to key its map on SpanId instead of the pointer
2. Add FindSpanById method on RunningSpanStoreImpl
3. Add reference count called active_child_count_ on SpanImpl to keep track of active children
4. Add logic at span start and end time to update the reference count on parents,
5. Add tests
@g-easy
Copy link
Contributor

g-easy commented Apr 3, 2019

Where does the StrictSpans idea come from? I can't find any references to this term.

@g-easy
Copy link
Contributor

g-easy commented Apr 3, 2019

This PR adds a new TraceOptions bit. Could you please open an issue at https://github.com/census-instrumentation/opencensus-specs and describe the motivating use case for "strict" mode. There are several implementations of OpenCensus beyond opencensus-cpp, and the project needs to decide whether to implement this feature in every language.

Also I'm worried about the performance overhead of the refcounting, and that we won't be able to replace the underlying implementation with a Disruptor-style approach (e.g. like what opencensus-java does).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants