[Observability] Use custom domain for A365 service #114

threddy · 2025-12-01T22:59:06Z

Optionally (using Agent365ExporterOptions.UseCustomDomain) export traces to service hosted at custom domain.

Copilot

Pull request overview

This PR adds support for optionally routing observability traces to a custom Agent365 domain instead of the default Power Platform API (PPAPI) gateway-derived tenant island endpoints. The feature is controlled by a new UseCustomDomain flag in Agent365ExporterOptions, defaulting to false to preserve existing behavior.

Key changes include:

Introduced Agent365EndpointDiscovery class to resolve custom domain endpoints based on cluster category
Extended BuildEndpointPath to support custom domain routing with simplified path structure (/agents/{agentId}/traces)
Added x-ms-tenant-id header when using custom domain (tenant is encoded in URL for PPAPI)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`Agent365EndpointDiscovery.cs`	New class providing endpoint discovery for custom Agent365 domain across different cluster categories (preprod, production)
`Agent365EndpointDiscoveryTest.cs`	Comprehensive test coverage for endpoint discovery including mapping verification, case-insensitivity, and default behavior
`Agent365ExporterOptions.cs`	Added `UseCustomDomain` property with documentation explaining the feature and default behavior
`Agent365ExporterCore.cs`	Updated `BuildEndpointPath` signature to accept `useCustomDomain` parameter; modified export logic to conditionally use custom domain endpoint and add tenant ID header
`Agent365ExporterTests.cs`	Added tests for custom domain routing behavior, endpoint path construction, and tenant header validation; removed `[TestMethod]` attribute from one existing test (appears unintentional)

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterOptions.cs

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterCore.cs

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

juliomenendez

Can we use a setting like we do for UseAgent365Exporter for this? With that in place, should we be able to remove the arguments from the methods we have here and just read the setting?

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

juliomenendez

The scope for which the token is generated also needs to change to api://9b975845-388f-4429-889e-eab1ef63949c/.default

threddy · 2025-12-02T00:42:45Z

UseAgent365Exporter

Since Agent365ExporterCore is static, passing IConfiguration to it will follow the same anti-pattern that we have for the logger. I have a ⁠separate PR to address it, will include IConfiguration. Until then reading this from an environment variable

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

...ts/Microsoft.Agents.A365.Observability.Runtime.Tests/Common/Agent365EndpointDiscoveryTest.cs

src/Observability/Runtime/Common/EnvironmentUtils.cs

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterCore.cs

src/Observability/Runtime/Common/EnvironmentUtils.cs

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

src/Observability/Runtime/Common/EnvironmentUtils.cs

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterCore.cs

src/Observability/Runtime/Common/EnvironmentUtils.cs

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Copilot · 2025-12-03T22:48:29Z

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterCore.cs

+            return EnvironmentUtils.IsCustomDomainEnabled(configuration: this._configuration)
+                ? $"/agents/{agentId}/traces"
+                : useS2SEndpoint
+                    ? $"/maven/agent365/service/agents/{agentId}/traces"
+                    : $"/maven/agent365/agents/{agentId}/traces";


The IsCustomDomainEnabled method is called multiple times within the export flow (in BuildEndpointPath, ExportBatchCoreAsync for endpoint determination, and again for header addition). Consider caching this value at the constructor level to avoid repeated configuration lookups:

private readonly bool _useCustomDomain; public Agent365ExporterCore(...) { // ... _useCustomDomain = EnvironmentUtils.IsCustomDomainEnabled(configuration); }

Copilot · 2025-12-03T22:48:30Z

...ts/Microsoft.Agents.A365.Observability.Runtime.Tests/Common/Agent365EndpointDiscoveryTest.cs

+
+
+[TestClass]
+public class Agent365EndpointDiscoveryTests


The class name should be Agent365EndpointDiscoveryTests (plural) to match the naming convention used elsewhere in the test suite (e.g., Agent365ExporterTests, EnvironmentUtilsTests).

Copilot · 2025-12-03T22:48:30Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

@@ -1,4 +1,4 @@
-using FluentAssertions;
+using FluentAssertions;


Missing required Microsoft copyright header. Add the following at the top of the file:

// ------------------------------------------------------------------------------ // Copyright (c) Microsoft Corporation. All rights reserved. // ------------------------------------------------------------------------------

Copilot · 2025-12-03T22:48:30Z

src/Observability/Runtime/Tracing/Exporters/Agent365ExporterCore.cs

+        public Agent365ExporterCore(ExportFormatter formatter, ILogger<Agent365ExporterCore> logger, IConfiguration? configuration)
        {
            _formatter = formatter ?? throw new ArgumentNullException(nameof(formatter));
            _logger = logger ?? NullLogger<Agent365ExporterCore>.Instance;
+            _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));


