From 750203054202a8a5872b03eb95c1b2be3aa07e55 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 12:44:30 -0800 Subject: [PATCH 01/13] [instrumentation] - Suppress tracing using environment variables --- .../src/instrumenter.ts | 36 ++++++- .../src/transformations.ts | 13 +++ .../test/public/node/instrumenter.spec.ts | 97 +++++++++++++++++++ .../test/public/util/setup.ts | 3 +- 4 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts index bcbc6e7c98b0..1efca24778c0 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts @@ -7,11 +7,11 @@ import { TracingContext, TracingSpan, } from "@azure/core-tracing"; +import { W3CTraceContextPropagator, suppressTracing } from "@opentelemetry/core"; import { context, defaultTextMapGetter, defaultTextMapSetter, trace } from "@opentelemetry/api"; +import { toBoolean, toSpanOptions } from "./transformations"; import { OpenTelemetrySpanWrapper } from "./spanWrapper"; -import { W3CTraceContextPropagator } from "@opentelemetry/core"; -import { toSpanOptions } from "./transformations"; // While default propagation is user-configurable, Azure services always use the W3C implementation. export const propagator = new W3CTraceContextPropagator(); @@ -21,11 +21,14 @@ export class OpenTelemetryInstrumenter implements Instrumenter { name: string, spanOptions: InstrumenterSpanOptions ): { span: TracingSpan; tracingContext: TracingContext } { + let ctx = spanOptions?.tracingContext || context.active(); + if (isTracingDisabled(name)) { + ctx = suppressTracing(ctx); + } + const span = trace .getTracer(spanOptions.packageName, spanOptions.packageVersion) - .startSpan(name, toSpanOptions(spanOptions)); - - let ctx = spanOptions?.tracingContext || context.active(); + .startSpan(name, toSpanOptions(spanOptions), ctx); // COMPAT: remove when core-rest-pipeline has upgraded to core-tracing 1.0 // https://github.com/Azure/azure-sdk-for-js/issues/20567 @@ -70,3 +73,26 @@ export class OpenTelemetryInstrumenter implements Instrumenter { return headers; } } + +/** + * Checks whether tracing is disabled by checking the relevant environment variables. + * + * @returns - `true` if tracing is disabled, `false` otherwise. + * + * @internal + */ +export function isTracingDisabled(spanName?: string): boolean { + if (typeof process === "undefined") { + return false; + } + + const tracingDisabledGlobally = toBoolean(process.env.AZURE_TRACING_DISABLED); + const httpSpansDisabled = toBoolean(process.env.AZURE_HTTP_TRACING_DISABLED); + + const conditions = [ + tracingDisabledGlobally, + spanName && spanName.startsWith("HTTP") && httpSpansDisabled, + ]; + + return conditions.some(Boolean); +} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts index 2bc9ad8926c0..2a7d0266b377 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts @@ -93,3 +93,16 @@ export function toSpanOptions(spanOptions?: InstrumenterSpanOptions): SpanOption links, }; } + +/** + * A helper function that converts a raw string to its boolean representation. + * + * This function treats "false" and "0" as false, but otherwise uses truthy-ness of the value. + * + * @param rawValue - The raw string value to convert to boolean. + * @returns - true if the {@link rawValue} is truthy, false otherwise. + */ +export function toBoolean(rawValue?: string): boolean { + const valueWithDefault = (rawValue ?? "").toLowerCase(); + return valueWithDefault !== "false" && valueWithDefault !== "0" && Boolean(valueWithDefault); +} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts new file mode 100644 index 000000000000..c6ffe3a3edf2 --- /dev/null +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { OpenTelemetryInstrumenter, isTracingDisabled } from "../../../src/instrumenter"; + +import { assert } from "chai"; +import { inMemoryExporter } from "../util/setup"; +import { isTracingSuppressed } from "@opentelemetry/core"; + +describe("OpenTelemetryInstrumenter (node)", () => { + const packageName = "test-package"; + + beforeEach(() => { + inMemoryExporter.reset(); + }); + + const instrumenter = new OpenTelemetryInstrumenter(); + + describe("startSpan", () => { + describe("when AZURE_TRACING_DISABLED is set", () => { + beforeEach(() => { + process.env.AZURE_TRACING_DISABLED = "1"; + }); + + afterEach(() => { + delete process.env.AZURE_TRACING_DISABLED; + }); + + it("suppresses tracing for all spans", () => { + const { tracingContext } = instrumenter.startSpan("test", { + packageName, + }); + assert.isTrue(isTracingSuppressed(tracingContext)); + }); + }); + + describe("when AZURE_HTTP_TRACING_DISABLED is set", () => { + beforeEach(() => { + process.env.AZURE_HTTP_TRACING_DISABLED = "1"; + }); + afterEach(() => { + delete process.env.AZURE_HTTP_TRACING_DISABLED; + }); + + it("suppresses tracing for http spans", () => { + const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { + packageName, + }); + + assert.isFalse(span.isRecording()); + assert.isTrue(isTracingSuppressed(tracingContext)); + }); + + it("does not suppress internal spans", () => { + const { span, tracingContext } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isTrue(span.isRecording()); + assert.isFalse(isTracingSuppressed(tracingContext)); + }); + }); + }); + + describe("#isTracingDisabled", () => { + it("is false when env var is blank or missing", () => { + process.env.AZURE_TRACING_DISABLED = ""; + assert.isFalse(isTracingDisabled()); + delete process.env.AZURE_TRACING_DISABLED; + assert.isFalse(isTracingDisabled()); + }); + + it("is false when env var is 'false'", () => { + process.env.AZURE_TRACING_DISABLED = "false"; + assert.isFalse(isTracingDisabled()); + process.env.AZURE_TRACING_DISABLED = "False"; + assert.isFalse(isTracingDisabled()); + process.env.AZURE_TRACING_DISABLED = "FALSE"; + assert.isFalse(isTracingDisabled()); + delete process.env.AZURE_TRACING_DISABLED; + }); + + it("is false when env var is 0", () => { + process.env.AZURE_TRACING_DISABLED = "0"; + assert.isFalse(isTracingDisabled()); + delete process.env.AZURE_TRACING_DISABLED; + }); + + it("is true otherwise", () => { + process.env.AZURE_TRACING_DISABLED = "true"; + assert.isTrue(isTracingDisabled()); + process.env.AZURE_TRACING_DISABLED = "1"; + assert.isTrue(isTracingDisabled()); + delete process.env.AZURE_TRACING_DISABLED; + }); + }); +}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/setup.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/setup.ts index f2c25b9b0482..8d5bfc646ca7 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/setup.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/setup.ts @@ -4,9 +4,8 @@ import { InMemorySpanExporter, SimpleSpanProcessor } from "@opentelemetry/sdk-trace-base"; import { OpenTelemetryInstrumenter } from "../../../src/instrumenter"; -import { useInstrumenter } from "@azure/core-tracing"; - import { tracerProvider } from "./tracerProvider"; +import { useInstrumenter } from "@azure/core-tracing"; // Setup all the necessary instrumenters, exporters, etc. export const inMemoryExporter = new InMemorySpanExporter(); From 795b5a4a167fa2998f4efac01896ebc696e15c46 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 13:22:54 -0800 Subject: [PATCH 02/13] only suppress a single span --- .../src/instrumenter.ts | 22 +++++++++++++------ .../test/public/node/instrumenter.spec.ts | 11 ++++------ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts index 1efca24778c0..311d1844fa4d 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts @@ -1,17 +1,24 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { + INVALID_SPAN_CONTEXT, + Span, + context, + defaultTextMapGetter, + defaultTextMapSetter, + trace, +} from "@opentelemetry/api"; import { Instrumenter, InstrumenterSpanOptions, TracingContext, TracingSpan, } from "@azure/core-tracing"; -import { W3CTraceContextPropagator, suppressTracing } from "@opentelemetry/core"; -import { context, defaultTextMapGetter, defaultTextMapSetter, trace } from "@opentelemetry/api"; import { toBoolean, toSpanOptions } from "./transformations"; import { OpenTelemetrySpanWrapper } from "./spanWrapper"; +import { W3CTraceContextPropagator } from "@opentelemetry/core"; // While default propagation is user-configurable, Azure services always use the W3C implementation. export const propagator = new W3CTraceContextPropagator(); @@ -22,14 +29,15 @@ export class OpenTelemetryInstrumenter implements Instrumenter { spanOptions: InstrumenterSpanOptions ): { span: TracingSpan; tracingContext: TracingContext } { let ctx = spanOptions?.tracingContext || context.active(); + let span: Span; if (isTracingDisabled(name)) { - ctx = suppressTracing(ctx); + span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT); + } else { + span = trace + .getTracer(spanOptions.packageName, spanOptions.packageVersion) + .startSpan(name, toSpanOptions(spanOptions), ctx); } - const span = trace - .getTracer(spanOptions.packageName, spanOptions.packageVersion) - .startSpan(name, toSpanOptions(spanOptions), ctx); - // 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"); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index c6ffe3a3edf2..ff3f2a8354cb 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -5,7 +5,6 @@ import { OpenTelemetryInstrumenter, isTracingDisabled } from "../../../src/instr import { assert } from "chai"; import { inMemoryExporter } from "../util/setup"; -import { isTracingSuppressed } from "@opentelemetry/core"; describe("OpenTelemetryInstrumenter (node)", () => { const packageName = "test-package"; @@ -27,10 +26,10 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); it("suppresses tracing for all spans", () => { - const { tracingContext } = instrumenter.startSpan("test", { + const { span } = instrumenter.startSpan("test", { packageName, }); - assert.isTrue(isTracingSuppressed(tracingContext)); + assert.isFalse(span.isRecording()); }); }); @@ -43,21 +42,19 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); it("suppresses tracing for http spans", () => { - const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { + const { span } = instrumenter.startSpan("HTTP POST", { packageName, }); assert.isFalse(span.isRecording()); - assert.isTrue(isTracingSuppressed(tracingContext)); }); it("does not suppress internal spans", () => { - const { span, tracingContext } = instrumenter.startSpan("foo", { + const { span } = instrumenter.startSpan("foo", { packageName, }); assert.isTrue(span.isRecording()); - assert.isFalse(isTracingSuppressed(tracingContext)); }); }); }); From 29803c57614fc94e6aa7b8c6ab665f288b849924 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 13:34:10 -0800 Subject: [PATCH 03/13] add a few more tests --- .../test/public/node/instrumenter.spec.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index ff3f2a8354cb..b807e6bc21c3 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -60,6 +60,11 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); describe("#isTracingDisabled", () => { + afterEach(() => { + delete process.env.AZURE_TRACING_DISABLED; + delete process.env.AZURE_HTTP_TRACING_DISABLED; + }); + it("is false when env var is blank or missing", () => { process.env.AZURE_TRACING_DISABLED = ""; assert.isFalse(isTracingDisabled()); @@ -74,13 +79,11 @@ describe("OpenTelemetryInstrumenter (node)", () => { assert.isFalse(isTracingDisabled()); process.env.AZURE_TRACING_DISABLED = "FALSE"; assert.isFalse(isTracingDisabled()); - delete process.env.AZURE_TRACING_DISABLED; }); it("is false when env var is 0", () => { process.env.AZURE_TRACING_DISABLED = "0"; assert.isFalse(isTracingDisabled()); - delete process.env.AZURE_TRACING_DISABLED; }); it("is true otherwise", () => { @@ -88,7 +91,19 @@ describe("OpenTelemetryInstrumenter (node)", () => { assert.isTrue(isTracingDisabled()); process.env.AZURE_TRACING_DISABLED = "1"; assert.isTrue(isTracingDisabled()); - delete process.env.AZURE_TRACING_DISABLED; + }); + + describe("when suppressing HTTP spans", () => { + it("is true when creating an HTTP span", () => { + process.env.AZURE_HTTP_TRACING_DISABLED = "true"; + assert.isTrue(isTracingDisabled("HTTP GET")); + assert.isTrue(isTracingDisabled("HTTPS GET")); + }); + + it("is false for non HTTP spans", () => { + process.env.AZURE_HTTP_TRACING_DISABLED = "1"; + assert.isFalse(isTracingDisabled("foo")); + }); }); }); }); From 7b7de32984712d591a1c5eb035d6e57a35334777 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 14:22:22 -0800 Subject: [PATCH 04/13] Revert "only suppress a single span" This reverts commit 795b5a4a167fa2998f4efac01896ebc696e15c46. --- .../src/instrumenter.ts | 22 ++++++------------- .../test/public/node/instrumenter.spec.ts | 11 ++++++---- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts index 311d1844fa4d..1efca24778c0 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts @@ -1,24 +1,17 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { - INVALID_SPAN_CONTEXT, - Span, - context, - defaultTextMapGetter, - defaultTextMapSetter, - trace, -} from "@opentelemetry/api"; import { Instrumenter, InstrumenterSpanOptions, TracingContext, TracingSpan, } from "@azure/core-tracing"; +import { W3CTraceContextPropagator, suppressTracing } from "@opentelemetry/core"; +import { context, defaultTextMapGetter, defaultTextMapSetter, trace } from "@opentelemetry/api"; import { toBoolean, toSpanOptions } from "./transformations"; import { OpenTelemetrySpanWrapper } from "./spanWrapper"; -import { W3CTraceContextPropagator } from "@opentelemetry/core"; // While default propagation is user-configurable, Azure services always use the W3C implementation. export const propagator = new W3CTraceContextPropagator(); @@ -29,15 +22,14 @@ export class OpenTelemetryInstrumenter implements Instrumenter { spanOptions: InstrumenterSpanOptions ): { span: TracingSpan; tracingContext: TracingContext } { let ctx = spanOptions?.tracingContext || context.active(); - let span: Span; if (isTracingDisabled(name)) { - span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT); - } else { - span = trace - .getTracer(spanOptions.packageName, spanOptions.packageVersion) - .startSpan(name, toSpanOptions(spanOptions), ctx); + ctx = suppressTracing(ctx); } + const span = trace + .getTracer(spanOptions.packageName, spanOptions.packageVersion) + .startSpan(name, toSpanOptions(spanOptions), ctx); + // 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"); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index b807e6bc21c3..6f21f9e3ae3a 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -5,6 +5,7 @@ import { OpenTelemetryInstrumenter, isTracingDisabled } from "../../../src/instr import { assert } from "chai"; import { inMemoryExporter } from "../util/setup"; +import { isTracingSuppressed } from "@opentelemetry/core"; describe("OpenTelemetryInstrumenter (node)", () => { const packageName = "test-package"; @@ -26,10 +27,10 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); it("suppresses tracing for all spans", () => { - const { span } = instrumenter.startSpan("test", { + const { tracingContext } = instrumenter.startSpan("test", { packageName, }); - assert.isFalse(span.isRecording()); + assert.isTrue(isTracingSuppressed(tracingContext)); }); }); @@ -42,19 +43,21 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); it("suppresses tracing for http spans", () => { - const { span } = instrumenter.startSpan("HTTP POST", { + const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { packageName, }); assert.isFalse(span.isRecording()); + assert.isTrue(isTracingSuppressed(tracingContext)); }); it("does not suppress internal spans", () => { - const { span } = instrumenter.startSpan("foo", { + const { span, tracingContext } = instrumenter.startSpan("foo", { packageName, }); assert.isTrue(span.isRecording()); + assert.isFalse(isTracingSuppressed(tracingContext)); }); }); }); From 324c66faf0e1cbe98bf1354904936475a972b215 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 14:51:36 -0800 Subject: [PATCH 05/13] updates after offline discussion --- .../src/instrumenter.ts | 58 ++++++++--------- .../src/transformations.ts | 14 +++-- .../internal/node/transformations.spec.ts | 39 ++++++++++++ .../test/public/node/instrumenter.spec.ts | 63 +++---------------- 4 files changed, 82 insertions(+), 92 deletions(-) create mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts index 1efca24778c0..d0a4b4b82188 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts @@ -1,6 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { + INVALID_SPAN_CONTEXT, + Span, + context, + defaultTextMapGetter, + defaultTextMapSetter, + trace, +} from "@opentelemetry/api"; import { Instrumenter, InstrumenterSpanOptions, @@ -8,8 +16,7 @@ import { TracingSpan, } from "@azure/core-tracing"; import { W3CTraceContextPropagator, suppressTracing } from "@opentelemetry/core"; -import { context, defaultTextMapGetter, defaultTextMapSetter, trace } from "@opentelemetry/api"; -import { toBoolean, toSpanOptions } from "./transformations"; +import { environmentVariableToBoolean, toSpanOptions } from "./transformations"; import { OpenTelemetrySpanWrapper } from "./spanWrapper"; @@ -22,13 +29,25 @@ export class OpenTelemetryInstrumenter implements Instrumenter { spanOptions: InstrumenterSpanOptions ): { span: TracingSpan; tracingContext: TracingContext } { let ctx = spanOptions?.tracingContext || context.active(); - if (isTracingDisabled(name)) { - ctx = suppressTracing(ctx); - } + let span: Span; - const span = trace - .getTracer(spanOptions.packageName, spanOptions.packageVersion) - .startSpan(name, toSpanOptions(spanOptions), ctx); + if (environmentVariableToBoolean("AZURE_TRACING_DISABLED")) { + // disable only our spans but not any downstream spans + span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT); + } else { + // Create our span + span = trace + .getTracer(spanOptions.packageName, spanOptions.packageVersion) + .startSpan(name, toSpanOptions(spanOptions), ctx); + + if ( + environmentVariableToBoolean("AZURE_HTTP_TRACING_DISABLED") && + name.toUpperCase().startsWith("HTTP") + ) { + // disable downstream spans + ctx = suppressTracing(ctx); + } + } // COMPAT: remove when core-rest-pipeline has upgraded to core-tracing 1.0 // https://github.com/Azure/azure-sdk-for-js/issues/20567 @@ -73,26 +92,3 @@ export class OpenTelemetryInstrumenter implements Instrumenter { return headers; } } - -/** - * Checks whether tracing is disabled by checking the relevant environment variables. - * - * @returns - `true` if tracing is disabled, `false` otherwise. - * - * @internal - */ -export function isTracingDisabled(spanName?: string): boolean { - if (typeof process === "undefined") { - return false; - } - - const tracingDisabledGlobally = toBoolean(process.env.AZURE_TRACING_DISABLED); - const httpSpansDisabled = toBoolean(process.env.AZURE_HTTP_TRACING_DISABLED); - - const conditions = [ - tracingDisabledGlobally, - spanName && spanName.startsWith("HTTP") && httpSpansDisabled, - ]; - - return conditions.some(Boolean); -} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts index 2a7d0266b377..797b99bbd446 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts @@ -95,14 +95,16 @@ export function toSpanOptions(spanOptions?: InstrumenterSpanOptions): SpanOption } /** - * A helper function that converts a raw string to its boolean representation. + * Given an environment variable name, returns whether it is truthy. * - * This function treats "false" and "0" as false, but otherwise uses truthy-ness of the value. + * We consider "false" and "0" as falsey when used in environment variables. * - * @param rawValue - The raw string value to convert to boolean. - * @returns - true if the {@link rawValue} is truthy, false otherwise. + * @internal */ -export function toBoolean(rawValue?: string): boolean { - const valueWithDefault = (rawValue ?? "").toLowerCase(); +export function environmentVariableToBoolean(environmentVariableName: string): boolean { + if (typeof process === "undefined") { + return false; + } + const valueWithDefault = (process.env[environmentVariableName] ?? "").toLowerCase(); return valueWithDefault !== "false" && valueWithDefault !== "0" && Boolean(valueWithDefault); } diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts new file mode 100644 index 000000000000..e7f5664c5a8b --- /dev/null +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { assert } from "chai"; +import { environmentVariableToBoolean } from "../../../src/transformations"; + +describe("#environmentVariableToBoolean", () => { + afterEach(() => { + delete process.env.FOO; + }); + + it("is false when env var is blank or missing", () => { + process.env.FOO = ""; + assert.isFalse(environmentVariableToBoolean("FOO")); + delete process.env.FOO; + assert.isFalse(environmentVariableToBoolean("FOO")); + }); + + it("is false when env var is 'false'", () => { + process.env.FOO = "false"; + assert.isFalse(environmentVariableToBoolean("FOO")); + process.env.FOO = "False"; + assert.isFalse(environmentVariableToBoolean("FOO")); + process.env.FOO = "FALSE"; + assert.isFalse(environmentVariableToBoolean("FOO")); + }); + + it("is false when env var is 0", () => { + process.env.FOO = "0"; + assert.isFalse(environmentVariableToBoolean("FOO")); + }); + + it("is true otherwise", () => { + process.env.FOO = "true"; + assert.isTrue(environmentVariableToBoolean("FOO")); + process.env.FOO = "1"; + assert.isTrue(environmentVariableToBoolean("FOO")); + }); +}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index 6f21f9e3ae3a..f07fef1cd662 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { OpenTelemetryInstrumenter, isTracingDisabled } from "../../../src/instrumenter"; - +import { OpenTelemetryInstrumenter } from "../../../src/instrumenter"; import { assert } from "chai"; import { inMemoryExporter } from "../util/setup"; import { isTracingSuppressed } from "@opentelemetry/core"; @@ -26,11 +25,12 @@ describe("OpenTelemetryInstrumenter (node)", () => { delete process.env.AZURE_TRACING_DISABLED; }); - it("suppresses tracing for all spans", () => { - const { tracingContext } = instrumenter.startSpan("test", { + it("suppresses tracing for our spans", () => { + const { tracingContext, span } = instrumenter.startSpan("test", { packageName, }); - assert.isTrue(isTracingSuppressed(tracingContext)); + assert.isFalse(span.isRecording()); + assert.isFalse(isTracingSuppressed(tracingContext)); }); }); @@ -38,16 +38,17 @@ describe("OpenTelemetryInstrumenter (node)", () => { beforeEach(() => { process.env.AZURE_HTTP_TRACING_DISABLED = "1"; }); + afterEach(() => { delete process.env.AZURE_HTTP_TRACING_DISABLED; }); - it("suppresses tracing for http spans", () => { + it("suppresses tracing for downstream spans", () => { const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { packageName, }); - assert.isFalse(span.isRecording()); + assert.isTrue(span.isRecording()); assert.isTrue(isTracingSuppressed(tracingContext)); }); @@ -61,52 +62,4 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); }); }); - - describe("#isTracingDisabled", () => { - afterEach(() => { - delete process.env.AZURE_TRACING_DISABLED; - delete process.env.AZURE_HTTP_TRACING_DISABLED; - }); - - it("is false when env var is blank or missing", () => { - process.env.AZURE_TRACING_DISABLED = ""; - assert.isFalse(isTracingDisabled()); - delete process.env.AZURE_TRACING_DISABLED; - assert.isFalse(isTracingDisabled()); - }); - - it("is false when env var is 'false'", () => { - process.env.AZURE_TRACING_DISABLED = "false"; - assert.isFalse(isTracingDisabled()); - process.env.AZURE_TRACING_DISABLED = "False"; - assert.isFalse(isTracingDisabled()); - process.env.AZURE_TRACING_DISABLED = "FALSE"; - assert.isFalse(isTracingDisabled()); - }); - - it("is false when env var is 0", () => { - process.env.AZURE_TRACING_DISABLED = "0"; - assert.isFalse(isTracingDisabled()); - }); - - it("is true otherwise", () => { - process.env.AZURE_TRACING_DISABLED = "true"; - assert.isTrue(isTracingDisabled()); - process.env.AZURE_TRACING_DISABLED = "1"; - assert.isTrue(isTracingDisabled()); - }); - - describe("when suppressing HTTP spans", () => { - it("is true when creating an HTTP span", () => { - process.env.AZURE_HTTP_TRACING_DISABLED = "true"; - assert.isTrue(isTracingDisabled("HTTP GET")); - assert.isTrue(isTracingDisabled("HTTPS GET")); - }); - - it("is false for non HTTP spans", () => { - process.env.AZURE_HTTP_TRACING_DISABLED = "1"; - assert.isFalse(isTracingDisabled("foo")); - }); - }); - }); }); From b3ee9edcba53fdab17920f8a80333066160e472a Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Thu, 10 Mar 2022 14:56:31 -0800 Subject: [PATCH 06/13] add tests --- .../test/public/node/instrumenter.spec.ts | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index f07fef1cd662..ceec26be941e 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -13,6 +13,11 @@ describe("OpenTelemetryInstrumenter (node)", () => { inMemoryExporter.reset(); }); + afterEach(() => { + delete process.env.AZURE_HTTP_TRACING_DISABLED; + delete process.env.AZURE_TRACING_DISABLED; + }); + const instrumenter = new OpenTelemetryInstrumenter(); describe("startSpan", () => { @@ -39,10 +44,6 @@ describe("OpenTelemetryInstrumenter (node)", () => { process.env.AZURE_HTTP_TRACING_DISABLED = "1"; }); - afterEach(() => { - delete process.env.AZURE_HTTP_TRACING_DISABLED; - }); - it("suppresses tracing for downstream spans", () => { const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { packageName, @@ -61,5 +62,28 @@ describe("OpenTelemetryInstrumenter (node)", () => { assert.isFalse(isTracingSuppressed(tracingContext)); }); }); + + describe("when both AZURE_TRACING_DISABLED and AZURE_HTTP_TRACING_DISABLED are set", () => { + beforeEach(() => { + process.env.AZURE_TRACING_DISABLED = "true"; + process.env.AZURE_HTTP_TRACING_DISABLED = "true"; + }); + + it("creates a non-recording span", () => { + const { span } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isFalse(span.isRecording()); + }); + + it("does not suppress downstream spans", () => { + const { tracingContext } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isFalse(isTracingSuppressed(tracingContext)); + }); + }); }); }); From 83b5a2b33cca1196ac4de2b72746d7b9e465d6db Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 10:24:38 -0800 Subject: [PATCH 07/13] cache env vars --- .../src/configuration.ts | 29 ++++++++++++++ .../src/constants.ts | 4 -- .../src/instrumentation.browser.ts | 2 +- .../src/instrumentation.ts | 2 +- .../src/instrumenter.ts | 10 ++--- .../src/transformations.ts | 15 ------- .../test/internal/configuration.spec.ts | 38 ++++++++++++++++++ .../internal/node/transformations.spec.ts | 39 ------------------- .../test/public/node/instrumenter.spec.ts | 16 +++----- .../test/public/util/testClient.ts | 2 +- 10 files changed, 80 insertions(+), 77 deletions(-) create mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts delete mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/constants.ts create mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts delete mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts new file mode 100644 index 000000000000..c84af658e52d --- /dev/null +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +export const SDK_VERSION: string = "1.0.0-beta.2"; + +/** + * @internal + * + * Keys of known environment variables we lookup. + */ +export type KnownEnvironmentKeys = "AZURE_HTTP_TRACING_DISABLED" | "AZURE_TRACING_DISABLED"; + +export const environment = new Map(); + +if (typeof process !== "undefined" && process.env) { + environment.set("AZURE_HTTP_TRACING_DISABLED", process.env.AZURE_HTTP_TRACING_DISABLED); + environment.set("AZURE_TRACING_DISABLED", process.env.AZURE_TRACING_DISABLED); +} + +/** + * Converts an environment variable to Boolean. + * the strings "false" and "0" are treated as falsy values. + * + * @internal + */ +export function envVarToBoolean(key: KnownEnvironmentKeys): boolean { + const valueWithDefault = (environment.get(key) ?? "").toLowerCase(); + return valueWithDefault !== "false" && valueWithDefault !== "0" && Boolean(valueWithDefault); +} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/constants.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/constants.ts deleted file mode 100644 index 328eef7797c9..000000000000 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/constants.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -export const SDK_VERSION: string = "1.0.0-beta.2"; diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.browser.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.browser.ts index 44c823e3f97f..3f470cd760b6 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.browser.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.browser.ts @@ -7,7 +7,7 @@ import { InstrumentationConfig, } from "@opentelemetry/instrumentation"; import { OpenTelemetryInstrumenter } from "./instrumenter"; -import { SDK_VERSION } from "./constants"; +import { SDK_VERSION } from "./configuration"; import { useInstrumenter } from "@azure/core-tracing"; /** diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.ts index 2103681fcba6..98ef4def2b7f 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumentation.ts @@ -10,7 +10,7 @@ import { InstrumentationNodeModuleDefinition, } from "@opentelemetry/instrumentation"; import { OpenTelemetryInstrumenter } from "./instrumenter"; -import { SDK_VERSION } from "./constants"; +import { SDK_VERSION } from "./configuration"; /** * Configuration options that can be passed to {@link createAzureSdkInstrumentation} function. diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts index d0a4b4b82188..af9f1c1f52cd 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/instrumenter.ts @@ -16,9 +16,10 @@ import { TracingSpan, } from "@azure/core-tracing"; import { W3CTraceContextPropagator, suppressTracing } from "@opentelemetry/core"; -import { environmentVariableToBoolean, toSpanOptions } from "./transformations"; import { OpenTelemetrySpanWrapper } from "./spanWrapper"; +import { envVarToBoolean } from "./configuration"; +import { toSpanOptions } from "./transformations"; // While default propagation is user-configurable, Azure services always use the W3C implementation. export const propagator = new W3CTraceContextPropagator(); @@ -31,7 +32,7 @@ export class OpenTelemetryInstrumenter implements Instrumenter { let ctx = spanOptions?.tracingContext || context.active(); let span: Span; - if (environmentVariableToBoolean("AZURE_TRACING_DISABLED")) { + if (envVarToBoolean("AZURE_TRACING_DISABLED")) { // disable only our spans but not any downstream spans span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT); } else { @@ -40,10 +41,7 @@ export class OpenTelemetryInstrumenter implements Instrumenter { .getTracer(spanOptions.packageName, spanOptions.packageVersion) .startSpan(name, toSpanOptions(spanOptions), ctx); - if ( - environmentVariableToBoolean("AZURE_HTTP_TRACING_DISABLED") && - name.toUpperCase().startsWith("HTTP") - ) { + if (envVarToBoolean("AZURE_HTTP_TRACING_DISABLED") && name.toUpperCase().startsWith("HTTP")) { // disable downstream spans ctx = suppressTracing(ctx); } diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts index 797b99bbd446..2bc9ad8926c0 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/transformations.ts @@ -93,18 +93,3 @@ export function toSpanOptions(spanOptions?: InstrumenterSpanOptions): SpanOption links, }; } - -/** - * Given an environment variable name, returns whether it is truthy. - * - * We consider "false" and "0" as falsey when used in environment variables. - * - * @internal - */ -export function environmentVariableToBoolean(environmentVariableName: string): boolean { - if (typeof process === "undefined") { - return false; - } - const valueWithDefault = (process.env[environmentVariableName] ?? "").toLowerCase(); - return valueWithDefault !== "false" && valueWithDefault !== "0" && Boolean(valueWithDefault); -} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts new file mode 100644 index 000000000000..ec7a93a1b9f3 --- /dev/null +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { KnownEnvironmentKeys, envVarToBoolean, environment } from "../../src/configuration"; + +import { assert } from "chai"; + +describe("#envVarToBoolean", () => { + const key = "FOO" as KnownEnvironmentKeys; + + it("is false when env var is blank or missing", () => { + environment.set(key, ""); + assert.isFalse(envVarToBoolean(key)); + environment.delete(key); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is false when env var is 'false'", () => { + environment.set(key, "false"); + assert.isFalse(envVarToBoolean(key)); + environment.set(key, "False"); + assert.isFalse(envVarToBoolean(key)); + environment.set(key, "FALSE"); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is false when env var is 0", () => { + environment.set(key, "0"); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is true otherwise", () => { + environment.set(key, "true"); + assert.isTrue(envVarToBoolean(key)); + environment.set(key, "1"); + assert.isTrue(envVarToBoolean(key)); + }); +}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts deleted file mode 100644 index e7f5664c5a8b..000000000000 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/transformations.spec.ts +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { assert } from "chai"; -import { environmentVariableToBoolean } from "../../../src/transformations"; - -describe("#environmentVariableToBoolean", () => { - afterEach(() => { - delete process.env.FOO; - }); - - it("is false when env var is blank or missing", () => { - process.env.FOO = ""; - assert.isFalse(environmentVariableToBoolean("FOO")); - delete process.env.FOO; - assert.isFalse(environmentVariableToBoolean("FOO")); - }); - - it("is false when env var is 'false'", () => { - process.env.FOO = "false"; - assert.isFalse(environmentVariableToBoolean("FOO")); - process.env.FOO = "False"; - assert.isFalse(environmentVariableToBoolean("FOO")); - process.env.FOO = "FALSE"; - assert.isFalse(environmentVariableToBoolean("FOO")); - }); - - it("is false when env var is 0", () => { - process.env.FOO = "0"; - assert.isFalse(environmentVariableToBoolean("FOO")); - }); - - it("is true otherwise", () => { - process.env.FOO = "true"; - assert.isTrue(environmentVariableToBoolean("FOO")); - process.env.FOO = "1"; - assert.isTrue(environmentVariableToBoolean("FOO")); - }); -}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts index ceec26be941e..92f52c040fb1 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts @@ -3,6 +3,7 @@ import { OpenTelemetryInstrumenter } from "../../../src/instrumenter"; import { assert } from "chai"; +import { environment } from "../../../src/configuration"; import { inMemoryExporter } from "../util/setup"; import { isTracingSuppressed } from "@opentelemetry/core"; @@ -14,8 +15,7 @@ describe("OpenTelemetryInstrumenter (node)", () => { }); afterEach(() => { - delete process.env.AZURE_HTTP_TRACING_DISABLED; - delete process.env.AZURE_TRACING_DISABLED; + environment.clear(); }); const instrumenter = new OpenTelemetryInstrumenter(); @@ -23,11 +23,7 @@ describe("OpenTelemetryInstrumenter (node)", () => { describe("startSpan", () => { describe("when AZURE_TRACING_DISABLED is set", () => { beforeEach(() => { - process.env.AZURE_TRACING_DISABLED = "1"; - }); - - afterEach(() => { - delete process.env.AZURE_TRACING_DISABLED; + environment.set("AZURE_TRACING_DISABLED", "1"); }); it("suppresses tracing for our spans", () => { @@ -41,7 +37,7 @@ describe("OpenTelemetryInstrumenter (node)", () => { describe("when AZURE_HTTP_TRACING_DISABLED is set", () => { beforeEach(() => { - process.env.AZURE_HTTP_TRACING_DISABLED = "1"; + environment.set("AZURE_HTTP_TRACING_DISABLED", "1"); }); it("suppresses tracing for downstream spans", () => { @@ -65,8 +61,8 @@ describe("OpenTelemetryInstrumenter (node)", () => { describe("when both AZURE_TRACING_DISABLED and AZURE_HTTP_TRACING_DISABLED are set", () => { beforeEach(() => { - process.env.AZURE_TRACING_DISABLED = "true"; - process.env.AZURE_HTTP_TRACING_DISABLED = "true"; + environment.set("AZURE_TRACING_DISABLED", "true"); + environment.set("AZURE_HTTP_TRACING_DISABLED", "True"); }); it("creates a non-recording span", () => { diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/testClient.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/testClient.ts index 0309ebf6d5cd..3ab3116f3059 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/testClient.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/util/testClient.ts @@ -10,7 +10,7 @@ import { createPipelineFromOptions, } from "@azure/core-rest-pipeline"; -import { SDK_VERSION } from "../../../src/constants"; +import { SDK_VERSION } from "../../../src/configuration"; /** * A partial interface compatible with OperationOptions. From ee3a5e7e80b56adc23e17e7402d485b4a0d3e235 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 10:27:54 -0800 Subject: [PATCH 08/13] changelog --- .../opentelemetry-instrumentation-azure-sdk/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md index 93f3e28c992e..c6b27ecb8611 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md @@ -4,6 +4,9 @@ ### Features Added +- Added support for `AZURE_TRACING_DISABLED` environment variable which allows disabling all Azure SDK tracing spans from being recorded. +- Added support for `AZURE_HTTP_TRACING_DISABLED` environment variable which allows disabling all children of our core HTTP spans from being recorded. + ### Breaking Changes ### Bugs Fixed From 3a44f9a7ecdb8639084c8101975d5e1a7236e595 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 10:31:10 -0800 Subject: [PATCH 09/13] update constants path --- .../opentelemetry-instrumentation-azure-sdk/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/package.json b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/package.json index c78fd954e3c8..712178a48a71 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/package.json +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/package.json @@ -12,7 +12,7 @@ "//metadata": { "constantPaths": [ { - "path": "src/constants.ts", + "path": "src/configuration.ts", "prefix": "SDK_VERSION" } ] From 7248bc7d8dcee5763d384e79bb8660009f670cfa Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 15:36:17 -0800 Subject: [PATCH 10/13] fixup! update constants path --- .../opentelemetry-instrumentation-azure-sdk/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md index c6b27ecb8611..ad9c708c62b0 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features Added -- Added support for `AZURE_TRACING_DISABLED` environment variable which allows disabling all Azure SDK tracing spans from being recorded. +- Added the ability to disable Azure SDK Spans from being recorded by setting the `AZURE_TRACING_DISABLED` environment variable to true. - Added support for `AZURE_HTTP_TRACING_DISABLED` environment variable which allows disabling all children of our core HTTP spans from being recorded. ### Breaking Changes From 72a2543422208d280c677af9328ac7b754847d24 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 16:20:51 -0800 Subject: [PATCH 11/13] renames --- .../src/configuration.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts index c84af658e52d..9c9db1126109 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts @@ -6,11 +6,11 @@ export const SDK_VERSION: string = "1.0.0-beta.2"; /** * @internal * - * Keys of known environment variables we lookup. + * Keys of known environment variables we look up. */ -export type KnownEnvironmentKeys = "AZURE_HTTP_TRACING_DISABLED" | "AZURE_TRACING_DISABLED"; +export type KnownEnvironmentKey = "AZURE_HTTP_TRACING_DISABLED" | "AZURE_TRACING_DISABLED"; -export const environment = new Map(); +export const environment = new Map(); if (typeof process !== "undefined" && process.env) { environment.set("AZURE_HTTP_TRACING_DISABLED", process.env.AZURE_HTTP_TRACING_DISABLED); @@ -23,7 +23,7 @@ if (typeof process !== "undefined" && process.env) { * * @internal */ -export function envVarToBoolean(key: KnownEnvironmentKeys): boolean { - const valueWithDefault = (environment.get(key) ?? "").toLowerCase(); - return valueWithDefault !== "false" && valueWithDefault !== "0" && Boolean(valueWithDefault); +export function envVarToBoolean(key: KnownEnvironmentKey): boolean { + const value = (environment.get(key) ?? "").toLowerCase(); + return value !== "false" && value !== "0" && Boolean(value); } From 9d95a468cdeda6951e9d5ce35fae21c7052fde4f Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Fri, 11 Mar 2022 16:32:59 -0800 Subject: [PATCH 12/13] cleanup tests --- .../test/internal/configuration.spec.ts | 4 +- .../test/public/instrumenter.spec.ts | 66 ++++++++++++++ .../test/public/node/instrumenter.spec.ts | 85 ------------------- 3 files changed, 68 insertions(+), 87 deletions(-) delete mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts index ec7a93a1b9f3..34d769e32ce5 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts @@ -1,12 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { KnownEnvironmentKeys, envVarToBoolean, environment } from "../../src/configuration"; +import { KnownEnvironmentKey, envVarToBoolean, environment } from "../../src/configuration"; import { assert } from "chai"; describe("#envVarToBoolean", () => { - const key = "FOO" as KnownEnvironmentKeys; + const key = "FOO" as KnownEnvironmentKey; it("is false when env var is blank or missing", () => { environment.set(key, ""); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts index 36125598f577..29c752da5ae0 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts @@ -9,7 +9,9 @@ import { Context } from "mocha"; import { OpenTelemetrySpanWrapper } from "../../src/spanWrapper"; import { Span } from "@opentelemetry/sdk-trace-base"; import { assert } from "chai"; +import { environment } from "../../src/configuration"; import { inMemoryExporter } from "./util/setup"; +import { isTracingSuppressed } from "@opentelemetry/core"; import sinon from "sinon"; function unwrap(span: TracingSpan): Span { @@ -197,6 +199,70 @@ describe("OpenTelemetryInstrumenter", () => { assert.deepEqual(links[0].context, trace.getSpan(linkedContext!)?.spanContext()); }); }); + + describe("environment variables", () => { + afterEach(() => { + environment.clear(); + }); + + describe("when AZURE_TRACING_DISABLED is set", () => { + it("suppresses tracing for our spans", () => { + environment.set("AZURE_TRACING_DISABLED", "1"); + const { tracingContext, span } = instrumenter.startSpan("test", { + packageName, + }); + assert.isFalse(span.isRecording()); + assert.isFalse(isTracingSuppressed(tracingContext)); + }); + }); + + describe("when AZURE_HTTP_TRACING_DISABLED is set", () => { + beforeEach(() => { + environment.set("AZURE_HTTP_TRACING_DISABLED", "1"); + }); + + it("suppresses tracing for downstream spans", () => { + const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { + packageName, + }); + + assert.isTrue(span.isRecording()); + assert.isTrue(isTracingSuppressed(tracingContext)); + }); + + it("does not suppress internal spans", () => { + const { span, tracingContext } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isTrue(span.isRecording()); + assert.isFalse(isTracingSuppressed(tracingContext)); + }); + }); + + describe("when both AZURE_TRACING_DISABLED and AZURE_HTTP_TRACING_DISABLED are set", () => { + beforeEach(() => { + environment.set("AZURE_TRACING_DISABLED", "true"); + environment.set("AZURE_HTTP_TRACING_DISABLED", "True"); + }); + + it("creates a non-recording span", () => { + const { span } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isFalse(span.isRecording()); + }); + + it("does not suppress downstream spans", () => { + const { tracingContext } = instrumenter.startSpan("foo", { + packageName, + }); + + assert.isFalse(isTracingSuppressed(tracingContext)); + }); + }); + }); }); describe("#withContext", () => { diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts deleted file mode 100644 index 92f52c040fb1..000000000000 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/node/instrumenter.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { OpenTelemetryInstrumenter } from "../../../src/instrumenter"; -import { assert } from "chai"; -import { environment } from "../../../src/configuration"; -import { inMemoryExporter } from "../util/setup"; -import { isTracingSuppressed } from "@opentelemetry/core"; - -describe("OpenTelemetryInstrumenter (node)", () => { - const packageName = "test-package"; - - beforeEach(() => { - inMemoryExporter.reset(); - }); - - afterEach(() => { - environment.clear(); - }); - - const instrumenter = new OpenTelemetryInstrumenter(); - - describe("startSpan", () => { - describe("when AZURE_TRACING_DISABLED is set", () => { - beforeEach(() => { - environment.set("AZURE_TRACING_DISABLED", "1"); - }); - - it("suppresses tracing for our spans", () => { - const { tracingContext, span } = instrumenter.startSpan("test", { - packageName, - }); - assert.isFalse(span.isRecording()); - assert.isFalse(isTracingSuppressed(tracingContext)); - }); - }); - - describe("when AZURE_HTTP_TRACING_DISABLED is set", () => { - beforeEach(() => { - environment.set("AZURE_HTTP_TRACING_DISABLED", "1"); - }); - - it("suppresses tracing for downstream spans", () => { - const { span, tracingContext } = instrumenter.startSpan("HTTP POST", { - packageName, - }); - - assert.isTrue(span.isRecording()); - assert.isTrue(isTracingSuppressed(tracingContext)); - }); - - it("does not suppress internal spans", () => { - const { span, tracingContext } = instrumenter.startSpan("foo", { - packageName, - }); - - assert.isTrue(span.isRecording()); - assert.isFalse(isTracingSuppressed(tracingContext)); - }); - }); - - describe("when both AZURE_TRACING_DISABLED and AZURE_HTTP_TRACING_DISABLED are set", () => { - beforeEach(() => { - environment.set("AZURE_TRACING_DISABLED", "true"); - environment.set("AZURE_HTTP_TRACING_DISABLED", "True"); - }); - - it("creates a non-recording span", () => { - const { span } = instrumenter.startSpan("foo", { - packageName, - }); - - assert.isFalse(span.isRecording()); - }); - - it("does not suppress downstream spans", () => { - const { tracingContext } = instrumenter.startSpan("foo", { - packageName, - }); - - assert.isFalse(isTracingSuppressed(tracingContext)); - }); - }); - }); -}); From d56d16d2baea9a850e1cae6810551f0b9865e229 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Mon, 14 Mar 2022 15:39:54 -0700 Subject: [PATCH 13/13] PR feedback --- .../src/configuration.ts | 24 ++++++--- .../test/internal/configuration.spec.ts | 38 -------------- .../test/internal/node/configuration.spec.ts | 51 +++++++++++++++++++ .../test/public/instrumenter.spec.ts | 12 ++--- 4 files changed, 74 insertions(+), 51 deletions(-) delete mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts create mode 100644 sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/configuration.spec.ts diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts index 9c9db1126109..c38e01e62438 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/src/configuration.ts @@ -10,12 +10,12 @@ export const SDK_VERSION: string = "1.0.0-beta.2"; */ export type KnownEnvironmentKey = "AZURE_HTTP_TRACING_DISABLED" | "AZURE_TRACING_DISABLED"; -export const environment = new Map(); - -if (typeof process !== "undefined" && process.env) { - environment.set("AZURE_HTTP_TRACING_DISABLED", process.env.AZURE_HTTP_TRACING_DISABLED); - environment.set("AZURE_TRACING_DISABLED", process.env.AZURE_TRACING_DISABLED); -} +/** + * @internal + * + * Cached values of environment variables that were fetched. + */ +export const environmentCache = new Map(); /** * Converts an environment variable to Boolean. @@ -24,6 +24,16 @@ if (typeof process !== "undefined" && process.env) { * @internal */ export function envVarToBoolean(key: KnownEnvironmentKey): boolean { - const value = (environment.get(key) ?? "").toLowerCase(); + if (!environmentCache.has(key)) { + loadEnvironmentVariable(key); + } + const value = (environmentCache.get(key) ?? "").toLowerCase(); return value !== "false" && value !== "0" && Boolean(value); } + +function loadEnvironmentVariable(key: KnownEnvironmentKey): void { + if (typeof process !== "undefined" && process.env) { + const rawValue = process.env[key] ?? process.env[key.toLowerCase()]; + environmentCache.set(key, rawValue); + } +} diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts deleted file mode 100644 index 34d769e32ce5..000000000000 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/configuration.spec.ts +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { KnownEnvironmentKey, envVarToBoolean, environment } from "../../src/configuration"; - -import { assert } from "chai"; - -describe("#envVarToBoolean", () => { - const key = "FOO" as KnownEnvironmentKey; - - it("is false when env var is blank or missing", () => { - environment.set(key, ""); - assert.isFalse(envVarToBoolean(key)); - environment.delete(key); - assert.isFalse(envVarToBoolean(key)); - }); - - it("is false when env var is 'false'", () => { - environment.set(key, "false"); - assert.isFalse(envVarToBoolean(key)); - environment.set(key, "False"); - assert.isFalse(envVarToBoolean(key)); - environment.set(key, "FALSE"); - assert.isFalse(envVarToBoolean(key)); - }); - - it("is false when env var is 0", () => { - environment.set(key, "0"); - assert.isFalse(envVarToBoolean(key)); - }); - - it("is true otherwise", () => { - environment.set(key, "true"); - assert.isTrue(envVarToBoolean(key)); - environment.set(key, "1"); - assert.isTrue(envVarToBoolean(key)); - }); -}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/configuration.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/configuration.spec.ts new file mode 100644 index 000000000000..7584c4f6a528 --- /dev/null +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/internal/node/configuration.spec.ts @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { KnownEnvironmentKey, envVarToBoolean, environmentCache } from "../../../src/configuration"; + +import { assert } from "chai"; + +describe("#envVarToBoolean", () => { + const key = "FOO" as KnownEnvironmentKey; + + it("is false when env var is blank or missing", () => { + environmentCache.set(key, ""); + assert.isFalse(envVarToBoolean(key)); + environmentCache.delete(key); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is false when env var is 'false'", () => { + environmentCache.set(key, "false"); + assert.isFalse(envVarToBoolean(key)); + environmentCache.set(key, "False"); + assert.isFalse(envVarToBoolean(key)); + environmentCache.set(key, "FALSE"); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is false when env var is 0", () => { + environmentCache.set(key, "0"); + assert.isFalse(envVarToBoolean(key)); + }); + + it("is true otherwise", () => { + environmentCache.set(key, "true"); + assert.isTrue(envVarToBoolean(key)); + environmentCache.set(key, "1"); + assert.isTrue(envVarToBoolean(key)); + }); + + it("reads from the environment on first access", () => { + const keyName = `caches-on-first-access-${Date.now()}`; + process.env[keyName] = "true"; + assert.isTrue(envVarToBoolean(keyName as KnownEnvironmentKey)); + delete process.env[keyName]; + }); + + it("supports lowercase and uppercase search", () => { + const keyName = `is-case-insensitive-${Date.now()}`; + process.env[keyName] = "true"; + assert.isTrue(envVarToBoolean(keyName.toUpperCase() as KnownEnvironmentKey)); + }); +}); diff --git a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts index 29c752da5ae0..9968311a9537 100644 --- a/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts +++ b/sdk/instrumentation/opentelemetry-instrumentation-azure-sdk/test/public/instrumenter.spec.ts @@ -9,7 +9,7 @@ import { Context } from "mocha"; import { OpenTelemetrySpanWrapper } from "../../src/spanWrapper"; import { Span } from "@opentelemetry/sdk-trace-base"; import { assert } from "chai"; -import { environment } from "../../src/configuration"; +import { environmentCache } from "../../src/configuration"; import { inMemoryExporter } from "./util/setup"; import { isTracingSuppressed } from "@opentelemetry/core"; import sinon from "sinon"; @@ -202,12 +202,12 @@ describe("OpenTelemetryInstrumenter", () => { describe("environment variables", () => { afterEach(() => { - environment.clear(); + environmentCache.clear(); }); describe("when AZURE_TRACING_DISABLED is set", () => { it("suppresses tracing for our spans", () => { - environment.set("AZURE_TRACING_DISABLED", "1"); + environmentCache.set("AZURE_TRACING_DISABLED", "1"); const { tracingContext, span } = instrumenter.startSpan("test", { packageName, }); @@ -218,7 +218,7 @@ describe("OpenTelemetryInstrumenter", () => { describe("when AZURE_HTTP_TRACING_DISABLED is set", () => { beforeEach(() => { - environment.set("AZURE_HTTP_TRACING_DISABLED", "1"); + environmentCache.set("AZURE_HTTP_TRACING_DISABLED", "1"); }); it("suppresses tracing for downstream spans", () => { @@ -242,8 +242,8 @@ describe("OpenTelemetryInstrumenter", () => { describe("when both AZURE_TRACING_DISABLED and AZURE_HTTP_TRACING_DISABLED are set", () => { beforeEach(() => { - environment.set("AZURE_TRACING_DISABLED", "true"); - environment.set("AZURE_HTTP_TRACING_DISABLED", "True"); + environmentCache.set("AZURE_TRACING_DISABLED", "true"); + environmentCache.set("AZURE_HTTP_TRACING_DISABLED", "True"); }); it("creates a non-recording span", () => {