Move SpanContext isValid to the API (#1447)

* refactor: make spanContext into class with isValid * refactor: use class instantiation for spanContext * refactor: moves test and fix tests * fix: add check for isValid * revert: the changes to spanContext * refactor: move spancontext-utils to api package * fix: lint and invalid psan id and invalid trace id * fix: make isValid more robust * refactor: rename isValid to isSpanContextValid * refactor: update regex and checks * fix: export isSpanContextValid from api.trace * fix: run npm run lint:fix * fix: run lint fix in api * refactor: prevent function overhead * fix: lint * fix: remove unused import Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
open-telemetry · Aug 31, 2020 · b88b95b · b88b95b
1 parent a7a6b7f
commit b88b95b
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 32 deletions.
diff --git a/packages/opentelemetry-api/src/api/trace.ts b/packages/opentelemetry-api/src/api/trace.ts
@@ -18,6 +18,7 @@ import { NOOP_TRACER_PROVIDER } from '../trace/NoopTracerProvider';
 import { ProxyTracerProvider } from '../trace/ProxyTracerProvider';
 import { Tracer } from '../trace/tracer';
 import { TracerProvider } from '../trace/tracer_provider';
+import { isSpanContextValid } from '../trace/spancontext-utils';
 import {
   API_BACKWARDS_COMPATIBILITY_VERSION,
   GLOBAL_TRACE_API_KEY,
@@ -87,4 +88,6 @@ export class TraceAPI {
     delete _global[GLOBAL_TRACE_API_KEY];
     this._proxyTracerProvider = new ProxyTracerProvider();
   }
+
+  public isSpanContextValid = isSpanContextValid;
 }
diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts
@@ -55,6 +55,12 @@ export * from './trace/trace_state';
 export * from './trace/tracer_provider';
 export * from './trace/tracer';
 
+export {
+  INVALID_SPANID,
+  INVALID_TRACEID,
+  INVALID_SPAN_CONTEXT,
+} from './trace/spancontext-utils';
+
 export { Context } from '@opentelemetry/context-base';
 
 import { ContextAPI } from './api/context';

diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts
@@ -20,15 +20,7 @@ import { Attributes } from './attributes';
 import { Span } from './span';
 import { SpanContext } from './span_context';
 import { Status } from './status';
-import { TraceFlags } from './trace_flags';
-
-export const INVALID_TRACE_ID = '0';
-export const INVALID_SPAN_ID = '0';
-const INVALID_SPAN_CONTEXT: SpanContext = {
-  traceId: INVALID_TRACE_ID,
-  spanId: INVALID_SPAN_ID,
-  traceFlags: TraceFlags.NONE,
-};
+import { INVALID_SPAN_CONTEXT } from './spancontext-utils';
 
 /**
  * The NoopSpan is the default {@link Span} that is used when no Span

diff --git a/...metry-core/src/trace/spancontext-utils.ts → ...emetry-api/src/trace/spancontext-utils.ts b/...metry-core/src/trace/spancontext-utils.ts → ...emetry-api/src/trace/spancontext-utils.ts
@@ -13,24 +13,33 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import { SpanContext } from './span_context';
+import { TraceFlags } from './trace_flags';
 
-import { SpanContext, TraceFlags } from '@opentelemetry/api';
-
-export const INVALID_SPANID = '0';
-export const INVALID_TRACEID = '0';
+const VALID_TRACEID_REGEX = /^([0-9a-f]{32})$/i;
+const VALID_SPANID_REGEX = /^[0-9a-f]{16}$/i;
+export const INVALID_SPANID = '0000000000000000';
+export const INVALID_TRACEID = '00000000000000000000000000000000';
 export const INVALID_SPAN_CONTEXT: SpanContext = {
   traceId: INVALID_TRACEID,
   spanId: INVALID_SPANID,
   traceFlags: TraceFlags.NONE,
 };
 
+export function isValidTraceId(traceId: string): boolean {
+  return VALID_TRACEID_REGEX.test(traceId) && traceId !== INVALID_TRACEID;
+}
+
+export function isValidSpanId(spanId: string): boolean {
+  return VALID_SPANID_REGEX.test(spanId) && spanId !== INVALID_SPANID;
+}
+
 /**
  * Returns true if this {@link SpanContext} is valid.
  * @return true if this {@link SpanContext} is valid.
  */
-export function isValid(spanContext: SpanContext): boolean {
+export function isSpanContextValid(spanContext: SpanContext): boolean {
   return (
-    spanContext.traceId !== INVALID_TRACEID &&
-    spanContext.spanId !== INVALID_SPANID
+    isValidTraceId(spanContext.traceId) && isValidSpanId(spanContext.spanId)
   );
 }
diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts
@@ -17,8 +17,8 @@
 import * as assert from 'assert';
 import {
   CanonicalCode,
-  INVALID_SPAN_ID,
-  INVALID_TRACE_ID,
+  INVALID_SPANID,
+  INVALID_TRACEID,
   NoopSpan,
   TraceFlags,
 } from '../../src';
@@ -45,8 +45,8 @@ describe('NoopSpan', () => {
 
     assert.ok(!span.isRecording());
     assert.deepStrictEqual(span.context(), {
-      traceId: INVALID_TRACE_ID,
-      spanId: INVALID_SPAN_ID,
+      traceId: INVALID_TRACEID,
+      spanId: INVALID_SPANID,
       traceFlags: TraceFlags.NONE,
     });
     span.end();

diff --git a/...core/test/trace/spancontext-utils.test.ts → ...-api/test/trace/spancontext-utils.test.ts b/...core/test/trace/spancontext-utils.test.ts → ...-api/test/trace/spancontext-utils.test.ts
@@ -16,7 +16,7 @@
 
 import * as assert from 'assert';
 import * as context from '../../src/trace/spancontext-utils';
-import { TraceFlags } from '@opentelemetry/api';
+import { TraceFlags } from '../../src';
 
 describe('spancontext-utils', () => {
   it('should return true for valid spancontext', () => {
@@ -25,7 +25,7 @@ describe('spancontext-utils', () => {
       spanId: '6e0c63257de34c92',
       traceFlags: TraceFlags.NONE,
     };
-    assert.ok(context.isValid(spanContext));
+    assert.ok(context.isSpanContextValid(spanContext));
   });
 
   it('should return false when traceId is invalid', () => {
@@ -34,7 +34,7 @@ describe('spancontext-utils', () => {
       spanId: '6e0c63257de34c92',
       traceFlags: TraceFlags.NONE,
     };
-    assert.ok(!context.isValid(spanContext));
+    assert.ok(!context.isSpanContextValid(spanContext));
   });
 
   it('should return false when spanId is invalid', () => {
@@ -43,7 +43,7 @@ describe('spancontext-utils', () => {
       spanId: context.INVALID_SPANID,
       traceFlags: TraceFlags.NONE,
     };
-    assert.ok(!context.isValid(spanContext));
+    assert.ok(!context.isSpanContextValid(spanContext));
   });
 
   it('should return false when traceId & spanId is invalid', () => {
@@ -52,6 +52,6 @@ describe('spancontext-utils', () => {
       spanId: context.INVALID_SPANID,
       traceFlags: TraceFlags.NONE,
     };
-    assert.ok(!context.isValid(spanContext));
+    assert.ok(!context.isSpanContextValid(spanContext));
   });
 });
diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts
@@ -33,7 +33,6 @@ export * from './trace/sampler/AlwaysOffSampler';
 export * from './trace/sampler/AlwaysOnSampler';
 export * from './trace/sampler/ParentOrElseSampler';
 export * from './trace/sampler/ProbabilitySampler';
-export * from './trace/spancontext-utils';
 export * from './trace/TraceState';
 export * from './trace/IdGenerator';
 export * from './utils/url';

diff --git a/packages/opentelemetry-core/src/trace/NoRecordingSpan.ts b/packages/opentelemetry-core/src/trace/NoRecordingSpan.ts
@@ -14,8 +14,11 @@
  * limitations under the License.
  */
 
-import { SpanContext, NoopSpan } from '@opentelemetry/api';
-import { INVALID_SPAN_CONTEXT } from '../trace/spancontext-utils';
+import {
+  SpanContext,
+  NoopSpan,
+  INVALID_SPAN_CONTEXT,
+} from '@opentelemetry/api';
 
 /**
  * The NoRecordingSpan extends the {@link NoopSpan}, making all operations no-op

diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
@@ -19,13 +19,12 @@ import * as opentracing from 'opentracing';
 import { BasicTracerProvider, Span } from '@opentelemetry/tracing';
 import { TracerShim, SpanShim, SpanContextShim } from '../src/shim';
 import {
-  INVALID_SPAN_CONTEXT,
   timeInputToHrTime,
   HttpTraceContext,
   CompositePropagator,
   HttpCorrelationContext,
 } from '@opentelemetry/core';
-import { propagation } from '@opentelemetry/api';
+import { INVALID_SPAN_CONTEXT, propagation } from '@opentelemetry/api';
 import { performance } from 'perf_hooks';
 
 describe('OpenTracing Shim', () => {

diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
@@ -20,7 +20,6 @@ import {
   getActiveSpan,
   getParentSpanContext,
   InstrumentationLibrary,
-  isValid,
   NoRecordingSpan,
   IdGenerator,
   RandomIdGenerator,
@@ -79,7 +78,7 @@ export class Tracer implements api.Tracer {
     const spanId = this._idGenerator.generateSpanId();
     let traceId;
     let traceState;
-    if (!parentContext || !isValid(parentContext)) {
+    if (!parentContext || !api.trace.isSpanContextValid(parentContext)) {
       // New root span.
       traceId = this._idGenerator.generateTraceId();
     } else {