feat: add rootSpan.createChildSpan and change none CLS semantics by kjin · Pull Request #731 · googleapis/cloud-trace-nodejs

kjin · 2018-04-27T17:32:16Z

Fixes #719

This PR does two things:

In feat: allow "disabling" cls, and relax requirements for creating root spans #728, a new config.clsMechanism option was added where, when set to 'none', a barebones CLS mechanism would be used where when a root span was started, it became the only possible active root span context until it was ended. This PR changes this so that root span context will never be written when clsMechanism is set to 'none'. Instead, the root span context always has a default value of UNTRACED. This affects the behavior of the custom tracing API in the following ways:
- traceApi.runInRootSpan will always return a root span (unless disallowed by the trace policy), even if run within a function passed to another call to runInRootSpan.
- traceApi.createChildSpan will always return an UNTRACED child span. This is because root spans can no longer be automatically determined from the root context.
To address the second difference above, there is now a rootSpan.createChildSpan which takes the same options as traceApi.createChildSpan. When clsMechanism is set to 'none', the latter will not work, but the former will; when clsMechanism is set to/defaults to 'auto', they should behave the same way.

codecov · 2018-04-27T23:07:12Z

Codecov Report

Merging #731 into master will decrease coverage by 0.04%.
The diff coverage is 91.17%.

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   90.92%   90.88%   -0.05%     
==========================================
  Files          30       30              
  Lines        1564     1568       +4     
  Branches      302      304       +2     
==========================================
+ Hits         1422     1425       +3     
  Misses         59       59              
- Partials       83       84       +1

Impacted Files	Coverage Δ
src/cls.ts	`96.15% <100%> (+1.92%)`	⬆️
src/cls/null.ts	`88.88% <100%> (ø)`
src/trace-api.ts	`93.4% <84.61%> (-1.04%)`	⬇️
src/span-data.ts	`93.87% <90%> (-1.37%)`	⬇️

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 6e46ed1...978ad95. Read the comment docs.

kjin · 2018-04-28T00:53:17Z

/cc @tjmehta

tjmehta · 2018-04-28T02:04:38Z

Wow you are fast @kjin! I am out of town and will check this out tomorrow

Sign in to view

    cwd: installDir
  });
-  await spawnP('npm', ['install', 'typescript', '@types/node', tgz[0]], {
+  await spawnP('npm', ['install', 'typescript', '@types/node@9', tgz[0]], {


Sign in to view


  enable(): void {
-    if (!this.enabled) {
+    // if this.CLSClass = DefaultCLS, the user specifically asked not to use


Sign in to view

 import {Constants, SpanDataType} from './constants';
-import {SpanData as SpanData} from './plugin-types';
+import * as types from './plugin-types';
+import {SpanData, SpanOptions} from './plugin-types';


Sign in to view

  }

+  createChildSpan(options: SpanOptions): SpanData {
+    options = options || {name: ''};


Sign in to view


  enable(): void {
-    if (!this.enabled) {
+    // if this.CLSClass = NoneCLS, the user specifically asked not to use


Sign in to view


 import {Constants, SpanDataType} from './constants';
-import {SpanData as SpanData} from './plugin-types';
+import * as types from './plugin-types';


kjin · 2018-05-01T17:11:09Z

@tjmehta Were you able to get a chance to look at the new API? (I understand that it's a sizeable PR -- I think the user-visible changes should mostly be encapsulated in this file)

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2018

kjin force-pushed the new-child branch 4 times, most recently from 2fdd4cc to 9d00124 Compare April 27, 2018 23:07

kjin force-pushed the new-child branch 3 times, most recently from c3add20 to c3bd690 Compare April 28, 2018 00:11

kjin requested review from DominicKramer, jinwoo and ofrobots April 28, 2018 00:52

jinwoo reviewed Apr 30, 2018

View reviewed changes

kjin added 3 commits April 30, 2018 15:20

feat: add rootSpan.createChildSpan and change none CLS semantics

629f88b

refactor: rename DefaultCLS to NullCLS

e323337

chore: address other comments

b5d1e9c

kjin force-pushed the new-child branch from c3bd690 to b5d1e9c Compare April 30, 2018 22:26

fix: make child span options optional

c5e0f07

jinwoo approved these changes Apr 30, 2018

View reviewed changes

chore: address comments

978ad95

kjin merged commit d0009ff into googleapis:master May 2, 2018

kjin mentioned this pull request May 2, 2018

chore: 2.8.0 release proposal #733

Merged

release-please Bot mentioned this pull request Jun 30, 2022

chore(main): release 7.0.0 #1452

Merged

Conversation

kjin commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kjin commented Apr 28, 2018

Uh oh!

tjmehta commented Apr 28, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

kjin commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kjin commented Apr 27, 2018 •

edited

Loading

codecov Bot commented Apr 27, 2018 •

edited

Loading

kjin commented May 1, 2018 •

edited

Loading