Cannot create nested Child Spans

[thesandlord]

Currently, there is no way to create nested spans:

Root Span
      Child Span
             Grandchild Span

In the type definition, startChildSpan has the following signature:

startChildSpan(name?: string, type?: string, parentSpanId?: string): Span;

However, if you look at the actual function implementation:

/**
   * Starts a span.
   * @param name The span name.
   * @param type The span type.
   * @param parentSpanId The parent span ID.
   */
  startChildSpan(name?: string, type?: string): types.Span {
    let newSpan: types.Span = null;
    if (!this.currentRootSpan) {
      this.logger.debug(
          'no current trace found - must start a new root span first');
    } else {
      newSpan = this.currentRootSpan.startChildSpan(name, type);
    }
    return newSpan;
  }

Even though the @param parentSpanId is present in the JavaDoc tags, it isn't actually used in the actual function!

This means that the parentSpanId is always set to the rootSpanId, which means nesting spans is impossible.

The simple workaround is to just override the parentSpanId attribute of the grandchild span to the child span id. I've tested this and it seems to work fine.

i.e:

const childSpan = tracing.tracer.startChildSpan('Child Span')

const grandChildSpan = tracing.tracer.startChildSpan('Child Span')
grandChildSpan.parentSpanId = childSpan.id

I think the the code above could be fixed with the following:

/**
   * Starts a span.
   * @param name The span name.
   * @param type The span type.
   * @param parentSpanId The parent span ID.
   */
  startChildSpan(name?: string, type?: string, parentSpanId?: string): types.Span {
    let newSpan: types.Span = null;
    if (!this.currentRootSpan) {
      this.logger.debug(
          'no current trace found - must start a new root span first');
    } else {
      newSpan = this.currentRootSpan.startChildSpan(name, type);
    }
    if (parentSpanId) {
      newSpan.parentSpanId = parentSpanId;
    }
    return newSpan;
  }

On another note, there really should be the option of passing in an object to the createChildSpan function instead of three arguments.

[hvent90]

On another note, there really should be the option of passing in an object to the createChildSpan function instead of three arguments.

I second this. For a first time user, my expectation of startChildSpan's arguments is that it would be an object because startRootSpan takes an object.

[kjin]

@thesandlord It is intended behavior for all child spans to be direct children of a root span. There isn't a notion of grandchild spans AFAIK on a single process; you can think of root and child spans as being spans tracing incoming and outgoing requests respectively.

@hvent90 On startChildSpan's arguments being an object: I completely agree. This change would be up for grabs if you (or anyone else) would like to take it.

[m1o1]

From the docs:

A child span can also be a parent of a span that it spawns

Seems like this is a bug in the node client. If OpenCensus is working with OpenTracing to come up with a common API for compatibility (which was the impression I had while attending KubeCon), it will need to support nested child spans.

[draffensperger]

I would agree with the value of creating nested child spans. If there are multiple interesting operations with sub-operations within a given request, and they are executing in parallel, then the relationships between the sub-operations could be lost because all spans are just connected to the overall request root. This could make the visualization of the sub-operation spans rendered in the wrong places and wouldn't allow for expand/collapse functionality of trace visualizers.

Here's what I think would be roughly be needed to make this work:

• Add functionality to the Span class to enable it to have children, and update the exporters to export the full tree of spans.
• Add support in the tracer Tracer to store currentParentSpan in addition to currentRootSpan. Make the startChildSpan function in the tracer make the new span be a child of currentParentSpan rather than currentRootSpan, and have it also change currentParentSpan to the new span.

I would ideally like the RootSpan and Span classes to almost or completely converge, but having them still be separate is probably best to limit the amount of breaking changes.