[eventhubs,service-bus]: re-add legacy tracing options

sdk/eventhub/event-hubs/review/event-hubs.api.md

@@ -9,6 +9,7 @@ import { MessagingError } from '@azure/core-amqp';
 import { OperationTracingOptions } from '@azure/core-tracing';
 import { RetryMode } from '@azure/core-amqp';
 import { RetryOptions } from '@azure/core-amqp';
+import { Span } from '@opentelemetry/api';
 import { SpanContext } from '@opentelemetry/api';
 import { TokenCredential } from '@azure/core-auth';
 import { WebSocketImpl } from 'rhea-promise';
@@ -288,6 +289,8 @@ export { TokenCredential }
 
 // @public
 export interface TryAddOptions {
+    // @deprecated (undocumented)
+    parentSpan?: Span | SpanContext;
     tracingOptions?: OperationTracingOptions;
 }
 

sdk/eventhub/event-hubs/src/diagnostics/tracing.ts

@@ -1,8 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { createSpanFunction, SpanOptions } from "@azure/core-tracing";
+import { createSpanFunction, SpanContext, SpanOptions } from "@azure/core-tracing";
 import { Span, SpanKind } from "@opentelemetry/api";
+import { TryAddOptions } from "../eventDataBatch";
 import { EventHubConnectionConfig } from "../eventhubConnectionConfig";
 import { OperationOptions } from "../util/operationOptions";
 
@@ -52,3 +53,41 @@ export function createMessageSpan(
     kind: SpanKind.PRODUCER
   });
 }
+
+/**
+ * Converts TryAddOptions into the modern shape (OperationOptions) when needed.
+ * (this is something we can eliminate at the next major release of EH _or_ when
+ * we release with the GA version of opentelemetry).
+ *
+ * @internal
+ */
+export function convertTryAddOptionsForCompatibility(tryAddOptions: TryAddOptions): TryAddOptions {
+  /* eslint-disable-next-line @typescript-eslint/ban-ts-comment */
+  // @ts-ignore: parentSpan is deprecated and this is compat code to translate it until we can get rid of it.
+  const possibleParentSpan = tryAddOptions.parentSpan;
+
+  if (!possibleParentSpan) {
+    // assume that the options are already in the modern shape.
+    return tryAddOptions;
+  }
+
+  const convertedOptions: TryAddOptions = {
+    ...tryAddOptions,
+    tracingOptions: {
+      spanOptions: {
+        parent: isSpan(possibleParentSpan) ? possibleParentSpan.context() : possibleParentSpan
+      }
+    }
+  };
+
+  return convertedOptions;
+}
+
+function isSpan(possibleSpan: Span | SpanContext | undefined): possibleSpan is Span {
+  if (possibleSpan == null) {
+    return false;
+  }
+
+  const x = possibleSpan as Span;
+  return typeof x.context === "function";
+}

sdk/eventhub/event-hubs/src/eventDataBatch.ts

@@ -5,9 +5,9 @@ import { EventData, toRheaMessage } from "./eventData";
 import { ConnectionContext } from "./connectionContext";
 import { MessageAnnotations, message, Message as RheaMessage } from "rhea-promise";
 import { throwTypeErrorIfParameterMissing } from "./util/error";
-import { SpanContext } from "@opentelemetry/api";
+import { Span, SpanContext } from "@opentelemetry/api";
 import { TRACEPARENT_PROPERTY, instrumentEventData } from "./diagnostics/instrumentEventData";
-import { createMessageSpan } from "./diagnostics/tracing";
+import { convertTryAddOptionsForCompatibility, createMessageSpan } from "./diagnostics/tracing";
 import { defaultDataTransformer } from "./dataTransformer";
 import { isDefined, isObjectWithProperties } from "./util/typeGuards";
 import { OperationTracingOptions } from "@azure/core-tracing";
