Skip to content

Commit

Permalink
Update Sampling Percentage to Accept 0 as Valid (#1337)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
JacksonWeber authored Jun 7, 2024
1 parent b07769c commit c98e546
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/shared/configuration/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down
8 changes: 6 additions & 2 deletions src/shim/shim-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions test/unitTests/shim/config.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c98e546

Please sign in to comment.