From c98e546f8b613fbe6eb71168a40542409492cecd Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Fri, 7 Jun 2024 16:20:24 -0700 Subject: [PATCH] Update Sampling Percentage to Accept 0 as Valid (#1337) * Update setting of sampling ratio to accept 0. * Add test for zero sampling. * Update to a simple undefined check since the distro will handle full sampling ratio validity logic. * Update to send warning if sampling percentage is set incorrectly in the app insights v3. * Add test to ensure warning will be thrown. --- src/shared/configuration/config.ts | 2 +- src/shim/shim-config.ts | 8 ++++++-- test/unitTests/shim/config.tests.ts | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/shared/configuration/config.ts b/src/shared/configuration/config.ts index 440b1e58..b88bbb32 100644 --- a/src/shared/configuration/config.ts +++ b/src/shared/configuration/config.ts @@ -109,7 +109,7 @@ export class ApplicationInsightsConfig { options.instrumentationOptions ); this.resource = Object.assign(this.resource, options.resource); - this.samplingRatio = options.samplingRatio || this.samplingRatio; + this.samplingRatio = options.samplingRatio !== undefined ? options.samplingRatio : this.samplingRatio; // Set console logging level from env var if (process.env[loggingLevel]) { diff --git a/src/shim/shim-config.ts b/src/shim/shim-config.ts index 4de17b7c..4e2c978e 100644 --- a/src/shim/shim-config.ts +++ b/src/shim/shim-config.ts @@ -151,8 +151,12 @@ class Config implements IConfig { ...options.instrumentationOptions, console: { enabled: false }, }; - if (this.samplingPercentage) { - options.samplingRatio = this.samplingPercentage / 100; + const samplingRatio = this.samplingPercentage / 100; + if (samplingRatio >= 0 && samplingRatio <= 1) { + options.samplingRatio = samplingRatio; + } else { + options.samplingRatio = 1; + this._configWarnings.push("Sampling percentage should be between 0 and 100. Defaulting to 100."); } options.instrumentationOptions = { ...options.instrumentationOptions, diff --git a/test/unitTests/shim/config.tests.ts b/test/unitTests/shim/config.tests.ts index ea12d0f6..ffba204f 100644 --- a/test/unitTests/shim/config.tests.ts +++ b/test/unitTests/shim/config.tests.ts @@ -102,6 +102,15 @@ describe("shim/configuration/config", () => { assert.equal(options.azureMonitorExporterOptions.disableOfflineStorage, false, "wrong disableOfflineStorage"); }); + it("should initialize zero sampling percentage", () => { + const config = new Config(connectionString); + config.samplingPercentage = 0; + + let options = config.parseConfig(); + + assert.equal(options.samplingRatio, 0, "wrong samplingRatio"); + }); + it("should activate DEBUG internal logger", () => { const env = <{ [id: string]: string }>{}; process.env = env; @@ -201,6 +210,14 @@ describe("shim/configuration/config", () => { assert.equal(process.env["APPLICATION_INSIGHTS_NO_STANDARD_METRICS"], "disable"); }); + it("should warn if an invalid sampling percentage is passed in", () => { + const config = new Config(connectionString); + const warnings = config["_configWarnings"]; + config.samplingPercentage = 101; + config.parseConfig(); + assert.ok(checkWarnings("Sampling percentage should be between 0 and 100. Defaulting to 100.", warnings), "warning was not raised"); + }); + describe("#Shim unsupported messages", () => { it("should warn if disableAppInsights is set", () => { const config = new Config(connectionString);