opencensus-node icon indicating copy to clipboard operation
opencensus-node copied to clipboard

[PROPOSAL] feat(tracer): separate cls out of tracer

Open hekike opened this issue 6 years ago • 1 comments

This PR is work in progress...

On the meeting with @bogdandrutu there was this idea to have a pure Tracer API and making Span creation synchronous and turning CLS to a layer top of the pure Tracer API. This PR experiments with this API with the current Opencensus Node library.

(CLS needs an async fn to carry on context)

Pure synchronous Tracer APIs:

  /**
   * Start a new RootSpan to currentRootSpan
   * @param options Options for tracer instance
   * @param fn Callback function
   * @returns {Span} A root span
   */
  startRootSpan(options: TraceOptions): Span;

  /**
   * Start a new Span instance to the currentRootSpan
   * @param childOf Span
   * @param [options] A TraceOptions object to start a root span.
   * @returns The new Span instance started
   */
  startChildSpan(options?: SpanOptions): Span;

CLS layer top of the pure API + sugar startWithRootSpan that combines both span creation and cls context set:

  /**
   * Sets span to CLS
   * @param span Span to setup in context.
   * @param fn A callback function that runs after context setup.
   */
  withSpan<T>(span: Span, fn: () => T): T;

  /**
   * Starts a root span and sets it to context.
   * @param options Options for tracer instance
   * @param fn A callback function to run after starting a root span.
   */
  startWithRootSpan<T>(options: TraceOptions, fn: (root: Span) => T): T;

An additional benefit of this API is that using withSpan developer could chain spans in CLS and client RPCs like gRPC would start from the correct contextual child span.

hekike avatar May 05 '19 00:05 hekike

Codecov Report

Merging #498 into master will increase coverage by <.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #498      +/-   ##
==========================================
+ Coverage   94.93%   94.94%   +<.01%     
==========================================
  Files         145      145              
  Lines       10195     9915     -280     
  Branches      872      865       -7     
==========================================
- Hits         9679     9414     -265     
+ Misses        516      501      -15
Impacted Files Coverage Δ
src/zpages-frontend/page-handlers/templates-dir.ts 80% <0%> (-20%) :arrow_down:
src/internal/util.ts 90% <0%> (-10%) :arrow_down:
src/tags/validation.ts 92.85% <0%> (-7.15%) :arrow_down:
src/common/validations.ts 90% <0%> (-6%) :arrow_down:
src/tags/propagation/variant-encoding.ts 95.23% <0%> (-4.77%) :arrow_down:
src/trace/propagation/noop-propagation.ts 37.5% <0%> (-2.5%) :arrow_down:
src/tags/propagation/text-format.ts 94.87% <0%> (-0.69%) :arrow_down:
src/tags/propagation/binary-serializer.ts 96.72% <0%> (-0.39%) :arrow_down:
src/tracecontext-format.ts 96.42% <0%> (-0.19%) :arrow_down:
test/test-tracecontext-format.ts 99.06% <0%> (-0.04%) :arrow_down:
... and 45 more

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 92edaa9...6decc9b. Read the comment docs.

codecov-io avatar May 05 '19 00:05 codecov-io