Skip to content

Commit

Permalink
[Azure Monitor OpenTelemetry] Fix issues with types (#26374)
Browse files Browse the repository at this point in the history 
Optional properties in InstrumentationOptions allow customer to only
specify settings they are interested
Typo in tracer provider method
  • Loading branch information
@hectorhdzg
hectorhdzg authored Jul 1, 2023
1 parent 1c8a7ce commit b74f4ba
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 94 deletions.
16 changes: 8 additions & 8 deletions sdk/monitor/monitor-opentelemetry/review/monitor-opentelemetry.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export class AzureMonitorOpenTelemetryClient {
getLoggerProvider(): LoggerProvider;
getMeter(): Meter;
getMeterProvider(): MeterProvider;
getTraceProvider(): TracerProvider;
getTracer(): Tracer;
getTracerProvider(): TracerProvider;
shutdown(): Promise<void>;
}

Expand All @@ -39,13 +39,13 @@ export interface AzureMonitorOpenTelemetryOptions {

// @public
export interface InstrumentationOptions {
azureSdk: InstrumentationConfig;
http: InstrumentationConfig;
mongoDb: InstrumentationConfig;
mySql: InstrumentationConfig;
postgreSql: InstrumentationConfig;
redis: InstrumentationConfig;
redis4: InstrumentationConfig;
azureSdk?: InstrumentationConfig;
http?: InstrumentationConfig;
mongoDb?: InstrumentationConfig;
mySql?: InstrumentationConfig;
postgreSql?: InstrumentationConfig;
redis?: InstrumentationConfig;
redis4?: InstrumentationConfig;
}

// (No @packageDocumentation comment for this package)
Expand Down
2 changes: 1 addition & 1 deletion sdk/monitor/monitor-opentelemetry/src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class AzureMonitorOpenTelemetryClient {
/**
*Get OpenTelemetry TracerProvider
*/
public getTraceProvider(): TracerProvider {
public getTracerProvider(): TracerProvider {
return this._traceHandler.getTracerProvider();
}

Expand Down
78 changes: 20 additions & 58 deletions sdk/monitor/monitor-opentelemetry/src/shared/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,28 @@ export class AzureMonitorOpenTelemetryConfig implements AzureMonitorOpenTelemetr
// Check for explicitly passed options when instantiating client
// This will take precedence over other settings
if (options) {
this.azureMonitorExporterConfig =
options.azureMonitorExporterConfig || this.azureMonitorExporterConfig;
// Merge default with provided options
this.azureMonitorExporterConfig = Object.assign(
this.azureMonitorExporterConfig,
options.azureMonitorExporterConfig
);
this.instrumentationOptions = Object.assign(
this.instrumentationOptions,
options.instrumentationOptions
);
this.resource = Object.assign(this.resource, options.resource);

this.enableAutoCollectPerformance =
options.enableAutoCollectPerformance || this.enableAutoCollectPerformance;
this.enableAutoCollectStandardMetrics =
options.enableAutoCollectStandardMetrics || this.enableAutoCollectStandardMetrics;
this.samplingRatio = options.samplingRatio || this.samplingRatio;
this.instrumentationOptions = options.instrumentationOptions || this.instrumentationOptions;
this.resource = options.resource || this.resource;
}
}

private _mergeConfig() {
try {
const jsonConfig = JsonConfig.getInstance();
this.azureMonitorExporterConfig =
jsonConfig.azureMonitorExporterConfig !== undefined
? jsonConfig.azureMonitorExporterConfig
: this.azureMonitorExporterConfig;
this.enableAutoCollectPerformance =
jsonConfig.enableAutoCollectPerformance !== undefined
? jsonConfig.enableAutoCollectPerformance
Expand All @@ -101,56 +104,15 @@ export class AzureMonitorOpenTelemetryConfig implements AzureMonitorOpenTelemetr
: this.enableAutoCollectStandardMetrics;
this.samplingRatio =
jsonConfig.samplingRatio !== undefined ? jsonConfig.samplingRatio : this.samplingRatio;
if (jsonConfig.instrumentationOptions) {
if (
jsonConfig.instrumentationOptions.azureSdk &&
jsonConfig.instrumentationOptions.azureSdk.enabled !== undefined
) {
this.instrumentationOptions.azureSdk.enabled =
jsonConfig.instrumentationOptions.azureSdk.enabled;
}
if (
jsonConfig.instrumentationOptions.http &&
jsonConfig.instrumentationOptions.http.enabled !== undefined
) {
this.instrumentationOptions.http.enabled = jsonConfig.instrumentationOptions.http.enabled;
}
if (
jsonConfig.instrumentationOptions.mongoDb &&
jsonConfig.instrumentationOptions.mongoDb.enabled !== undefined
) {
this.instrumentationOptions.mongoDb.enabled =
jsonConfig.instrumentationOptions.mongoDb.enabled;
}
if (
jsonConfig.instrumentationOptions.mySql &&
jsonConfig.instrumentationOptions.mySql.enabled !== undefined
) {
this.instrumentationOptions.mySql.enabled =
jsonConfig.instrumentationOptions.mySql.enabled;
}
if (
jsonConfig.instrumentationOptions.postgreSql &&
jsonConfig.instrumentationOptions.postgreSql.enabled !== undefined
) {
this.instrumentationOptions.postgreSql.enabled =
jsonConfig.instrumentationOptions.postgreSql.enabled;
}
if (
jsonConfig.instrumentationOptions.redis4 &&
jsonConfig.instrumentationOptions.redis4.enabled !== undefined
) {
this.instrumentationOptions.redis4.enabled =
jsonConfig.instrumentationOptions.redis4.enabled;
}
if (
jsonConfig.instrumentationOptions.redis &&
jsonConfig.instrumentationOptions.redis.enabled !== undefined
) {
this.instrumentationOptions.redis.enabled =
jsonConfig.instrumentationOptions.redis.enabled;
}
}

this.azureMonitorExporterConfig = Object.assign(
this.azureMonitorExporterConfig,
jsonConfig.azureMonitorExporterConfig
);
this.instrumentationOptions = Object.assign(
this.instrumentationOptions,
jsonConfig.instrumentationOptions
);
} catch (error) {
Logger.getInstance().error("Failed to load JSON config file values.", error);
}
Expand Down
14 changes: 7 additions & 7 deletions sdk/monitor/monitor-opentelemetry/src/shared/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ export interface AzureMonitorOpenTelemetryOptions {
*/
export interface InstrumentationOptions {
/** Azure SDK Instrumentation Config */
azureSdk: InstrumentationConfig;
azureSdk?: InstrumentationConfig;
/** HTTP Instrumentation Config */
http: InstrumentationConfig;
http?: InstrumentationConfig;
/** MongoDB Instrumentation Config */
mongoDb: InstrumentationConfig;
mongoDb?: InstrumentationConfig;
/** MySQL Instrumentation Config */
mySql: InstrumentationConfig;
mySql?: InstrumentationConfig;
/** PostgreSql Instrumentation Config */
postgreSql: InstrumentationConfig;
postgreSql?: InstrumentationConfig;
/** Redis Instrumentation Config */
redis: InstrumentationConfig;
redis?: InstrumentationConfig;
/** Redis4 Instrumentation Config */
redis4: InstrumentationConfig;
redis4?: InstrumentationConfig;
}
3 changes: 1 addition & 2 deletions sdk/monitor/monitor-opentelemetry/src/traces/handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
SpanProcessor,
Tracer,
} from "@opentelemetry/sdk-trace-base";
import { TracerProvider } from "@opentelemetry/api";
import { Instrumentation } from "@opentelemetry/instrumentation";
import {
HttpInstrumentation,
Expand Down Expand Up @@ -92,7 +91,7 @@ export class TraceHandler {
/**
*Get OpenTelemetry TracerProvider
*/
public getTracerProvider(): TracerProvider {
public getTracerProvider(): NodeTracerProvider {
return this._tracerProvider;
}

Expand Down
28 changes: 16 additions & 12 deletions sdk/monitor/monitor-opentelemetry/test/internal/unit/shared/config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ describe("Library/Config", () => {
"Wrong enableAutoCollectStandardMetrics"
);
assert.deepStrictEqual(
config.instrumentationOptions.azureSdk.enabled,
config.instrumentationOptions.azureSdk?.enabled,
true,
"Wrong azureSdk"
);
assert.deepStrictEqual(config.instrumentationOptions.mongoDb.enabled, true, "Wrong mongoDb");
assert.deepStrictEqual(config.instrumentationOptions.mySql.enabled, true, "Wrong mySql");
assert.deepStrictEqual(config.instrumentationOptions.mongoDb?.enabled, true, "Wrong mongoDb");
assert.deepStrictEqual(config.instrumentationOptions.mySql?.enabled, true, "Wrong mySql");
assert.deepStrictEqual(
config.instrumentationOptions.postgreSql.enabled,
config.instrumentationOptions.postgreSql?.enabled,
true,
"Wrong postgreSql"
);
assert.deepStrictEqual(config.instrumentationOptions.redis.enabled, true, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions.redis4.enabled, true, "Wrong redis4");
assert.deepStrictEqual(config.instrumentationOptions.redis?.enabled, true, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions.redis4?.enabled, true, "Wrong redis4");
});

it("Default config", () => {
Expand All @@ -93,19 +93,23 @@ describe("Library/Config", () => {
"Wrong enableAutoCollectStandardMetrics"
);
assert.deepStrictEqual(
config.instrumentationOptions.azureSdk.enabled,
config.instrumentationOptions.azureSdk?.enabled,
false,
"Wrong azureSdk"
);
assert.deepStrictEqual(config.instrumentationOptions.mongoDb.enabled, false, "Wrong mongoDb");
assert.deepStrictEqual(config.instrumentationOptions.mySql.enabled, false, "Wrong mySql");
assert.deepStrictEqual(
config.instrumentationOptions.postgreSql.enabled,
config.instrumentationOptions.mongoDb?.enabled,
false,
"Wrong mongoDb"
);
assert.deepStrictEqual(config.instrumentationOptions.mySql?.enabled, false, "Wrong mySql");
assert.deepStrictEqual(
config.instrumentationOptions.postgreSql?.enabled,
false,
"Wrong postgreSql"
);
assert.deepStrictEqual(config.instrumentationOptions.redis.enabled, false, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions.redis4.enabled, false, "Wrong redis4");
assert.deepStrictEqual(config.instrumentationOptions.redis?.enabled, false, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions.redis4?.enabled, false, "Wrong redis4");
assert.deepStrictEqual(
config.azureMonitorExporterConfig?.disableOfflineStorage,
undefined,
Expand Down
16 changes: 10 additions & 6 deletions sdk/monitor/monitor-opentelemetry/test/internal/unit/shared/jsonConfig.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,23 @@ describe("Json Config", () => {
"Wrong enableAutoCollectStandardMetrics"
);
assert.deepStrictEqual(
config.instrumentationOptions?.azureSdk.enabled,
config.instrumentationOptions?.azureSdk?.enabled,
true,
"Wrong azureSdk"
);
assert.deepStrictEqual(config.instrumentationOptions?.mongoDb.enabled, true, "Wrong mongoDb");
assert.deepStrictEqual(config.instrumentationOptions?.mySql.enabled, true, "Wrong mySql");
assert.deepStrictEqual(
config.instrumentationOptions?.postgreSql.enabled,
config.instrumentationOptions?.mongoDb?.enabled,
true,
"Wrong mongoDb"
);
assert.deepStrictEqual(config.instrumentationOptions?.mySql?.enabled, true, "Wrong mySql");
assert.deepStrictEqual(
config.instrumentationOptions?.postgreSql?.enabled,
true,
"Wrong postgreSql"
);
assert.deepStrictEqual(config.instrumentationOptions?.redis.enabled, true, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions?.redis4.enabled, true, "Wrong redis4");
assert.deepStrictEqual(config.instrumentationOptions?.redis?.enabled, true, "Wrong redis");
assert.deepStrictEqual(config.instrumentationOptions?.redis4?.enabled, true, "Wrong redis4");
});

it("Should take configurations from JSON config file over environment variables if both are configured", () => {
Expand Down

0 comments on commit b74f4ba

Please sign in to comment.