Conversation
Resolved conflicts: - observability/src/index.ts: Combined ILogger exports with configuration exports - observability/src/utils/logging.ts: Merged setLogger/getLogger/resetLogger pattern with configuration provider support for DefaultLogger - tooling/src/McpToolServerConfigurationService.ts: Combined imports for IConfigurationProvider with Authorization and AgenticAuthenticationService - tooling/src/Utility.ts: Combined imports for IConfigurationProvider with ChannelAccount Updated tests to match merged implementation: - Removed createLogger factory tests (replaced by setLogger/getLogger/resetLogger) - Updated GetAgenticUserToken expectations to include scopes parameter Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The scopes parameter was added as required in main branch, which is a breaking change for existing consumers. This adds overloads to maintain backward compatibility: - 3-param signature (deprecated): Uses default MCP platform scope - 4-param signature: Explicit scopes for better control Existing code calling GetAgenticUserToken(auth, handler, ctx) will continue to work, but callers are encouraged to migrate to the explicit scopes overload for clarity. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…vider Added detailed JSDoc explaining: - The provider creates a single cached configuration instance - Default module-level providers are singletons - Two approaches for multi-tenant scenarios: 1. Dynamic override functions reading from async context (recommended) 2. Per-tenant provider instances when needed This clarifies that while the singleton pattern is used, multi-tenancy is still supported through function-based overrides that resolve at runtime. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added documentation to AgenticTokenCacheInstance explaining it uses default configuration and when to create custom instances - Exported AgenticTokenCache class so consumers can create instances with custom IConfigurationProvider for multi-tenant scenarios - Added class-level documentation with usage example Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…onment utility functions
…onfigurations for LangChain and OpenAI
… utility imports and updating server listing logic
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive hierarchical configuration provider pattern throughout the Agents365 SDK, replacing direct process.env access with configuration classes that support programmatic overrides and multi-tenant scenarios. The PR adds new configuration classes (RuntimeConfiguration, ToolingConfiguration, ObservabilityConfiguration) with default singleton providers, applies these patterns across runtime, tooling, and observability packages, and introduces extensive test coverage.
Changes:
- Introduces configuration provider pattern with base
RuntimeConfigurationclass and package-specific extensions - Adds comprehensive test coverage for all configuration classes and related functionality
- Deprecates direct environment variable access and legacy utility functions
- Updates ESLint rules to enforce configuration usage and prevent deprecated API usage
Reviewed changes
Copilot reviewed 70 out of 71 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/agents-a365-runtime/src/configuration/* |
New configuration infrastructure with interfaces, base classes, and default providers |
packages/agents-a365-runtime/src/power-platform-api-discovery.ts |
Converts ClusterCategory from type alias to enum for better type safety |
packages/agents-a365-runtime/src/environment-utils.ts |
Deprecates utility functions and delegates to configuration classes where possible |
packages/agents-a365-runtime/src/agentic-authorization-service.ts |
Adds overload accepting explicit scopes parameter |
packages/agents-a365-tooling/src/configuration/* |
Tooling configuration extending runtime configuration |
packages/agents-a365-tooling/src/Utility.ts |
Deprecates URL construction methods in favor of service usage |
packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts |
Refactored to use configuration provider |
packages/agents-a365-observability/src/configuration/* |
Observability configuration extending runtime configuration |
packages/agents-a365-observability/src/utils/logging.ts |
Updated to use configuration provider for dynamic log level resolution |
packages/agents-a365-observability/src/tracing/* |
Multiple tracing utilities updated to use configuration |
packages/agents-a365-observability-hosting/src/caching/AgenticTokenCache.ts |
Refactored to accept configuration provider |
packages/agents-a365-*-extensions-*/src/configuration/* |
Extension-specific configurations for OpenAI, LangChain, Claude tooling and observability |
tests/**/*.test.ts |
Comprehensive test coverage for all new configuration classes |
eslint.config.mjs |
New rules to enforce configuration usage and prevent deprecated API usage |
docs/*.md |
Documentation updates reflecting the new configuration pattern |
|
CHANGELOG.md is not updated. |
…ationImprovements # Conflicts: # pnpm-lock.yaml
packages/agents-a365-observability/src/configuration/ObservabilityConfiguration.ts
Show resolved
Hide resolved
…ub.com/microsoft/Agent365-nodejs into users/johanb/ConfigurationImprovements
pontemonti
dismissed stale reviews from nikhilNava, juliomenendez, and tmlsousa
via
e983d40
…ants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| return override.trim().split(/\s+/); | ||
| } | ||
|
|
||
| // Returns production default - use ObservabilityConfiguration for proper env var support |
There was a problem hiding this comment.
This deprecated function now ignores A365_OBSERVABILITY_SCOPES_OVERRIDE and always returns the production default, which conflicts with the file-level note about keeping these utilities for backward compatibility. If backward compatibility is still a goal, consider preserving the prior env-var override behavior (and add an eslint suppression for process.env usage here) or clearly call out the breaking change and ensure versioning reflects it.
| // Returns production default - use ObservabilityConfiguration for proper env var support | |
| // eslint-disable-next-line no-process-env -- Deprecated helper preserved for backward compatibility with env var override | |
| const overrideScope = process.env.A365_OBSERVABILITY_SCOPES_OVERRIDE; | |
| if (overrideScope && overrideScope.trim().length > 0) { | |
| return [overrideScope]; | |
| } | |
| // Returns production default - use ObservabilityConfiguration for full configuration support |
| * // => "https://agent365.svc.cloud.microsoft/agents/servers/MyServer/" | ||
| * | ||
| * @param serverName - The MCP server resource name. | ||
| * @param configProvider - Optional configuration provider. Defaults to defaultToolingConfigurationProvider. | ||
| * @returns The fully-qualified MCP server URL including trailing slash. | ||
| * @deprecated This method is for internal use only. Use McpToolServerConfigurationService instead. |
There was a problem hiding this comment.
The JSDoc says BuildMcpServerUrl returns a URL “including trailing slash” and the example shows a trailing /, but the implementation returns ${baseUrl}/${serverName} with no trailing slash. Please align the documentation (or the return value) to avoid confusion and subtle URL concatenation bugs in callers.
| * | ||
| * Inherited from RuntimeConfigurationOptions: | ||
| * - clusterCategory | ||
| * - isDevelopmentEnvironment |
There was a problem hiding this comment.
The comment claims isDevelopmentEnvironment is inherited from RuntimeConfigurationOptions, but that option type doesn’t include an isDevelopmentEnvironment override (it’s a derived getter on RuntimeConfiguration). Update this doc block to avoid implying consumers can provide an isDevelopmentEnvironment override function.
| * - isDevelopmentEnvironment |
| import { | ||
| RuntimeConfiguration, | ||
| RuntimeConfigurationOptions, | ||
| createRuntimeConfigurationProvider, | ||
| } from '@microsoft/agents-a365-runtime'; |
There was a problem hiding this comment.
The docs reference createRuntimeConfigurationProvider, but that symbol doesn’t exist in the runtime package (no implementation/export in the repo). Update the example to use the actual provider API (DefaultConfigurationProvider, defaultRuntimeConfigurationProvider, or constructing a provider instance directly) so the snippet is copy/pasteable.
| // Multi-tenant: per-request configuration with dynamic overrides | ||
| const options: RuntimeConfigurationOptions = { | ||
| clusterCategory: () => getTenantCluster(currentTenantId), | ||
| mcpPlatformAuthenticationScope: () => getTenantScope(currentTenantId), |
There was a problem hiding this comment.
This example assigns mcpPlatformAuthenticationScope inside RuntimeConfigurationOptions, but runtime options don’t define that property (it belongs to ToolingConfiguration). The snippet should either show ToolingConfigurationOptions/ToolingConfiguration usage for MCP scope, or remove that property from the runtime example.
| mcpPlatformAuthenticationScope: () => getTenantScope(currentTenantId), |
| | `ENABLE_A365_OBSERVABILITY_PER_REQUEST_EXPORT` | Enable per-request export mode | `true`, `false` (default) | | ||
| | `A365_OBSERVABILITY_USE_CUSTOM_DOMAIN` | Use custom domain for export | `true`, `false` (default) | | ||
| | `A365_OBSERVABILITY_DOMAIN_OVERRIDE` | Custom domain URL override | URL string | | ||
| | `A365_OBSERVABILITY_LOG_LEVEL` | Internal logging level | `none` (default), `error`, `warn`, `info`, `debug` | |
There was a problem hiding this comment.
A365_OBSERVABILITY_LOG_LEVEL is documented as supporting debug, but the current logger implementation only recognizes none|info|warn|error (anything else becomes none). Either add debug handling in the logger or remove debug from this table to match behavior.
| | `A365_OBSERVABILITY_LOG_LEVEL` | Internal logging level | `none` (default), `error`, `warn`, `info`, `debug` | | |
| | `A365_OBSERVABILITY_LOG_LEVEL` | Internal logging level | `none` (default), `info`, `warn`, `error` | |
| * import { defaultToolingConfigurationProvider } from '@microsoft/agents-a365-tooling'; | ||
| * const scope = defaultToolingConfigurationProvider.getConfiguration().mcpPlatformAuthenticationScope; | ||
| */ | ||
| export function getMcpPlatformAuthenticationScope(): string { |
There was a problem hiding this comment.
getMcpPlatformAuthenticationScope() is marked deprecated but now always returns the hardcoded production scope and ignores MCP_PLATFORM_AUTHENTICATION_SCOPE. That’s a behavior change for existing callers relying on env-based overrides. Consider restoring env-var support (with an eslint suppression) or updating docs/versioning to reflect that this deprecated API no longer honors overrides.
| export function getMcpPlatformAuthenticationScope(): string { | |
| export function getMcpPlatformAuthenticationScope(): string { | |
| const envScope = process.env.MCP_PLATFORM_AUTHENTICATION_SCOPE; | |
| if (envScope && envScope.trim().length > 0) { | |
| return envScope; | |
| } |
| get mcpPlatformAuthenticationScope(): string { | ||
| const override = this.toolingOverrides.mcpPlatformAuthenticationScope?.(); | ||
| if (override) return override; | ||
|
|
||
| const envValue = process.env.MCP_PLATFORM_AUTHENTICATION_SCOPE; | ||
| if (envValue) return envValue; |
There was a problem hiding this comment.
mcpPlatformAuthenticationScope returns override/env values without trimming and treats whitespace-only strings as valid. Since this value is used as an OAuth scope, leading/trailing whitespace can cause token exchange failures. Consider trimming and treating empty/whitespace-only values as “not set” (similar to how mcpPlatformEndpoint is handled).
| * { clusterCategory: () => 'gov' } | ||
| * | ||
| * // Dynamic per-tenant override using async context | ||
| * { clusterCategory: () => context.active().getValue(TENANT_CONFIG_KEY)?.cluster ?? 'prod' } |
There was a problem hiding this comment.
The JSDoc examples use string literals like 'gov', but clusterCategory is typed as the ClusterCategory enum. Update examples to use ClusterCategory.gov (and similarly for other categories) to match the actual type and avoid misleading consumers.
| * { clusterCategory: () => 'gov' } | |
| * | |
| * // Dynamic per-tenant override using async context | |
| * { clusterCategory: () => context.active().getValue(TENANT_CONFIG_KEY)?.cluster ?? 'prod' } | |
| * { clusterCategory: () => ClusterCategory.gov } | |
| * | |
| * // Dynamic per-tenant override using async context | |
| * { clusterCategory: () => context.active().getValue(TENANT_CONFIG_KEY)?.cluster ?? ClusterCategory.prod } |
| // Create configuration with programmatic overrides | ||
| const config = new RuntimeConfiguration({ | ||
| clusterCategory: () => 'gov', | ||
| isNodeEnvDevelopment: () => false, | ||
| }); | ||
|
|
||
| // Dynamic per-tenant configuration | ||
| const tenantConfigs = { 'tenant-a': 'prod', 'tenant-b': 'gov' }; | ||
| let currentTenant = 'tenant-a'; | ||
|
|
||
| const dynamicConfig = new RuntimeConfiguration({ | ||
| clusterCategory: () => tenantConfigs[currentTenant] as ClusterCategory, |
There was a problem hiding this comment.
The docs example uses clusterCategory: () => 'gov' but clusterCategory is a ClusterCategory enum. Update the snippet to import ClusterCategory and use ClusterCategory.gov (and likewise use enum values in tenantConfigs) so it compiles without casts.
| // Create configuration with programmatic overrides | |
| const config = new RuntimeConfiguration({ | |
| clusterCategory: () => 'gov', | |
| isNodeEnvDevelopment: () => false, | |
| }); | |
| // Dynamic per-tenant configuration | |
| const tenantConfigs = { 'tenant-a': 'prod', 'tenant-b': 'gov' }; | |
| let currentTenant = 'tenant-a'; | |
| const dynamicConfig = new RuntimeConfiguration({ | |
| clusterCategory: () => tenantConfigs[currentTenant] as ClusterCategory, | |
| import { RuntimeConfiguration, ClusterCategory } from '@microsoft/agents-a365-runtime'; | |
| // Create configuration with programmatic overrides | |
| const config = new RuntimeConfiguration({ | |
| clusterCategory: () => ClusterCategory.gov, | |
| isNodeEnvDevelopment: () => false, | |
| }); | |
| // Dynamic per-tenant configuration | |
| const tenantConfigs: Record<string, ClusterCategory> = { | |
| 'tenant-a': ClusterCategory.prod, | |
| 'tenant-b': ClusterCategory.gov, | |
| }; | |
| let currentTenant = 'tenant-a'; | |
| const dynamicConfig = new RuntimeConfiguration({ | |
| clusterCategory: () => tenantConfigs[currentTenant], |
| ?? RuntimeConfiguration.parseEnvInt(process.env.A365_PER_REQUEST_MAX_CONCURRENT_EXPORTS, DEFAULT_MAX_CONCURRENT_EXPORTS); | ||
| return value > 0 ? value : DEFAULT_MAX_CONCURRENT_EXPORTS; | ||
| } | ||
| } |
There was a problem hiding this comment.
These configurations are for PerREquestPanProcessor which is added for a particular scenario asked. We don't expect for normal case that it would be used. Therefore, we don't want to it is exposed in the common ObservabilityConfiguration . Can you create a separated configuration class for PerREquestPanProcessor?
5 participants
This pull request introduces a hierarchical, provider-based configuration pattern throughout the Agents365 SDK, deprecating direct environment variable access and legacy utility functions. It adds new configuration classes and providers for runtime, tooling, and observability settings, and applies these patterns to both documentation and code. The changes also introduce OpenAI-specific observability configuration scaffolding, update linting rules to enforce the new configuration approach, and refactor token caching to use the new provider model.
Configuration System Overhaul
RuntimeConfiguration,ToolingConfiguration, andObservabilityConfiguration, each with default singleton providers (e.g.,defaultRuntimeConfigurationProvider). This enables hierarchical, function-based overrides for multi-tenant support and deprecates direct use of environment utilities. [1] [2] [3] [4]OpenAI Observability Extension
OpenAIObservabilityConfigurationand associated options/types, following the provider pattern for future extensibility and type safety. Exported a singleton provider (defaultOpenAIObservabilityConfigurationProvider) for this configuration. [1] [2] [3] [4]@microsoft/agents-a365-runtimein the OpenAI observability extension package.Agentic Token Cache Refactor
AgenticTokenCacheto accept anIConfigurationProvider<ObservabilityConfiguration>, defaulting todefaultObservabilityConfigurationProvider, enabling per-tenant configuration and improved testability. Exported both the class and a default singleton instance. [1] [2] [3] [4] [5]Linting and Code Quality
process.envaccess and to disallow deprecated API usage, with exceptions for configuration, test, and utility files. [1] [2]Documentation and Environment Variables
These changes collectively modernize the SDK’s configuration approach, improve extensibility and testability, and lay the groundwork for multi-tenant and advanced observability scenarios.