set CurrentRootSpan correctly

[mayurkale22]

While debugging #526, I found we are not assigning currentRootSpan correctly, especially in case of tracer with cls. Ideally, we should set currentRootSpan before the onStartSpan event. This change is done in 92edaa9#diff-b46c2c01fbf239aae015e19185803e0dL154

The onStartSpan and onEndSpan events contains following check ->

if (this.currentRootSpan.traceId !== root.traceId) {
      this.logger.debug(
          'currentRootSpan != root on notifyStart. Need more investigation.');
    }

[mayurkale22]

@hekike This is related to #484, Could you please review?

[hekike]

LGTM

Related topic: should a child span become a parent for the next child span with CLS?
It's not really possible with the current sync startChildSpan interface as we need to update span in CLS context.

[codecov-io]

Codecov Report

Merging #532 into master will decrease coverage by 0.25%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
- Coverage   95.21%   94.96%   -0.26%     
==========================================
  Files         147      147              
  Lines        9870     9568     -302     
  Branches      539      539              
==========================================
- Hits         9398     9086     -312     
- Misses        472      482      +10

Impacted Files	Coverage Δ
src/common-utils.ts	75% <0%> (-20.24%)	⬇️
test/test-instana.ts	88.46% <0%> (-11.54%)	⬇️
test/test-stackdriver-cloudtrace.ts	92% <0%> (-6.96%)	⬇️
test/test-stackdriver-monitoring-utils.ts	93.33% <0%> (-6.67%)	⬇️
test/test-zipkin.ts	91.66% <0%> (-6.07%)	⬇️
src/trace/model/tracer.ts	66.66% <0%> (-3.11%)	⬇️
src/trace/model/tracer-base.ts	92.7% <0%> (-0.08%)	⬇️
src/trace/model/types.ts	100% <0%> (ø)	⬆️
src/trace/model/span.ts	97.6% <0%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5ff47c...471538c. Read the comment docs.

[mayurkale22]

Related topic: should a child span become a parent for the next child span with CLS? It's not really possible with the current sync startChildSpan interface as we need to update span in CLS context.

Yes, currently it's not possible with startChildSpan interface. We already have issue for same #256. If you are feeling motivated, maybe you can attempt to write solution and open a PR. :)

[hekike]

@mayurkale22 what do you think about this proposal #498 it would make it possible.