[tracing] Moving spanOptions to `tracingOptions` in PipelineRequest (#14094)

Creating a `tracingOptions` property on PipelineRequest and using createSpan internally rather than starting spans directly.

Also, allows callers to use createSpanFunction() without a package prefix or namespace (needed for core-http and lower level libraries).

sdk/core/core-client/src/serviceClient.ts

@@ -164,8 +164,8 @@ export class ServiceClient {
         request.abortSignal = options.abortSignal;
       }
 
-      if (options.tracingOptions?.spanOptions) {
-        request.spanOptions = options.tracingOptions.spanOptions;
+      if (options.tracingOptions) {
+        request.tracingOptions = options.tracingOptions;
       }
     }
 

sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md

@@ -6,7 +6,7 @@
 
 import { AbortSignalLike } from '@azure/abort-controller';
 import { Debugger } from '@azure/logger';
-import { SpanOptions } from '@azure/core-tracing';
+import { OperationTracingOptions } from '@azure/core-tracing';
 import { TokenCredential } from '@azure/core-auth';
 
 // @public
@@ -162,9 +162,9 @@ export interface PipelineRequest {
     onUploadProgress?: (progress: TransferProgressEvent) => void;
     proxySettings?: ProxySettings;
     requestId: string;
-    spanOptions?: SpanOptions;
     streamResponseStatusCodes?: Set<number>;
     timeout: number;
+    tracingOptions?: OperationTracingOptions;
     url: string;
     withCredentials: boolean;
 }
@@ -181,9 +181,9 @@ export interface PipelineRequestOptions {
     onUploadProgress?: (progress: TransferProgressEvent) => void;
     proxySettings?: ProxySettings;
     requestId?: string;
-    spanOptions?: SpanOptions;
     streamResponseStatusCodes?: Set<number>;
     timeout?: number;
+    tracingOptions?: OperationTracingOptions;
     url: string;
     withCredentials?: boolean;
 }

sdk/core/core-rest-pipeline/src/interfaces.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 import { AbortSignalLike } from "@azure/abort-controller";
-import { SpanOptions } from "@azure/core-tracing";
+import { OperationTracingOptions } from "@azure/core-tracing";
 
 // eslint-disable-next-line @azure/azure-sdk/ts-no-namespaces
 declare global {
@@ -149,9 +149,9 @@ export interface PipelineRequest {
   abortSignal?: AbortSignalLike;
 
   /**
-   * Options used to create a span when tracing is enabled.
+   * Tracing options to use for any created Spans.
    */
-  spanOptions?: SpanOptions;
+  tracingOptions?: OperationTracingOptions;
 
   /**
    * Callback which fires upon upload progress.

sdk/core/core-rest-pipeline/src/pipelineRequest.ts

@@ -13,7 +13,7 @@ import {
 import { createHttpHeaders } from "./httpHeaders";
 import { AbortSignalLike } from "@azure/abort-controller";
 import { generateUuid } from "./util/uuid";
-import { SpanOptions } from "@azure/core-tracing";
+import { OperationTracingOptions } from "@azure/core-tracing";
 
 /**
  * Settings to initialize a request.
@@ -86,7 +86,7 @@ export interface PipelineRequestOptions {
   /**
    * Options used to create a span when tracing is enabled.
    */
-  spanOptions?: SpanOptions;
+  tracingOptions?: OperationTracingOptions;
 
   /**
    * Callback which fires upon upload progress.
@@ -110,7 +110,7 @@ class PipelineRequestImpl implements PipelineRequest {
   public disableKeepAlive: boolean;
   public abortSignal?: AbortSignalLike;
   public requestId: string;
-  public spanOptions?: SpanOptions;
+  public tracingOptions?: OperationTracingOptions;
   public onUploadProgress?: (progress: TransferProgressEvent) => void;
   public onDownloadProgress?: (progress: TransferProgressEvent) => void;
 
@@ -126,7 +126,7 @@ class PipelineRequestImpl implements PipelineRequest {
     this.streamResponseStatusCodes = options.streamResponseStatusCodes;
     this.withCredentials = options.withCredentials ?? false;
     this.abortSignal = options.abortSignal;
-    this.spanOptions = options.spanOptions;
+    this.tracingOptions = options.tracingOptions;
     this.onUploadProgress = options.onUploadProgress;
     this.onDownloadProgress = options.onDownloadProgress;
     this.requestId = options.requestId || generateUuid();

sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts

@@ -48,9 +48,7 @@ export function bearerTokenAuthenticationPolicy(
     async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
       const token = await getToken({
         abortSignal: request.abortSignal,
-        tracingOptions: {
-          spanOptions: request.spanOptions
-        }
+        tracingOptions: request.tracingOptions
       });
       request.headers.set("Authorization", `Bearer ${token}`);
       return next(request);

sdk/core/core-rest-pipeline/src/policies/tracingPolicy.ts

@@ -1,13 +1,22 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { getTracer, getTraceParentHeader } from "@azure/core-tracing";
-import { SpanOptions, SpanKind } from "@opentelemetry/api";
+import {
+  getTraceParentHeader,
+  OperationTracingOptions,
+  createSpanFunction
+} from "@azure/core-tracing";
+import { SpanKind } from "@opentelemetry/api";
 import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { URL } from "../util/url";
 import { getUserAgentValue } from "../util/userAgent";
 
+const createSpan = createSpanFunction({
+  packagePrefix: "",
+  namespace: ""
+});
+
 /**
  * The programmatic identifier of the tracingPolicy.
  */
@@ -37,19 +46,24 @@ export function tracingPolicy(options: TracingPolicyOptions = {}): PipelinePolic
   return {
     name: tracingPolicyName,
     async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
-      if (!request.spanOptions || !request.spanOptions.parent) {
+      if (!request.tracingOptions?.spanOptions || !request.tracingOptions.spanOptions?.parent) {
         return next(request);
       }
 
       // create a new span
-      const tracer = getTracer();
-      const spanOptions: SpanOptions = {
-        ...request.spanOptions,
-        kind: SpanKind.CLIENT
+      const tracingOptions: OperationTracingOptions = {
+        ...request.tracingOptions,
+        spanOptions: {
+          ...request.tracingOptions.spanOptions,
+          kind: SpanKind.CLIENT
+        }
       };
+
       const url = new URL(request.url);
       const path = url.pathname || "/";
-      const span = tracer.startSpan(path, spanOptions);
+
+      const { span } = createSpan(path, { tracingOptions });
+
       span.setAttributes({
         "http.method": request.method,
         "http.url": request.url,

sdk/core/core-rest-pipeline/test/bearerTokenAuthenticationPolicy.spec.ts

@@ -41,7 +41,7 @@ describe("BearerTokenAuthenticationPolicy", function() {
     assert(
       fakeGetToken.calledWith(tokenScopes, {
         abortSignal: undefined,
-        tracingOptions: { spanOptions: undefined }
+        tracingOptions: undefined
       }),
       "fakeGetToken called incorrectly."
     );

sdk/core/core-rest-pipeline/test/tracingPolicy.spec.ts

@@ -118,8 +118,10 @@ describe("tracingPolicy", function() {
 
     const request = createPipelineRequest({
       url: "https://bing.com",
-      spanOptions: {
-        parent: ROOT_SPAN.context()
+      tracingOptions: {
+        spanOptions: {
+          parent: ROOT_SPAN.context()
+        }
       }
     });
     const response: PipelineResponse = {
@@ -155,8 +157,10 @@ describe("tracingPolicy", function() {
 
     const request = createPipelineRequest({
       url: "https://bing.com",
-      spanOptions: {
-        parent: ROOT_SPAN.context()
+      tracingOptions: {
+        spanOptions: {
+          parent: ROOT_SPAN.context()
+        }
       }
     });
     const response: PipelineResponse = {
@@ -192,8 +196,10 @@ describe("tracingPolicy", function() {
 
     const request = createPipelineRequest({
       url: "https://bing.com",
-      spanOptions: {
-        parent: ROOT_SPAN.context()
+      tracingOptions: {
+        spanOptions: {
+          parent: ROOT_SPAN.context()
+        }
       }
     });
     const response: PipelineResponse = {
@@ -229,8 +235,10 @@ describe("tracingPolicy", function() {
 
     const request = createPipelineRequest({
       url: "https://bing.com",
-      spanOptions: {
-        parent: ROOT_SPAN.context()
+      tracingOptions: {
+        spanOptions: {
+          parent: ROOT_SPAN.context()
+        }
       }
     });
     const response: PipelineResponse = {
@@ -267,8 +275,10 @@ describe("tracingPolicy", function() {
 
     const request = createPipelineRequest({
       url: "https://bing.com",
-      spanOptions: {
-        parent: ROOT_SPAN.context()
+      tracingOptions: {
+        spanOptions: {
+          parent: ROOT_SPAN.context()
+        }
       }
     });
     const response: PipelineResponse = {

sdk/core/core-tracing/src/createSpan.ts

@@ -13,11 +13,15 @@ import { OperationTracingOptions } from "./interfaces";
  */
 export interface CreateSpanFunctionArgs {
   /**
-   * Package name prefix
+   * Package name prefix.
+   *
+   * NOTE: if this is empty no prefix will be applied to created Span names.
    */
   packagePrefix: string;
   /**
    * Service namespace
+   *
+   * NOTE: if this is empty no `az.namespace` attribute will be added to created Spans.
    */
   namespace: string;
 }
@@ -52,12 +56,16 @@ export function createSpanFunction(args: CreateSpanFunctionArgs) {
       ...tracingOptions.spanOptions
     };
 
-    const span = tracer.startSpan(`${args.packagePrefix}.${operationName}`, spanOptions);
+    const spanName = args.packagePrefix ? `${args.packagePrefix}.${operationName}` : operationName;
+    const span = tracer.startSpan(spanName, spanOptions);
 
-    span.setAttribute("az.namespace", args.namespace);
+    if (args.namespace) {
+      span.setAttribute("az.namespace", args.namespace);
+    }
 
     let newSpanOptions = tracingOptions.spanOptions || {};
-    if (span.isRecording()) {
+
+    if (span.isRecording() && args.namespace) {
       newSpanOptions = {
         ...tracingOptions.spanOptions,
         parent: span.context(),

sdk/core/core-tracing/test/createSpan.spec.ts

@@ -108,6 +108,59 @@ describe("createSpan", () => {
     assert.deepEqual(updatedOptions, expected);
   });
 
+  it("namespace and packagePrefix can be empty (and thus ignored)", () => {
+    const tracer = new TestTracer();
+
+    const testSpan = new TestSpan(
+      tracer,
+      "testing",
+      { traceId: "", spanId: "", traceFlags: TraceFlags.NONE },
+      SpanKind.INTERNAL // this isn't used by anything in our test.
+    );
+
+    const setAttributeSpy = sinon.spy(testSpan, "setAttribute");
+    const startSpanStub = sinon.stub(tracer, "startSpan");
+    startSpanStub.returns(testSpan);
+    setTracer(tracer);
+
+    const cf = createSpanFunction({
+      namespace: "",
+      packagePrefix: ""
+    });
+
+    const { span, updatedOptions } = cf("myVerbatimOperationName", {
+      tracingOptions: {
+        spanOptions: {
+          attributes: {
+            testAttribute: "testValue"
+          }
+        }
+      }
+    });
+
+    assert.ok(span);
+    assert.ok(startSpanStub.called);
+
+    const [name] = startSpanStub.firstCall.args;
+
+    assert.equal(
+      name,
+      "myVerbatimOperationName",
+      "operation name should be exactly as passed in (no prefix)"
+    );
+    assert.ok(!setAttributeSpy.called, "When the namespace is undefined it should not be set");
+
+    assert.deepEqual(updatedOptions, {
+      tracingOptions: {
+        spanOptions: {
+          attributes: {
+            testAttribute: "testValue"
+          }
+        }
+      }
+    });
+  });
+
   afterEach(() => {
     sinon.restore();
   });

sdk/tables/data-tables/src/TableBatch.ts

@@ -146,7 +146,7 @@ export class TableBatchImpl implements TableBatch {
       method: "POST",
       body,
       headers: createHttpHeaders(headers),
-      spanOptions: updatedOptions.tracingOptions?.spanOptions
+      tracingOptions: updatedOptions.tracingOptions
     });
 
     if (this.credential) {