The configuration parameter is marked as nullable but throws ArgumentNullException if null. This creates an inconsistent API contract. Consider one of these approaches:

Make it non-nullable (IConfiguration configuration) if it's required

Allow null and use NullConfiguration or handle gracefully (e.g., _configuration = configuration ?? new ConfigurationBuilder().Build())

The inconsistency is especially problematic since line 80 in ObservabilityTracerProviderBuilderExtensions.cs passes potentially null configuration to this constructor.

Suggested change

public Agent365ExporterCore(ExportFormatter formatter, ILogger<Agent365ExporterCore> logger, IConfiguration? configuration)

{

_formatter = formatter ?? throw new ArgumentNullException(nameof(formatter));

_logger = logger ?? NullLogger<Agent365ExporterCore>.Instance;

_configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));

public Agent365ExporterCore(ExportFormatter formatter, ILogger<Agent365ExporterCore> logger, IConfiguration configuration)

{

_formatter = formatter ?? throw new ArgumentNullException(nameof(formatter));

_logger = logger ?? NullLogger<Agent365ExporterCore>.Instance;

_configuration = configuration;

Copilot · 2025-12-03T22:48:30Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

+        _configuration[name] = enabled ? bool.TrueString : bool.FalseString;
+        return new ConfigReset(name, prev);
+    }
+


Missing [TestMethod] attribute. This test method will not be executed by the test runner without it.

Suggested change

[TestMethod]

Copilot · 2025-12-03T22:48:31Z

src/Observability/Runtime/Common/EnvironmentUtils.cs


+        /// <summary>
+        /// Returns true if the custom domain feature is enabled.
+        /// </summary>


The XML documentation should include the parameter description. Add:

/// <param name="configuration">The configuration instance to check for custom domain settings. If null, returns false.</param> /// <returns>True if custom domain is enabled; otherwise, false.</returns>

Suggested change

/// </summary>

/// </summary>

/// <param name="configuration">The configuration instance to check for custom domain settings. If null, returns false.</param>

/// <returns>True if custom domain is enabled; otherwise, false.</returns>

Copilot · 2025-12-03T22:48:31Z

src/Observability/Runtime/Common/EnvironmentUtils.cs

-        {
-            // clusterCategory is ignored; always returns production scope
-            return new[] { ProdObservabilityScope };
+            return EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration) ? new[] { Agent365EndpointProdObservabilityScope } : new[] { ProdObservabilityScope };


This line is very long and hard to read. Consider breaking it into multiple lines for better maintainability:

return EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration) ? new[] { Agent365EndpointProdObservabilityScope } : new[] { ProdObservabilityScope };

Suggested change

return EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration) ? new[] { Agent365EndpointProdObservabilityScope } : new[] { ProdObservabilityScope };

string[] scope;

if (EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration))

{

scope = new[] { Agent365EndpointProdObservabilityScope };

}

else

{

scope = new[] { ProdObservabilityScope };

}

return scope;

Copilot · 2025-12-03T22:48:31Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

+    private static readonly IConfigurationRoot _configuration = new ConfigurationBuilder()
+        .AddInMemoryCollection(new Dictionary<string, string?>())
+        .Build();
+    private static readonly Agent365ExporterCore _agent365ExporterCore = new Agent365ExporterCore(new ExportFormatter(NullLogger<ExportFormatter>.Instance), NullLogger<Agent365ExporterCore>.Instance, _configuration);


Using a static shared configuration across all tests can cause test flakiness when tests run in parallel. Consider using [TestInitialize] and [TestCleanup] methods to create and reset configuration per test, or use the [DoNotParallelize] attribute on the test class if parallel execution is causing issues.

Copilot · 2025-12-03T22:48:32Z

src/Observability/Runtime/Common/EnvironmentUtils.cs

+            if (configuration != null && configuration["EnableAgent365CustomDomain"] != null)
+            {
+                string enabled = configuration["EnableAgent365CustomDomain"]!;
+                return enabled.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase);


The null check configuration["EnableAgent365CustomDomain"] != null is redundant. The indexer returns null when the key doesn't exist, so you can simplify this to:

if (configuration != null) { string? enabled = configuration["EnableAgent365CustomDomain"]; if (enabled != null) { return enabled.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase); } } return false;

Suggested change

if (configuration != null && configuration["EnableAgent365CustomDomain"] != null)

{

string enabled = configuration["EnableAgent365CustomDomain"]!;

return enabled.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase);