@@ -47,6 +47,11 @@ export interface TryAddOptions {
    * The options to use when creating Spans for tracing.
    */
   tracingOptions?: OperationTracingOptions;
+
+  /**
+   * @deprecated Tracing options have been moved to the `tracingOptions` property.
+   */
+  parentSpan?: Span | SpanContext;
 }
 
 /**
@@ -281,6 +286,7 @@ export class EventDataBatchImpl implements EventDataBatch {
    */
   public tryAdd(eventData: EventData, options: TryAddOptions = {}): boolean {
     throwTypeErrorIfParameterMissing(this._context.connectionId, "tryAdd", "eventData", eventData);
+    options = convertTryAddOptionsForCompatibility(options);
 
     // check if the event has already been instrumented
     const previouslyInstrumented = Boolean(

sdk/eventhub/event-hubs/test/internal/sender.spec.ts

@@ -12,8 +12,10 @@ import {
   EventHubConsumerClient,
   EventHubProducerClient,
   EventPosition,
+  OperationOptions,
   ReceivedEventData,
-  SendBatchOptions
+  SendBatchOptions,
+  TryAddOptions
 } from "../../src";
 import {
   EnvVarKeys,
@@ -355,123 +357,134 @@ describe("EventHub Sender", function(): void {
       resetTracer();
     });
 
-    it("will not instrument already instrumented events", async function(): Promise<void> {
-      const { tracer, resetTracer } = setTracerForTest();
+    function legacyOptionsUsingSpanContext(rootSpan: TestSpan): Pick<TryAddOptions, "parentSpan"> {
+      return {
+        parentSpan: rootSpan.context()
+      };
+    }
 
-      const rootSpan = tracer.startSpan("test");
+    function legacyOptionsUsingSpan(rootSpan: TestSpan): Pick<TryAddOptions, "parentSpan"> {
+      return {
+        parentSpan: rootSpan
+      };
+    }
 
-      const list = [
-        { name: "Albert" },
-        {
-          name: "Marie",
-          properties: {
-            [TRACEPARENT_PROPERTY]: "foo"
+    function modernOptions(rootSpan: TestSpan): OperationOptions {
+      return {
+        tracingOptions: {
+          spanOptions: {
+            parent: rootSpan.context()
           }
         }
-      ];
+      };
+    }
 
-      const eventDataBatch = await producerClient.createBatch({
-        partitionId: "0"
-      });
+    [legacyOptionsUsingSpan, legacyOptionsUsingSpanContext, modernOptions].forEach((optionsFn) => {
+      describe(`tracing (${optionsFn.name})`, () => {
+        it("will not instrument already instrumented events", async function(): Promise<void> {
+          const { tracer, resetTracer } = setTracerForTest();
 
-      for (let i = 0; i < 2; i++) {
-        eventDataBatch.tryAdd(
-          { body: `${list[i].name}`, properties: list[i].properties },
-          {
-            tracingOptions: {
-              spanOptions: {
-                parent: rootSpan.context()
+          const rootSpan = tracer.startSpan("test");
+
+          const list = [
+            { name: "Albert" },
+            {
+              name: "Marie",
+              properties: {
+                [TRACEPARENT_PROPERTY]: "foo"
               }
             }
+          ];
+
+          const eventDataBatch = await producerClient.createBatch({
+            partitionId: "0"
+          });
+
+          for (let i = 0; i < 2; i++) {
+            eventDataBatch.tryAdd(
+              { body: `${list[i].name}`, properties: list[i].properties },
+              optionsFn(rootSpan)
+            );
           }
-        );
-      }
-      await producerClient.sendBatch(eventDataBatch);
-      rootSpan.end();
+          await producerClient.sendBatch(eventDataBatch);
+          rootSpan.end();
 
-      const rootSpans = tracer.getRootSpans();
-      rootSpans.length.should.equal(2, "Should only have two root spans.");
-      rootSpans[0].should.equal(rootSpan, "The root span should match what was passed in.");
+          const rootSpans = tracer.getRootSpans();
+          rootSpans.length.should.equal(2, "Should only have two root spans.");
+          rootSpans[0].should.equal(rootSpan, "The root span should match what was passed in.");
 
-      const expectedGraph: SpanGraph = {
-        roots: [
-          {
-            name: rootSpan.name,
-            children: [
+          const expectedGraph: SpanGraph = {
+            roots: [
               {
-                name: "Azure.EventHubs.message",
-                children: []
+                name: rootSpan.name,
+                children: [
+                  {
+                    name: "Azure.EventHubs.message",
+                    children: []
+                  }
+                ]
               }
             ]
-          }
-        ]
-      };
+          };
 
-      tracer.getSpanGraph(rootSpan.context().traceId).should.eql(expectedGraph);
-      tracer.getActiveSpans().length.should.equal(0, "All spans should have had end called.");
-      resetTracer();
-    });
+          tracer.getSpanGraph(rootSpan.context().traceId).should.eql(expectedGraph);
+          tracer.getActiveSpans().length.should.equal(0, "All spans should have had end called.");
+          resetTracer();
+        });
 
-    it("will support tracing batch and send", async function(): Promise<void> {
-      const { tracer, resetTracer } = setTracerForTest();
+        it("will support tracing batch and send", async function(): Promise<void> {
+          const { tracer, resetTracer } = setTracerForTest();
 
-      const rootSpan = tracer.startSpan("root");
+          const rootSpan = tracer.startSpan("root");
 
-      const list = [{ name: "Albert" }, { name: "Marie" }];
+          const list = [{ name: "Albert" }, { name: "Marie" }];
 
-      const eventDataBatch = await producerClient.createBatch({
-        partitionId: "0"
-      });
-      for (let i = 0; i < 2; i++) {
-        eventDataBatch.tryAdd(
-          { body: `${list[i].name}` },
-          {
+          const eventDataBatch = await producerClient.createBatch({
+            partitionId: "0"
+          });
+          for (let i = 0; i < 2; i++) {
+            eventDataBatch.tryAdd({ body: `${list[i].name}` }, optionsFn(rootSpan));
+          }
+          await producerClient.sendBatch(eventDataBatch, {
             tracingOptions: {
               spanOptions: {
                 parent: rootSpan.context()
               }
             }
-          }
-        );
-      }
-      await producerClient.sendBatch(eventDataBatch, {
-        tracingOptions: {
-          spanOptions: {
-            parent: rootSpan.context()
-          }
-        }
-      });
-      rootSpan.end();
+          });
+          rootSpan.end();
 
-      const rootSpans = tracer.getRootSpans();
-      rootSpans.length.should.equal(1, "Should only have one root span.");
-      rootSpans[0].should.equal(rootSpan, "The root span should match what was passed in.");
+          const rootSpans = tracer.getRootSpans();
+          rootSpans.length.should.equal(1, "Should only have one root span.");
+          rootSpans[0].should.equal(rootSpan, "The root span should match what was passed in.");
 
-      const expectedGraph: SpanGraph = {
-        roots: [
-          {
-            name: rootSpan.name,
-            children: [
-              {
-                name: "Azure.EventHubs.message",
-                children: []
-              },
-              {
-                name: "Azure.EventHubs.message",
-                children: []
-              },
+          const expectedGraph: SpanGraph = {
+            roots: [
               {
-                name: "Azure.EventHubs.send",
-                children: []
+                name: rootSpan.name,
+                children: [
+                  {
+                    name: "Azure.EventHubs.message",
+                    children: []
+                  },
+                  {
+                    name: "Azure.EventHubs.message",
+                    children: []
+                  },
+                  {
+                    name: "Azure.EventHubs.send",
+                    children: []
+                  }
+                ]
               }
             ]
-          }
-        ]
-      };
+          };
 
-      tracer.getSpanGraph(rootSpan.context().traceId).should.eql(expectedGraph);
-      tracer.getActiveSpans().length.should.equal(0, "All spans should have had end called.");
-      resetTracer();
+          tracer.getSpanGraph(rootSpan.context().traceId).should.eql(expectedGraph);
+          tracer.getActiveSpans().length.should.equal(0, "All spans should have had end called.");
+          resetTracer();
+        });
+      });
     });
 
     it("with partition key should be sent successfully.", async function(): Promise<void> {

sdk/servicebus/service-bus/review/service-bus.api.md

@@ -18,6 +18,7 @@ import { PipelineOptions } from '@azure/core-http';
 import { RetryMode } from '@azure/core-amqp';
 import { RetryOptions } from '@azure/core-amqp';
 import { ServiceClient } from '@azure/core-http';
+import { Span } from '@opentelemetry/api';
 import { SpanContext } from '@opentelemetry/api';
 import { TokenCredential } from '@azure/core-auth';
 import { TokenType } from '@azure/core-amqp';
@@ -567,6 +568,8 @@ export interface TopicRuntimeProperties {
 
 // @public
 export interface TryAddOptions {
+    // @deprecated (undocumented)
+    parentSpan?: Span | SpanContext;
     tracingOptions?: OperationTracingOptions;
 }