From 46870f941138e974f99cbb88351546b0fe75650c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:21:31 +0000 Subject: [PATCH 1/6] Initial plan From 82819f317a38ffcde04acc230b3e3843b88377fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:30:53 +0000 Subject: [PATCH 2/6] Enable observability by default with console logging Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com> --- .../src/ObservabilityBuilder.ts | 21 +++ .../src/tracing/exporter/utils.ts | 12 +- .../src/tracing/util.ts | 53 ++++++-- .../core/observabilityBuilder-options.test.ts | 17 ++- tests/observability/core/util.test.ts | 123 ++++++++++++++++++ 5 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 tests/observability/core/util.test.ts diff --git a/packages/agents-a365-observability/src/ObservabilityBuilder.ts b/packages/agents-a365-observability/src/ObservabilityBuilder.ts index 08d23cce..09a7b22e 100644 --- a/packages/agents-a365-observability/src/ObservabilityBuilder.ts +++ b/packages/agents-a365-observability/src/ObservabilityBuilder.ts @@ -13,6 +13,7 @@ import { resourceFromAttributes } from '@opentelemetry/resources'; import { ATTR_SERVICE_NAME } from '@opentelemetry/semantic-conventions'; import { trace } from '@opentelemetry/api'; import { ClusterCategory } from '@microsoft/agents-a365-runtime'; +import { OpenTelemetryConstants } from './tracing/constants'; /** * Configuration options for Agent 365 Observability Builder */ @@ -91,6 +92,20 @@ export class ObservabilityBuilder { } private createBatchProcessor(): BatchSpanProcessor { + // Check if we should use Agent365Exporter: + // 1. tokenResolver is explicitly provided via withTokenResolver, OR + // 2. tokenResolver is provided via exporterOptions, OR + // 3. ENABLE_A365_OBSERVABILITY_EXPORTER is explicitly enabled (for backward compatibility) + const hasTokenResolver = this.options.tokenResolver || this.options.exporterOptions?.tokenResolver; + const isExporterExplicitlyEnabled = isAgent365ExporterEnabled() && + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; + + // If no token resolver and exporter is not explicitly enabled, use console exporter + if (!hasTokenResolver && !isExporterExplicitlyEnabled) { + return new BatchSpanProcessor(new ConsoleSpanExporter()); + } + + // If explicitly disabled via env var, use console exporter if (!isAgent365ExporterEnabled()) { return new BatchSpanProcessor(new ConsoleSpanExporter()); } @@ -103,6 +118,12 @@ export class ObservabilityBuilder { if (this.options.tokenResolver) { opts.tokenResolver = this.options.tokenResolver; } + + // If we still don't have a token resolver at this point, use console exporter + if (!opts.tokenResolver) { + return new BatchSpanProcessor(new ConsoleSpanExporter()); + } + return new BatchSpanProcessor(new Agent365Exporter(opts), { maxQueueSize: opts.maxQueueSize, scheduledDelayMillis: opts.scheduledDelayMilliseconds, diff --git a/packages/agents-a365-observability/src/tracing/exporter/utils.ts b/packages/agents-a365-observability/src/tracing/exporter/utils.ts index 1a63dfc1..b2741c39 100644 --- a/packages/agents-a365-observability/src/tracing/exporter/utils.ts +++ b/packages/agents-a365-observability/src/tracing/exporter/utils.ts @@ -110,11 +110,19 @@ export function partitionByIdentity( /** * Check if Agent 365 exporter is enabled via environment variable + * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' */ export function isAgent365ExporterEnabled(): boolean { const a365Env = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]?.toLowerCase() || ''; - const validValues = ['true', '1', 'yes', 'on']; - const enabled: boolean = validValues.includes(a365Env); + + // If not set, default to enabled (true) + if (!a365Env) { + logger.info('[Agent365Exporter] Agent 365 exporter enabled: true (default)'); + return true; + } + + const disabledValues = ['false', '0', 'no', 'off']; + const enabled: boolean = !disabledValues.includes(a365Env); logger.info(`[Agent365Exporter] Agent 365 exporter enabled: ${enabled}`); return enabled; } diff --git a/packages/agents-a365-observability/src/tracing/util.ts b/packages/agents-a365-observability/src/tracing/util.ts index e8568fed..8059abab 100644 --- a/packages/agents-a365-observability/src/tracing/util.ts +++ b/packages/agents-a365-observability/src/tracing/util.ts @@ -7,31 +7,62 @@ import { OpenTelemetryConstants } from './constants'; import { ClusterCategory } from '@microsoft/agents-a365-runtime'; /** * Check if exporter is enabled via environment variables + * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' */ export const isAgent365ExporterEnabled: () => boolean = (): boolean => { const enableA365Exporter = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]?.toLowerCase(); - return ( - enableA365Exporter === 'true' || - enableA365Exporter === '1' || - enableA365Exporter === 'yes' || - enableA365Exporter === 'on' + // If not set, default to enabled (true) + if (!enableA365Exporter) { + return true; + } + + // Only return false if explicitly disabled + return !( + enableA365Exporter === 'false' || + enableA365Exporter === '0' || + enableA365Exporter === 'no' || + enableA365Exporter === 'off' ); }; /** * Gets the enable telemetry configuration value + * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' */ export const isAgent365TelemetryEnabled: () => boolean = (): boolean => { const enableObservability = process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY]?.toLowerCase(); const enableA365 = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY]?.toLowerCase(); - return ( - enableObservability === 'true' || - enableObservability === '1' || - enableA365 === 'true' || - enableA365 === '1' - ); + // If neither is set, default to enabled (true) + if (!enableObservability && !enableA365) { + return true; + } + + // Check if explicitly disabled + const isObservabilityDisabled = + enableObservability === 'false' || + enableObservability === '0' || + enableObservability === 'no' || + enableObservability === 'off'; + + const isA365Disabled = + enableA365 === 'false' || + enableA365 === '0' || + enableA365 === 'no' || + enableA365 === 'off'; + + // If either is explicitly set and not disabled, return true + // If both are explicitly set, both must not be disabled + if (enableObservability && enableA365) { + return !isObservabilityDisabled && !isA365Disabled; + } else if (enableObservability) { + return !isObservabilityDisabled; + } else if (enableA365) { + return !isA365Disabled; + } + + return true; }; /** diff --git a/tests/observability/core/observabilityBuilder-options.test.ts b/tests/observability/core/observabilityBuilder-options.test.ts index 1da05640..199f277f 100644 --- a/tests/observability/core/observabilityBuilder-options.test.ts +++ b/tests/observability/core/observabilityBuilder-options.test.ts @@ -66,12 +66,25 @@ describe('ObservabilityBuilder exporterOptions merging', () => { it('defaults to prod clusterCategory when none provided', () => { const builder = new ObservabilityBuilder() - .withExporterOptions({ maxQueueSize: 15 }); // no cluster category passed + .withExporterOptions({ maxQueueSize: 15 }) // no cluster category passed + .withTokenResolver(() => 'test-token'); // Add token resolver so Agent365Exporter is used builder.build(); const captured: any = (global as any).__capturedExporterOptions; expect(captured.clusterCategory).toBe('prod'); expect(captured.maxQueueSize).toBe(15); expect(captured.scheduledDelayMilliseconds).toBe(5000); // default value - }); + }); + + it('uses ConsoleSpanExporter when no tokenResolver is provided', () => { + const builder = new ObservabilityBuilder() + .withExporterOptions({ maxQueueSize: 15 }); // no token resolver + + const built = builder.build(); + expect(built).toBe(true); + + // Since no tokenResolver was provided, Agent365Exporter should NOT be created + const captured: any = (global as any).__capturedExporterOptions; + expect(captured).toBeUndefined(); + }); }); diff --git a/tests/observability/core/util.test.ts b/tests/observability/core/util.test.ts new file mode 100644 index 00000000..869766ea --- /dev/null +++ b/tests/observability/core/util.test.ts @@ -0,0 +1,123 @@ +// ------------------------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +// ------------------------------------------------------------------------------ + +import { isAgent365TelemetryEnabled, isAgent365ExporterEnabled } from '@microsoft/agents-a365-observability/src/tracing/util'; +import { OpenTelemetryConstants } from '@microsoft/agents-a365-observability/src/tracing/constants'; + +describe('Observability Utility Functions', () => { + describe('isAgent365TelemetryEnabled', () => { + beforeEach(() => { + // Clear all relevant environment variables before each test + delete process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY]; + delete process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY]; + }); + + it('should return true by default when no environment variables are set', () => { + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + + it('should return false when ENABLE_OBSERVABILITY is explicitly set to false', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'false'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return false when ENABLE_OBSERVABILITY is set to 0', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = '0'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return false when ENABLE_OBSERVABILITY is set to no', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'no'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return false when ENABLE_OBSERVABILITY is set to off', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'off'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return true when ENABLE_OBSERVABILITY is set to true', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'true'; + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + + it('should return true when ENABLE_OBSERVABILITY is set to 1', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = '1'; + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + + it('should return false when ENABLE_A365_OBSERVABILITY is explicitly set to false', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY] = 'false'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return true when ENABLE_A365_OBSERVABILITY is set to true', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY] = 'true'; + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + + it('should return false when both are explicitly disabled', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'false'; + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY] = 'false'; + expect(isAgent365TelemetryEnabled()).toBe(false); + }); + + it('should return true when ENABLE_OBSERVABILITY is enabled and ENABLE_A365_OBSERVABILITY is not set', () => { + process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY] = 'true'; + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + + it('should return true when ENABLE_A365_OBSERVABILITY is enabled and ENABLE_OBSERVABILITY is not set', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY] = 'true'; + expect(isAgent365TelemetryEnabled()).toBe(true); + }); + }); + + describe('isAgent365ExporterEnabled', () => { + beforeEach(() => { + // Clear environment variable before each test + delete process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; + }); + + it('should return true by default when environment variable is not set', () => { + expect(isAgent365ExporterEnabled()).toBe(true); + }); + + it('should return false when explicitly set to false', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'false'; + expect(isAgent365ExporterEnabled()).toBe(false); + }); + + it('should return false when set to 0', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = '0'; + expect(isAgent365ExporterEnabled()).toBe(false); + }); + + it('should return false when set to no', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'no'; + expect(isAgent365ExporterEnabled()).toBe(false); + }); + + it('should return false when set to off', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'off'; + expect(isAgent365ExporterEnabled()).toBe(false); + }); + + it('should return true when set to true', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'true'; + expect(isAgent365ExporterEnabled()).toBe(true); + }); + + it('should return true when set to 1', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = '1'; + expect(isAgent365ExporterEnabled()).toBe(true); + }); + + it('should return true when set to any other value', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'yes'; + expect(isAgent365ExporterEnabled()).toBe(true); + }); + }); +}); From 8be131b8862082c58a56365faecdda5fb7622ce1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:37:54 +0000 Subject: [PATCH 3/6] Address code review feedback - refactor duplicate logic Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com> --- .../src/ObservabilityBuilder.ts | 26 ++++-------- .../src/tracing/util.ts | 41 ++++++++++--------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/packages/agents-a365-observability/src/ObservabilityBuilder.ts b/packages/agents-a365-observability/src/ObservabilityBuilder.ts index 09a7b22e..754dfa0a 100644 --- a/packages/agents-a365-observability/src/ObservabilityBuilder.ts +++ b/packages/agents-a365-observability/src/ObservabilityBuilder.ts @@ -13,7 +13,6 @@ import { resourceFromAttributes } from '@opentelemetry/resources'; import { ATTR_SERVICE_NAME } from '@opentelemetry/semantic-conventions'; import { trace } from '@opentelemetry/api'; import { ClusterCategory } from '@microsoft/agents-a365-runtime'; -import { OpenTelemetryConstants } from './tracing/constants'; /** * Configuration options for Agent 365 Observability Builder */ @@ -92,24 +91,20 @@ export class ObservabilityBuilder { } private createBatchProcessor(): BatchSpanProcessor { - // Check if we should use Agent365Exporter: - // 1. tokenResolver is explicitly provided via withTokenResolver, OR - // 2. tokenResolver is provided via exporterOptions, OR - // 3. ENABLE_A365_OBSERVABILITY_EXPORTER is explicitly enabled (for backward compatibility) - const hasTokenResolver = this.options.tokenResolver || this.options.exporterOptions?.tokenResolver; - const isExporterExplicitlyEnabled = isAgent365ExporterEnabled() && - process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; - - // If no token resolver and exporter is not explicitly enabled, use console exporter - if (!hasTokenResolver && !isExporterExplicitlyEnabled) { + // If explicitly disabled via env var, use console exporter + if (!isAgent365ExporterEnabled()) { return new BatchSpanProcessor(new ConsoleSpanExporter()); } - // If explicitly disabled via env var, use console exporter - if (!isAgent365ExporterEnabled()) { + // Check if we have a token resolver (needed for Agent365Exporter) + const hasTokenResolver = this.options.tokenResolver || this.options.exporterOptions?.tokenResolver; + + // If no token resolver is provided, use console exporter + if (!hasTokenResolver) { return new BatchSpanProcessor(new ConsoleSpanExporter()); } + // Use Agent365Exporter when token resolver is available const opts = new Agent365ExporterOptions(); if (this.options.exporterOptions) { Object.assign(opts, this.options.exporterOptions); @@ -119,11 +114,6 @@ export class ObservabilityBuilder { opts.tokenResolver = this.options.tokenResolver; } - // If we still don't have a token resolver at this point, use console exporter - if (!opts.tokenResolver) { - return new BatchSpanProcessor(new ConsoleSpanExporter()); - } - return new BatchSpanProcessor(new Agent365Exporter(opts), { maxQueueSize: opts.maxQueueSize, scheduledDelayMillis: opts.scheduledDelayMilliseconds, diff --git a/packages/agents-a365-observability/src/tracing/util.ts b/packages/agents-a365-observability/src/tracing/util.ts index 8059abab..830db1f5 100644 --- a/packages/agents-a365-observability/src/tracing/util.ts +++ b/packages/agents-a365-observability/src/tracing/util.ts @@ -5,12 +5,27 @@ import { OpenTelemetryConstants } from './constants'; import { ClusterCategory } from '@microsoft/agents-a365-runtime'; + +/** + * Helper function to check if a value is explicitly disabled + */ +const isExplicitlyDisabled = (value: string | undefined): boolean => { + if (!value) return false; + const lowerValue = value.toLowerCase(); + return ( + lowerValue === 'false' || + lowerValue === '0' || + lowerValue === 'no' || + lowerValue === 'off' + ); +}; + /** * Check if exporter is enabled via environment variables * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' */ export const isAgent365ExporterEnabled: () => boolean = (): boolean => { - const enableA365Exporter = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]?.toLowerCase(); + const enableA365Exporter = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; // If not set, default to enabled (true) if (!enableA365Exporter) { @@ -18,12 +33,7 @@ export const isAgent365ExporterEnabled: () => boolean = (): boolean => { } // Only return false if explicitly disabled - return !( - enableA365Exporter === 'false' || - enableA365Exporter === '0' || - enableA365Exporter === 'no' || - enableA365Exporter === 'off' - ); + return !isExplicitlyDisabled(enableA365Exporter); }; /** @@ -31,8 +41,8 @@ export const isAgent365ExporterEnabled: () => boolean = (): boolean => { * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' */ export const isAgent365TelemetryEnabled: () => boolean = (): boolean => { - const enableObservability = process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY]?.toLowerCase(); - const enableA365 = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY]?.toLowerCase(); + const enableObservability = process.env[OpenTelemetryConstants.ENABLE_OBSERVABILITY]; + const enableA365 = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY]; // If neither is set, default to enabled (true) if (!enableObservability && !enableA365) { @@ -40,17 +50,8 @@ export const isAgent365TelemetryEnabled: () => boolean = (): boolean => { } // Check if explicitly disabled - const isObservabilityDisabled = - enableObservability === 'false' || - enableObservability === '0' || - enableObservability === 'no' || - enableObservability === 'off'; - - const isA365Disabled = - enableA365 === 'false' || - enableA365 === '0' || - enableA365 === 'no' || - enableA365 === 'off'; + const isObservabilityDisabled = isExplicitlyDisabled(enableObservability); + const isA365Disabled = isExplicitlyDisabled(enableA365); // If either is explicitly set and not disabled, return true // If both are explicitly set, both must not be disabled From 99042d0399dcb0f548a6098cee8341d9eeb5c5ea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:40:42 +0000 Subject: [PATCH 4/6] Further refactor to eliminate code duplication and simplify logic Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com> --- .../src/tracing/exporter/utils.ts | 19 +++++++--------- .../src/tracing/util.ts | 22 ++++++------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/packages/agents-a365-observability/src/tracing/exporter/utils.ts b/packages/agents-a365-observability/src/tracing/exporter/utils.ts index b2741c39..667d60fb 100644 --- a/packages/agents-a365-observability/src/tracing/exporter/utils.ts +++ b/packages/agents-a365-observability/src/tracing/exporter/utils.ts @@ -6,6 +6,7 @@ import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { SpanKind, SpanStatusCode } from '@opentelemetry/api'; import { OpenTelemetryConstants } from '../constants'; import logger from '../../utils/logging'; +import { isAgent365ExporterEnabled as isExporterEnabled } from '../util'; /** * Convert trace ID to hex string format @@ -111,19 +112,15 @@ export function partitionByIdentity( /** * Check if Agent 365 exporter is enabled via environment variable * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' + * This wrapper adds logging for debugging purposes */ export function isAgent365ExporterEnabled(): boolean { - const a365Env = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]?.toLowerCase() || ''; - - // If not set, default to enabled (true) - if (!a365Env) { - logger.info('[Agent365Exporter] Agent 365 exporter enabled: true (default)'); - return true; - } - - const disabledValues = ['false', '0', 'no', 'off']; - const enabled: boolean = !disabledValues.includes(a365Env); - logger.info(`[Agent365Exporter] Agent 365 exporter enabled: ${enabled}`); + const enabled = isExporterEnabled(); + const envVar = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; + const message = envVar + ? `[Agent365Exporter] Agent 365 exporter enabled: ${enabled} (env var: ${envVar})` + : '[Agent365Exporter] Agent 365 exporter enabled: true (default)'; + logger.info(message); return enabled; } diff --git a/packages/agents-a365-observability/src/tracing/util.ts b/packages/agents-a365-observability/src/tracing/util.ts index 830db1f5..2e361573 100644 --- a/packages/agents-a365-observability/src/tracing/util.ts +++ b/packages/agents-a365-observability/src/tracing/util.ts @@ -49,21 +49,13 @@ export const isAgent365TelemetryEnabled: () => boolean = (): boolean => { return true; } - // Check if explicitly disabled - const isObservabilityDisabled = isExplicitlyDisabled(enableObservability); - const isA365Disabled = isExplicitlyDisabled(enableA365); - - // If either is explicitly set and not disabled, return true - // If both are explicitly set, both must not be disabled - if (enableObservability && enableA365) { - return !isObservabilityDisabled && !isA365Disabled; - } else if (enableObservability) { - return !isObservabilityDisabled; - } else if (enableA365) { - return !isA365Disabled; - } - - return true; + // If both are set, both must not be disabled + // If only one is set, it must not be disabled + return enableObservability && enableA365 + ? !isExplicitlyDisabled(enableObservability) && !isExplicitlyDisabled(enableA365) + : enableObservability + ? !isExplicitlyDisabled(enableObservability) + : !isExplicitlyDisabled(enableA365); }; /** From fee7e8e92c59408bd51ba86966e2cf69b40040a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 00:15:57 +0000 Subject: [PATCH 5/6] Remove outdated env var references from comments and tests Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com> --- .../src/tracing/exporter/Agent365ExporterOptions.ts | 4 ++-- .../core/observabilityBuilder-options.test.ts | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/agents-a365-observability/src/tracing/exporter/Agent365ExporterOptions.ts b/packages/agents-a365-observability/src/tracing/exporter/Agent365ExporterOptions.ts index 547b4e47..9b5db4c0 100644 --- a/packages/agents-a365-observability/src/tracing/exporter/Agent365ExporterOptions.ts +++ b/packages/agents-a365-observability/src/tracing/exporter/Agent365ExporterOptions.ts @@ -29,8 +29,8 @@ export class Agent365ExporterOptions { /** Environment / cluster category (e.g. "preprod", "prod"). */ public clusterCategory: ClusterCategory | string = 'prod'; - /** Optional delegate to resolve auth token used by exporter */ - public tokenResolver?: TokenResolver; // Optional if ENABLE_A365_OBSERVABILITY_EXPORTER is false + /** Optional delegate to resolve auth token used by exporter. Required to send telemetry to Agent365 service; if not provided, telemetry is logged to console. */ + public tokenResolver?: TokenResolver; /** Maximum span queue size before new spans are dropped. */ public maxQueueSize: number = 2048; diff --git a/tests/observability/core/observabilityBuilder-options.test.ts b/tests/observability/core/observabilityBuilder-options.test.ts index 199f277f..2ed5d136 100644 --- a/tests/observability/core/observabilityBuilder-options.test.ts +++ b/tests/observability/core/observabilityBuilder-options.test.ts @@ -24,16 +24,11 @@ jest.mock('@microsoft/agents-a365-observability/src/tracing/exporter/Agent365Exp describe('ObservabilityBuilder exporterOptions merging', () => { beforeEach(() => { - // Ensure exporter is enabled so BatchSpanProcessor is created with Agent365Exporter - process.env.ENABLE_A365_OBSERVABILITY_EXPORTER = 'true'; + // Clean up any captured options from previous tests delete (global as any).__capturedExporterOptions; delete (global as any).__capturedExporterOptionsCallCount; }); - afterEach(() => { - delete process.env.ENABLE_A365_OBSERVABILITY_EXPORTER; - }); - it('applies provided exporterOptions and allows builder overrides to take precedence', () => { const builder = new ObservabilityBuilder() .withExporterOptions({ From 988da1852baf3a1ca2af98ecf52043254594eb35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 00:22:32 +0000 Subject: [PATCH 6/6] Revert ENABLE_A365_OBSERVABILITY_EXPORTER to require explicit enabling Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com> --- .../src/ObservabilityBuilder.ts | 48 +++++++++---------- .../src/tracing/exporter/utils.ts | 4 +- .../src/tracing/util.ts | 17 ++++--- .../core/observabilityBuilder-options.test.ts | 24 ++++++++++ tests/observability/core/util.test.ts | 11 +++-- 5 files changed, 65 insertions(+), 39 deletions(-) diff --git a/packages/agents-a365-observability/src/ObservabilityBuilder.ts b/packages/agents-a365-observability/src/ObservabilityBuilder.ts index 754dfa0a..1c1604f9 100644 --- a/packages/agents-a365-observability/src/ObservabilityBuilder.ts +++ b/packages/agents-a365-observability/src/ObservabilityBuilder.ts @@ -91,35 +91,33 @@ export class ObservabilityBuilder { } private createBatchProcessor(): BatchSpanProcessor { - // If explicitly disabled via env var, use console exporter - if (!isAgent365ExporterEnabled()) { - return new BatchSpanProcessor(new ConsoleSpanExporter()); - } - - // Check if we have a token resolver (needed for Agent365Exporter) + // To send telemetry to Agent365 service, BOTH conditions must be met: + // 1. ENABLE_A365_OBSERVABILITY_EXPORTER=true must be explicitly set + // 2. A tokenResolver must be provided + const isExporterEnabled = isAgent365ExporterEnabled(); const hasTokenResolver = this.options.tokenResolver || this.options.exporterOptions?.tokenResolver; - // If no token resolver is provided, use console exporter - if (!hasTokenResolver) { - return new BatchSpanProcessor(new ConsoleSpanExporter()); - } - - // Use Agent365Exporter when token resolver is available - const opts = new Agent365ExporterOptions(); - if (this.options.exporterOptions) { - Object.assign(opts, this.options.exporterOptions); - } - opts.clusterCategory = this.options.clusterCategory || opts.clusterCategory || 'prod'; - if (this.options.tokenResolver) { - opts.tokenResolver = this.options.tokenResolver; + // Use Agent365Exporter only if both exporter is enabled AND token resolver is available + if (isExporterEnabled && hasTokenResolver) { + const opts = new Agent365ExporterOptions(); + if (this.options.exporterOptions) { + Object.assign(opts, this.options.exporterOptions); + } + opts.clusterCategory = this.options.clusterCategory || opts.clusterCategory || 'prod'; + if (this.options.tokenResolver) { + opts.tokenResolver = this.options.tokenResolver; + } + + return new BatchSpanProcessor(new Agent365Exporter(opts), { + maxQueueSize: opts.maxQueueSize, + scheduledDelayMillis: opts.scheduledDelayMilliseconds, + exportTimeoutMillis: opts.exporterTimeoutMilliseconds, + maxExportBatchSize: opts.maxExportBatchSize + }); } - return new BatchSpanProcessor(new Agent365Exporter(opts), { - maxQueueSize: opts.maxQueueSize, - scheduledDelayMillis: opts.scheduledDelayMilliseconds, - exportTimeoutMillis: opts.exporterTimeoutMilliseconds, - maxExportBatchSize: opts.maxExportBatchSize - }); + // Default: use console exporter (for local development and when service export is not configured) + return new BatchSpanProcessor(new ConsoleSpanExporter()); } private createResource() { diff --git a/packages/agents-a365-observability/src/tracing/exporter/utils.ts b/packages/agents-a365-observability/src/tracing/exporter/utils.ts index 667d60fb..40c0fcc6 100644 --- a/packages/agents-a365-observability/src/tracing/exporter/utils.ts +++ b/packages/agents-a365-observability/src/tracing/exporter/utils.ts @@ -111,7 +111,7 @@ export function partitionByIdentity( /** * Check if Agent 365 exporter is enabled via environment variable - * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' + * Requires explicit enabling by setting to 'true', '1', 'yes', or 'on' * This wrapper adds logging for debugging purposes */ export function isAgent365ExporterEnabled(): boolean { @@ -119,7 +119,7 @@ export function isAgent365ExporterEnabled(): boolean { const envVar = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; const message = envVar ? `[Agent365Exporter] Agent 365 exporter enabled: ${enabled} (env var: ${envVar})` - : '[Agent365Exporter] Agent 365 exporter enabled: true (default)'; + : '[Agent365Exporter] Agent 365 exporter enabled: false (not set, requires explicit enabling)'; logger.info(message); return enabled; } diff --git a/packages/agents-a365-observability/src/tracing/util.ts b/packages/agents-a365-observability/src/tracing/util.ts index 2e361573..afe02cee 100644 --- a/packages/agents-a365-observability/src/tracing/util.ts +++ b/packages/agents-a365-observability/src/tracing/util.ts @@ -22,18 +22,17 @@ const isExplicitlyDisabled = (value: string | undefined): boolean => { /** * Check if exporter is enabled via environment variables - * Enabled by default, can be disabled by setting to 'false', '0', 'no', or 'off' + * Requires explicit enabling by setting to 'true', '1', 'yes', or 'on' */ export const isAgent365ExporterEnabled: () => boolean = (): boolean => { - const enableA365Exporter = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; + const enableA365Exporter = process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]?.toLowerCase(); - // If not set, default to enabled (true) - if (!enableA365Exporter) { - return true; - } - - // Only return false if explicitly disabled - return !isExplicitlyDisabled(enableA365Exporter); + return ( + enableA365Exporter === 'true' || + enableA365Exporter === '1' || + enableA365Exporter === 'yes' || + enableA365Exporter === 'on' + ); }; /** diff --git a/tests/observability/core/observabilityBuilder-options.test.ts b/tests/observability/core/observabilityBuilder-options.test.ts index 2ed5d136..6b1e69b7 100644 --- a/tests/observability/core/observabilityBuilder-options.test.ts +++ b/tests/observability/core/observabilityBuilder-options.test.ts @@ -29,7 +29,15 @@ describe('ObservabilityBuilder exporterOptions merging', () => { delete (global as any).__capturedExporterOptionsCallCount; }); + afterEach(() => { + // Clean up environment variable after each test + delete process.env.ENABLE_A365_OBSERVABILITY_EXPORTER; + }); + it('applies provided exporterOptions and allows builder overrides to take precedence', () => { + // Enable Agent365 exporter to test the exporter options + process.env.ENABLE_A365_OBSERVABILITY_EXPORTER = 'true'; + const builder = new ObservabilityBuilder() .withExporterOptions({ maxQueueSize: 10, @@ -60,6 +68,9 @@ describe('ObservabilityBuilder exporterOptions merging', () => { }); it('defaults to prod clusterCategory when none provided', () => { + // Enable Agent365 exporter to test the exporter options + process.env.ENABLE_A365_OBSERVABILITY_EXPORTER = 'true'; + const builder = new ObservabilityBuilder() .withExporterOptions({ maxQueueSize: 15 }) // no cluster category passed .withTokenResolver(() => 'test-token'); // Add token resolver so Agent365Exporter is used @@ -82,4 +93,17 @@ describe('ObservabilityBuilder exporterOptions merging', () => { const captured: any = (global as any).__capturedExporterOptions; expect(captured).toBeUndefined(); }); + + it('uses ConsoleSpanExporter when ENABLE_A365_OBSERVABILITY_EXPORTER is not set', () => { + // Even with tokenResolver, if env var is not set, should use ConsoleSpanExporter + const builder = new ObservabilityBuilder() + .withTokenResolver(() => 'test-token'); + + const built = builder.build(); + expect(built).toBe(true); + + // Since ENABLE_A365_OBSERVABILITY_EXPORTER is not set, Agent365Exporter should NOT be created + const captured: any = (global as any).__capturedExporterOptions; + expect(captured).toBeUndefined(); + }); }); diff --git a/tests/observability/core/util.test.ts b/tests/observability/core/util.test.ts index 869766ea..c4521b14 100644 --- a/tests/observability/core/util.test.ts +++ b/tests/observability/core/util.test.ts @@ -81,8 +81,8 @@ describe('Observability Utility Functions', () => { delete process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER]; }); - it('should return true by default when environment variable is not set', () => { - expect(isAgent365ExporterEnabled()).toBe(true); + it('should return false by default when environment variable is not set', () => { + expect(isAgent365ExporterEnabled()).toBe(false); }); it('should return false when explicitly set to false', () => { @@ -115,9 +115,14 @@ describe('Observability Utility Functions', () => { expect(isAgent365ExporterEnabled()).toBe(true); }); - it('should return true when set to any other value', () => { + it('should return true when set to yes', () => { process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'yes'; expect(isAgent365ExporterEnabled()).toBe(true); }); + + it('should return true when set to on', () => { + process.env[OpenTelemetryConstants.ENABLE_A365_OBSERVABILITY_EXPORTER] = 'on'; + expect(isAgent365ExporterEnabled()).toBe(true); + }); }); });