if (configuration != null)

{

string? enabled = configuration["EnableAgent365CustomDomain"];

if (enabled != null)

{

return enabled.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase);

}

nikhilNava · 2025-12-03T23:27:01Z

src/Observability/Runtime/Common/EnvironmentUtils.cs

+        /// </summary>
+        public static bool IsCustomDomainEnabled(IConfiguration? configuration)
+        {
+            if (configuration != null && configuration["EnableAgent365CustomDomain"] != null)


Why do we need this configuration to be set by the agent developer?
If we need a fallback we should handle it on the exporter right?
let me know if I'm missing something

We want to continue using the ppapi endpoint for backward compatibility for now. We want to use the custom domain just for our E2E testing for now. Once we are ready to switch all our clients over, we wont need this configuration at all.

Does this answer your question?

Should the code default to the new endpoint? the tenant endpoint should be a fallback right?
If not, agent developers will need to add a env variable and then remove it later.

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

juliomenendez · 2025-12-05T17:53:57Z

src/Observability/Runtime/Common/EnvironmentUtils.cs

+        /// </summary>
+        /// <param name="configuration">The configuration instance.</param>
+        /// <param name="force">When true, re-initializes even if already initialized.</param>
+        public static void Initialize(IConfiguration? configuration, bool force = false)


Instead forcing this to be static and use this Initialize method, do you think it would make sense to convert this class to instantiable? Meaning we make the methods non-static and do this IConfiguration in the constructor?

We can but it would be a breaking change for consumers. We will register and they will have to resolve EnvironmentUtils from their service collection. Or they create an instance of EnvironmentUtils and supply IConfiguration. All this temporarily (because eventually we just want to use the custom domain, no need of the configuration at all, right?)

Whereas today they simply invoke EnvironmentUtils.GetAuthenticationScope()

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

src/Observability/Runtime/Tracing/Exporters/ObservabilityTracerProviderBuilderExtensions.cs:80

This assignment to configuration is useless, since its value is never read.

            var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>();

Copilot · 2025-12-05T19:49:13Z

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs

+    /// </summary>
+    public sealed class Agent365EndpointDiscovery
+    {
+        private readonly string clusterCategory;


The field name clusterCategory should follow C# naming conventions and use PascalCase with an underscore prefix for private fields (e.g., _clusterCategory), consistent with the naming pattern used elsewhere in the codebase.

Copilot · 2025-12-05T19:49:14Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

+        var _configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?>
+            {
+                { "EnableAgent365CustomDomain", enabled ? bool.TrueString : bool.FalseString }
+            })
+            .Build();
+        EnvironmentUtils.Initialize(configuration: _configuration, force: true);


The local variable _configuration uses an underscore prefix which is typically reserved for class-level fields. Consider renaming to configuration to follow standard C# naming conventions for local variables.

Suggested change

var _configuration = new ConfigurationBuilder()

.AddInMemoryCollection(new Dictionary<string, string?>

{

{ "EnableAgent365CustomDomain", enabled ? bool.TrueString : bool.FalseString }

})

.Build();

EnvironmentUtils.Initialize(configuration: _configuration, force: true);

var configuration = new ConfigurationBuilder()

.AddInMemoryCollection(new Dictionary<string, string?>

{

{ "EnableAgent365CustomDomain", enabled ? bool.TrueString : bool.FalseString }

})

.Build();

EnvironmentUtils.Initialize(configuration: configuration, force: true);

Copilot · 2025-12-05T19:49:14Z

src/Observability/Runtime/Tracing/Exporters/ObservabilityTracerProviderBuilderExtensions.cs

+            var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>();
+


The configuration variable is retrieved from the service provider but never used. If this was intended to initialize EnvironmentUtils, it should be done here. Otherwise, this variable should be removed to avoid confusion.

Suggested change

var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>();

Copilot · 2025-12-05T19:49:14Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

+        Agent365ExporterTests.SetCustomDomain(true);
+        var path = Agent365ExporterTests._agent365ExporterCore.BuildEndpointPath("agent-123", useS2SEndpoint: true);
+        path.Should().Be("/agents/agent-123/traces");


The test modifies global static state via SetCustomDomain(), which can cause test isolation issues if tests run in parallel or if subsequent tests don't reset the state. Consider:

Using a [TestCleanup] method to reset state after each test

Adding test execution order attributes or making tests serial

Refactoring EnvironmentUtils to allow dependency injection for better testability

Copilot · 2025-12-05T19:49:15Z

src/Observability/Runtime/Builder.cs

                tracerProviderBuilder.Build();
            }

