Respect sampled bit in probability sampler (#798)

open-telemetry · Feb 20, 2020 · 5228dfa · kintel · Jul 2, 2020 · 5228dfa
1 parent 5fc9576
commit 5228dfa
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 32 deletions.
diff --git a/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts b/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Sampler, SpanContext } from '@opentelemetry/api';
+import { Sampler, SpanContext, TraceFlags } from '@opentelemetry/api';
 
 /** Sampler that samples a given fraction of traces. */
 export class ProbabilitySampler implements Sampler {
@@ -23,6 +23,12 @@ export class ProbabilitySampler implements Sampler {
   }
 
   shouldSample(parentContext?: SpanContext) {
+    // Respect the parent sampling decision if there is one
+    if (parentContext && typeof parentContext.traceFlags !== 'undefined') {
+      return (
+        (TraceFlags.SAMPLED & parentContext.traceFlags) === TraceFlags.SAMPLED
+      );
+    }
     if (this._probability >= 1.0) return true;
     else if (this._probability <= 0) return false;
     return Math.random() < this._probability;

diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts
@@ -14,13 +14,12 @@
  * limitations under the License.
  */
 
-import { TraceFlags } from '@opentelemetry/api';
 import { unrefTimer } from '@opentelemetry/core';
-import { SpanProcessor } from '../SpanProcessor';
-import { SpanExporter } from './SpanExporter';
 import { Span } from '../Span';
-import { ReadableSpan } from './ReadableSpan';
+import { SpanProcessor } from '../SpanProcessor';
 import { BufferConfig } from '../types';
+import { ReadableSpan } from './ReadableSpan';
+import { SpanExporter } from './SpanExporter';
 
 const DEFAULT_BUFFER_SIZE = 100;
 const DEFAULT_BUFFER_TIMEOUT_MS = 20_000;
@@ -56,7 +55,6 @@ export class BatchSpanProcessor implements SpanProcessor {
   onStart(span: Span): void {}
 
   onEnd(span: Span): void {
-    if (span.context().traceFlags !== TraceFlags.SAMPLED) return;
     this._addToBuffer(span.toReadableSpan());
   }
 

diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts
@@ -14,10 +14,9 @@
  * limitations under the License.
  */
 
-import { TraceFlags } from '@opentelemetry/api';
+import { Span } from '../Span';
 import { SpanProcessor } from '../SpanProcessor';
 import { SpanExporter } from './SpanExporter';
-import { Span } from '../Span';
 
 /**
  * An implementation of the {@link SpanProcessor} that converts the {@link Span}
@@ -32,7 +31,6 @@ export class SimpleSpanProcessor implements SpanProcessor {
   onStart(span: Span): void {}
 
   onEnd(span: Span): void {
-    if (span.context().traceFlags !== TraceFlags.SAMPLED) return;
     this._exporter.export([span.toReadableSpan()], () => {});
   }
 

diff --git a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts
@@ -14,15 +14,15 @@
  * limitations under the License.
  */
 
+import { ALWAYS_SAMPLER } from '@opentelemetry/core';
 import * as assert from 'assert';
 import * as sinon from 'sinon';
 import {
-  Span,
   BasicTracerProvider,
-  InMemorySpanExporter,
   BatchSpanProcessor,
+  InMemorySpanExporter,
+  Span,
 } from '../../src';
-import { NEVER_SAMPLER, ALWAYS_SAMPLER, NoopLogger } from '@opentelemetry/core';
 
 function createSampledSpan(spanName: string): Span {
   const tracer = new BasicTracerProvider({
@@ -33,16 +33,6 @@ function createSampledSpan(spanName: string): Span {
   return span as Span;
 }
 
-function createUnSampledSpan(spanName: string): Span {
-  const tracer = new BasicTracerProvider({
-    sampler: NEVER_SAMPLER,
-    logger: new NoopLogger(),
-  }).getTracer('default');
-  const span = tracer.startSpan(spanName, { isRecording: false });
-  span.end();
-  return span as Span;
-}
-
 describe('BatchSpanProcessor', () => {
   const name = 'span-name';
   const defaultBufferConfig = {
@@ -97,16 +87,6 @@ describe('BatchSpanProcessor', () => {
       assert.strictEqual(exporter.getFinishedSpans().length, 0);
     });
 
-    it('should not export the unsampled spans', () => {
-      const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);
-      for (let i = 0; i < defaultBufferConfig.bufferSize * 2; i++) {
-        const span = createUnSampledSpan(`${name}_${i}`);
-        processor.onEnd(span);
-        assert.strictEqual(exporter.getFinishedSpans().length, 0);
-      }
-      processor.shutdown();
-    });
-
     it('should force flush when timeout exceeded', done => {
       const clock = sinon.useFakeTimers();
       const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);