fix: change SpanContext.traceFlags to mandatory (open-telemetry#818)

* fix: change SpanContext.traceFlags to mandatory According to spec SpanContext represents the W3C tracestate which includes traceId, spanId and traceFlags. As a side effect a new LinkContext types was added as links don't have traceFlags according to spec. * chore: review findings, rename TraceFlags.UNSAMPLED to NONE * fix: build * fix: tests * fix: correct merge Co-authored-by: Daniel Dyla <[email protected]>
dyladan · Feb 18, 2021 · f2e04b5 · f2e04b5
1 parent e19c514
commit f2e04b5
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 28 deletions.
diff --git a/api/src/index.ts b/api/src/index.ts
@@ -31,6 +31,7 @@ export * from './metrics/ObserverResult';
 export * from './trace/attributes';
 export * from './trace/Event';
 export * from './trace/instrumentation/Plugin';
+export * from './trace/link_context';
 export * from './trace/link';
 export * from './trace/NoopSpan';
 export * from './trace/NoopTracer';

diff --git a/api/src/trace/NoopSpan.ts b/api/src/trace/NoopSpan.ts
@@ -26,7 +26,7 @@ export const INVALID_SPAN_ID = '0';
 const INVALID_SPAN_CONTEXT: SpanContext = {
   traceId: INVALID_TRACE_ID,
   spanId: INVALID_SPAN_ID,
-  traceFlags: TraceFlags.UNSAMPLED,
+  traceFlags: TraceFlags.NONE,
 };
 
 /**
@@ -59,11 +59,6 @@ export class NoopSpan implements Span {
     return this;
   }
 
-  // By default does nothing
-  addLink(spanContext: SpanContext, attributes?: Attributes): this {
-    return this;
-  }
-
   // By default does nothing
   setStatus(status: Status): this {
     return this;

diff --git a/api/src/trace/link.ts b/api/src/trace/link.ts
@@ -15,16 +15,16 @@
  */
 
 import { Attributes } from './attributes';
-import { SpanContext } from './span_context';
+import { LinkContext } from './link_context';
 
 /**
  * A pointer from the current {@link Span} to another span in the same trace or
  * in a different trace. Used (for example) in batching operations, where a
  * single batch handler processes multiple requests from different traces.
  */
 export interface Link {
-  /** The {@link SpanContext} of a linked span. */
-  spanContext: SpanContext;
+  /** The {@link LinkContext} of a linked span. */
+  context: LinkContext;
   /** A set of {@link Attributes} on the link. */
   attributes?: Attributes;
 }
diff --git a/api/src/trace/link_context.ts b/api/src/trace/link_context.ts
@@ -0,0 +1,22 @@
+/*!
+ * Copyright 2020, OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { SpanContext } from './span_context';
+
+/**
+ * A pointer to another span.
+ */
+export type LinkContext = Pick<SpanContext, 'traceId' | 'spanId'>;
diff --git a/api/src/trace/span_context.ts b/api/src/trace/span_context.ts
@@ -47,9 +47,9 @@ export interface SpanContext {
    * caller may have recorded trace data. A caller who does not record trace
    * data out-of-band leaves this flag unset.
    *
-   * SAMPLED = 0x1 and UNSAMPLED = 0x0;
+   * SAMPLED = 0x1 and NONE = 0x0;
    */
-  traceFlags?: TraceFlags;
+  traceFlags: TraceFlags;
   /**
    * Tracing-system-specific info to propagate.
    *

diff --git a/api/src/trace/trace_flags.ts b/api/src/trace/trace_flags.ts
@@ -20,8 +20,8 @@
  * whether a Span should be traced. It is implemented as a bitmask.
  */
 export enum TraceFlags {
-  /** Bit to represent whether trace is unsampled in trace flags. */
-  UNSAMPLED = 0x0,
+  /** Represents no flag set. */
+  NONE = 0x0,
   /** Bit to represent whether trace is sampled in trace flags. */
-  SAMPLED = 0x1,
+  SAMPLED = 0x1 << 0,
 }
diff --git a/api/test/api/api.test.ts b/api/test/api/api.test.ts
@@ -42,7 +42,7 @@ describe('API', () => {
     const spanContext = {
       traceId: 'd4cda95b652f4a1592b449d5929fda1b',
       spanId: '6e0c63257de34c92',
-      traceFlags: TraceFlags.UNSAMPLED,
+      traceFlags: TraceFlags.NONE,
     };
     const dummySpan = new NoopSpan(spanContext);
 

diff --git a/api/test/noop-implementations/noop-span.test.ts b/api/test/noop-implementations/noop-span.test.ts
@@ -39,18 +39,6 @@ describe('NoopSpan', () => {
     span.addEvent('sent');
     span.addEvent('sent', { id: '42', key: 'value' });
 
-    span.addLink({
-      traceId: 'd4cda95b652f4a1592b449d5929fda1b',
-      spanId: '6e0c63257de34c92',
-    });
-    span.addLink(
-      {
-        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
-        spanId: '6e0c63257de34c92',
-      },
-      { id: '42', key: 'value' }
-    );
-
     span.setStatus({ code: CanonicalCode.CANCELLED });
 
     span.updateName('my-span');
@@ -59,7 +47,7 @@ describe('NoopSpan', () => {
     assert.deepStrictEqual(span.context(), {
       traceId: INVALID_TRACE_ID,
       spanId: INVALID_SPAN_ID,
-      traceFlags: TraceFlags.UNSAMPLED,
+      traceFlags: TraceFlags.NONE,
     });
     span.end();
   });