+            EnvironmentUtils.Initialize(configuration: this.Configuration);


Potential null reference exception: this.Configuration can be null (as indicated by the nullable type on line 30), but EnvironmentUtils.Initialize() throws ArgumentNullException if configuration is null (line 68-71 in EnvironmentUtils.cs).

Either:

Add a null check before calling Initialize, or

Pass a default empty configuration if Configuration is null, or

Make Configuration non-nullable if it's always required

Suggested change

EnvironmentUtils.Initialize(configuration: this.Configuration);

EnvironmentUtils.Initialize(configuration: this.Configuration ?? new ConfigurationBuilder().Build());

Copilot · 2025-12-05T19:49:15Z

...Microsoft.Agents.A365.Observability.Runtime.Tests/Tracing/Exporters/Agent365ExporterTests.cs

+            .Build();
+        EnvironmentUtils.Initialize(configuration: _configuration, force: true);
+    }
+


This test method is missing the [TestMethod] attribute, which means it will not be executed by the test runner. Add the attribute before the method declaration.

Suggested change

[TestMethod]

threddy added 3 commits December 1, 2025 14:26

Conditionally use custom domain endpoint.

a4ba9cd

Unit tests.

177f36e

Agent365Exporter core request uri and headers tests

c52e780

threddy requested a review from a team as a code owner December 1, 2025 22:59

Copilot AI review requested due to automatic review settings December 1, 2025 22:59

Copilot started reviewing on behalf of threddy December 1, 2025 22:59 View session

Copilot finished reviewing on behalf of threddy December 1, 2025 23:02

Copilot AI reviewed Dec 1, 2025

View reviewed changes

juliomenendez requested changes Dec 1, 2025

View reviewed changes

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs Outdated Show resolved Hide resolved

src/Observability/Runtime/Common/Agent365EndpointDiscovery.cs Show resolved Hide resolved

Rename to GetHost(). Remove preprod.

4b24e73

juliomenendez requested changes Dec 1, 2025

View reviewed changes

Use env variable

550c88a

Return new token scope.

c0797fa

Copilot AI review requested due to automatic review settings December 2, 2025 00:58

Copilot started reviewing on behalf of threddy December 2, 2025 00:59 View session

Copilot finished reviewing on behalf of threddy December 2, 2025 01:03

Copilot AI reviewed Dec 2, 2025

View reviewed changes

fpfp100 previously approved these changes Dec 2, 2025

View reviewed changes

threddy added 3 commits December 3, 2025 11:31

merge

8147a09

Fix tests after merge

4fd2532

Use IConfiguration.

c003625

threddy dismissed fpfp100’s stale review via c003625 December 3, 2025 22:40

Remove newline

0b9a400

Copilot AI review requested due to automatic review settings December 3, 2025 22:42

Copilot started reviewing on behalf of threddy December 3, 2025 22:43 View session

Copilot finished reviewing on behalf of threddy December 3, 2025 22:46

Copilot AI reviewed Dec 3, 2025

View reviewed changes

nikhilNava requested changes Dec 3, 2025

View reviewed changes

Merge branch 'main' into users/threddy/custom-domain

9452183

Initialize configuration in EnvironmentUtils.

619af64

juliomenendez requested changes Dec 5, 2025

View reviewed changes

Default to production.

b0cd9e4

Copilot AI review requested due to automatic review settings December 5, 2025 19:43

Copilot started reviewing on behalf of threddy December 5, 2025 19:43 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

		@@ -1,4 +1,4 @@
		using FluentAssertions;
		using FluentAssertions;

-            return EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration) ? new[] { Agent365EndpointProdObservabilityScope } : new[] { ProdObservabilityScope };
+            string[] scope;
+            if (EnvironmentUtils.IsCustomDomainEnabled(configuration: configuration))
+            {
+                scope = new[] { Agent365EndpointProdObservabilityScope };
+            }
+            else
+            {
+                scope = new[] { ProdObservabilityScope };
+            }
+            return scope;

		var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>();

	EnvironmentUtils.Initialize(configuration: this.Configuration);
	EnvironmentUtils.Initialize(configuration: this.Configuration ?? new ConfigurationBuilder().Build());

[Observability] Use custom domain for A365 service #114

Are you sure you want to change the base?

[Observability] Use custom domain for A365 service #114

Uh oh!

Conversation

threddy commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juliomenendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

juliomenendez left a comment

Choose a reason for hiding this comment

Uh oh!

threddy commented Dec 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

nikhilNava Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

threddy Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

nikhilNava Dec 5, 2025

Choose a reason for hiding this comment