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

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

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

Conversation

hekike
Copy link
Contributor

@hekike hekike commented May 5, 2019

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.

@codecov-io
Copy link

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%) ⬇️
src/internal/util.ts 90% <0%> (-10%) ⬇️
src/tags/validation.ts 92.85% <0%> (-7.15%) ⬇️
src/common/validations.ts 90% <0%> (-6%) ⬇️
src/tags/propagation/variant-encoding.ts 95.23% <0%> (-4.77%) ⬇️
src/trace/propagation/noop-propagation.ts 37.5% <0%> (-2.5%) ⬇️
src/tags/propagation/text-format.ts 94.87% <0%> (-0.69%) ⬇️
src/tags/propagation/binary-serializer.ts 96.72% <0%> (-0.39%) ⬇️
src/tracecontext-format.ts 96.42% <0%> (-0.19%) ⬇️
test/test-tracecontext-format.ts 99.06% <0%> (-0.04%) ⬇️
... 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.

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

Successfully merging this pull request may close these issues.

3 participants