[instrumentation] - Add integration tests (Azure#20462)

### Packages impacted by this PR
@azure/opentelemetry-instrumentation-azure-sdk

### Issues associated with this PR
Resolves Azure#18738

### Describe the problem that is addressed by this PR
This PR solves the following:
1. I would like to have an end-to-end test that uses our pipeline and ensures
tracing works as expected. The idea here is to ensure we do not regress in the
future when making core changes. I added a node-only integration test that uses
core-rest-pipeline and a fake client.
1. We currently maintain a TestTracer, TestTracerProvider, and TestSpan
independently in multiple places. OpenTelemetry already provides testing utils
in their base package. I replaced our Test* objects with an InMemorySpanExporter
which allows us to grab all exported spans and inspect them.
1. In-passing, I realized that for a bit there could be a drift between client
packages that use the new naming conventions and core packages that use the old
naming conventions to pull az.namespace into a span. While it doesn't feel
appropriate to add compatibility code in our core packages, this package will be
in beta for quite some time so I think it's fine to add it here temporarily.

Author: maorleger

common/config/rush/pnpm-lock.yaml

@@ -6,7 +6,8 @@
   "main": "dist/index.js",
   "module": "dist-esm/src/index.js",
   "browser": {
-    "./dist-esm/src/instrumentation.js": "./dist-esm/src/instrumentation.browser.js"
+    "./dist-esm/src/instrumentation.js": "./dist-esm/src/instrumentation.browser.js",
+    "./dist-esm/test/public/util/tracerProvider.js": "./dist-esm/test/public/util/tracerProvider.browser.js"
   },
   "//metadata": {
     "constantPaths": [
@@ -86,9 +87,12 @@
   },
   "devDependencies": {
     "@azure-tools/test-recorder": "^1.0.0",
+    "@azure/core-rest-pipeline": "^1.5.1",
     "@azure/dev-tool": "^1.0.0",
     "@azure/eslint-plugin-azure-sdk": "^3.0.0",
     "@microsoft/api-extractor": "^7.18.11",
+    "@opentelemetry/sdk-trace-base": "^1.0.1",
+    "@opentelemetry/sdk-trace-node": "^1.0.1",
     "@types/chai": "^4.1.6",
     "@types/mocha": "^7.0.2",
     "@types/node": "^12.0.0",

sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/package.json

@@ -25,7 +25,15 @@ export class OpenTelemetryInstrumenter implements Instrumenter {
       .getTracer(spanOptions.packageName, spanOptions.packageVersion)
       .startSpan(name, toSpanOptions(spanOptions));
 
-    const ctx = spanOptions?.tracingContext || context.active();
+    let ctx = spanOptions?.tracingContext || context.active();
+
+    // COMPAT: remove when core-rest-pipeline has upgraded to core-tracing 1.0
+    // https://github.com/Azure/azure-sdk-for-js/issues/20567
+    const newNamespaceKey = Symbol.for("@azure/core-tracing namespace");
+    const oldNamespaceKey = Symbol.for("az.namespace");
+    if (!ctx.getValue(oldNamespaceKey) && ctx.getValue(newNamespaceKey)) {
+      ctx = ctx.setValue(oldNamespaceKey, ctx.getValue(newNamespaceKey));
+    }
 
     return {
       span: new OpenTelemetrySpanWrapper(span),

sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts

@@ -0,0 +1,53 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { SpanKind, SpanStatusCode } from "@opentelemetry/api";
+import { TestClient, tracingClientAttributes } from "../../public/util/testClient";
+
+import { assert } from "chai";
+import { inMemoryExporter } from "../../public/util/setup";
+
+describe("instrumentation end-to-end tests", () => {
+  beforeEach(() => {
+    inMemoryExporter.reset();
+  });
+
+  // This is node-only since we use the BasicTracerProvider in the browser
+  // which does not set up a context manager. Altenatively we can always pull in
+  // @opentelemetry/sdk-trace-web but it did not feel necessary at this time.
+  describe("with a configured client", () => {
+    it("works when using withSpan", async () => {
+      await new TestClient().exampleOperation();
+      const spans = inMemoryExporter.getFinishedSpans();
+      assert.lengthOf(spans, 3);
+      const [coreRestPipeline, inner, outer] = spans;
+
+      // Check parenting chain
+      assert.equal(coreRestPipeline.parentSpanId, inner.spanContext().spanId);
+      assert.equal(inner.parentSpanId, outer.spanContext().spanId);
+      assert.notExists(outer.parentSpanId);
+
+      // Check default span kind
+      assert.equal(outer.kind, SpanKind.INTERNAL);
+
+      // Check specified span kind
+      assert.equal(inner.kind, SpanKind.CLIENT);
+
+      // Check status
+      assert.deepEqual(coreRestPipeline.status, { code: SpanStatusCode.OK });
+      assert.deepEqual(inner.status, { code: SpanStatusCode.OK });
+      assert.deepEqual(outer.status, { code: SpanStatusCode.OK });
+
+      // Check instrumentationLibrary
+      assert.deepEqual(outer.instrumentationLibrary, {
+        name: tracingClientAttributes.packageName,
+        version: tracingClientAttributes.packageVersion,
+      });
+
+      // Check attributes on all spans
+      assert.equal(coreRestPipeline.attributes["az.namespace"], tracingClientAttributes.namespace);
+      assert.equal(inner.attributes["az.namespace"], tracingClientAttributes.namespace);
+      assert.equal(outer.attributes["az.namespace"], tracingClientAttributes.namespace);
+    });
+  });
+});

sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/instrumentation.spec.ts

@@ -4,16 +4,16 @@
 import { OpenTelemetryInstrumenter, propagator } from "../../src/instrumenter";
 import { SpanKind, context, trace } from "@opentelemetry/api";
 import { TracingSpan, TracingSpanKind } from "@azure/core-tracing";
-import { resetTracer, setTracer } from "./util/testTracerProvider";
+
 import { Context } from "mocha";
 import { OpenTelemetrySpanWrapper } from "../../src/spanWrapper";
-import { TestSpan } from "./util/testSpan";
-import { TestTracer } from "./util/testTracer";
+import { Span } from "@opentelemetry/sdk-trace-base";
 import { assert } from "chai";
+import { inMemoryExporter } from "./util/setup";
 import sinon from "sinon";
 
-function unwrap(span: TracingSpan): TestSpan {
-  return (span as OpenTelemetrySpanWrapper).unwrap() as TestSpan;
+function unwrap(span: TracingSpan): Span {
+  return (span as OpenTelemetrySpanWrapper).unwrap() as Span;
 }
 
 describe("OpenTelemetryInstrumenter", () => {
@@ -26,7 +26,7 @@ describe("OpenTelemetryInstrumenter", () => {
 
     it("uses the passed in context if it exists", () => {
       const propagationSpy = sinon.spy(propagator);
-      const span = new TestTracer().startSpan("test");
+      const span = trace.getTracer("test").startSpan("test");
       const tracingContext = trace.setSpan(context.active(), span);
       instrumenter.createRequestHeaders(tracingContext);
       assert.isTrue(propagationSpy.inject.calledWith(tracingContext));
@@ -40,24 +40,20 @@ describe("OpenTelemetryInstrumenter", () => {
     });
   });
 
-  // TODO: the following still uses existing test support for OTel.
-  // Once the new APIs are available we should move away from those.
   describe("#startSpan", () => {
-    let tracer: TestTracer;
     const packageName = "test-package";
     const packageVersion = "test-version";
-    beforeEach(() => {
-      tracer = setTracer(tracer);
-    });
 
-    afterEach(() => {
-      resetTracer();
+    beforeEach(() => {
+      inMemoryExporter.reset();
     });
 
     it("returns a newly started TracingSpan", () => {
       const { span } = instrumenter.startSpan("test", { packageName, packageVersion });
+      span.end();
       const otSpan = unwrap(span);
-      assert.equal(otSpan, tracer.getActiveSpans()[0]);
+      assert.lengthOf(inMemoryExporter.getFinishedSpans(), 1);
+      assert.equal(otSpan, inMemoryExporter.getFinishedSpans()[0]);
       assert.equal(otSpan.kind, SpanKind.INTERNAL);
     });
 
@@ -90,6 +86,19 @@ describe("OpenTelemetryInstrumenter", () => {
 
         assert.equal(trace.getSpan(tracingContext), unwrap(span));
       });
+
+      it("adds az.namespace as a context attribute for compatibility", async () => {
+        const currentContext = context
+          .active()
+          .setValue(Symbol.for("@azure/core-tracing namespace"), "test-namespace");
+
+        const { tracingContext } = instrumenter.startSpan("test", {
+          tracingContext: currentContext,
+          packageName,
+        });
+
+        assert.equal(tracingContext.getValue(Symbol.for("az.namespace")), "test-namespace");
+      });
     });
 
     describe("when a context is not provided", () => {
@@ -208,7 +217,7 @@ describe("OpenTelemetryInstrumenter", () => {
 
       // Function syntax
       instrumenter.withContext(context.active(), function (this: any) {
-        assert.isUndefined(this);
+        assert.notExists(this);
       });
       instrumenter.withContext(
         context.active(),