-
Notifications
You must be signed in to change notification settings - Fork 8
[Observability] Use custom domain for A365 service #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a4ba9cd
177f36e
c52e780
4b24e73
550c88a
c0797fa
8147a09
4fd2532
c003625
0b9a400
9452183
619af64
b0cd9e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // ------------------------------------------------------------------------------ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // ------------------------------------------------------------------------------ | ||
threddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| using System; | ||
|
|
||
| namespace Microsoft.Agents.A365.Observability.Runtime.Common | ||
| { | ||
| /// <summary> | ||
| /// Provides discovery for Agent365 endpoints. | ||
| /// </summary> | ||
| public sealed class Agent365EndpointDiscovery | ||
| { | ||
| private readonly string clusterCategory; | ||
|
||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="Agent365EndpointDiscovery"/> class. | ||
| /// </summary> | ||
| /// <param name="clusterCategory">The cluster category.</param> | ||
| public Agent365EndpointDiscovery(string clusterCategory) | ||
| { | ||
| this.clusterCategory = clusterCategory ?? "production"; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the base host for the specified cluster category. | ||
| /// </summary> | ||
| public string GetHost() | ||
| { | ||
| switch (this.clusterCategory?.ToLowerInvariant()) | ||
threddy marked this conversation as resolved.
Show resolved
Hide resolved
threddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| case "firstrelease": | ||
threddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case "production": | ||
| case "prod": | ||
| return "agent365.svc.cloud.microsoft"; | ||
| default: | ||
| throw new ArgumentException($"Invalid ClusterCategory value: {clusterCategory}"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||
| // ------------------------------------------------------------------------------ | ||||||||||
|
|
||||||||||
| using Microsoft.Extensions.Configuration; | ||||||||||
| using System; | ||||||||||
|
|
||||||||||
| namespace Microsoft.Agents.A365.Observability.Runtime.Common | ||||||||||
|
|
@@ -14,26 +15,17 @@ public class EnvironmentUtils | |||||||||
| private const string ProdObservabilityScope = "https://api.powerplatform.com/.default"; | ||||||||||
| private const string ProdObservabilityClusterCategory = "prod"; | ||||||||||
| private const string DevelopmentEnvironmentName = "development"; | ||||||||||
| private const string Agent365EndpointProdObservabilityScope = "api://9b975845-388f-4429-889e-eab1ef63949c/.default"; | ||||||||||
| private static bool _initialized; | ||||||||||
| private static bool _customDomainEnabled; | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Returns the scope for authenticating to the observability service based on the current environment. | ||||||||||
| /// </summary> | ||||||||||
| /// <returns>The authentication scope.</returns> | ||||||||||
| public static string[] GetObservabilityAuthenticationScope() | ||||||||||
| { | ||||||||||
| return new[] { ProdObservabilityScope }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// [Deprecated] Returns the scope for authenticating to the observability service based on the cluster category. | ||||||||||
| /// </summary> | ||||||||||
| /// <param name="clusterCategory">Cluster category (deprecated, defaults to production).</param> | ||||||||||
| /// <returns>The authentication scope.</returns> | ||||||||||
| [Obsolete("Cluster category argument is deprecated and will be removed in future versions. Defaults to production.")] | ||||||||||
| public static string[] GetObservabilityAuthenticationScope(string clusterCategory = ProdObservabilityClusterCategory) | ||||||||||
| { | ||||||||||
| // clusterCategory is ignored; always returns production scope | ||||||||||
| return new[] { ProdObservabilityScope }; | ||||||||||
| return IsCustomDomainEnabled() ? new[] { Agent365EndpointProdObservabilityScope } : new[] { ProdObservabilityScope }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
|
|
@@ -66,6 +58,40 @@ public static bool IsDevelopmentEnvironment() | |||||||||
| return string.Equals(environment, DevelopmentEnvironmentName, StringComparison.OrdinalIgnoreCase); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Initializes the cached configuration values for environment utilities. Should be called once at application startup. | ||||||||||
| /// </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) | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead forcing this to be static and use this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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() |
||||||||||
| { | ||||||||||
| if (configuration == null) | ||||||||||
| { | ||||||||||
| throw new ArgumentNullException(nameof(configuration)); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (_initialized && !force) | ||||||||||
| { | ||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| string enabled = configuration["EnableAgent365CustomDomain"] ?? string.Empty; | ||||||||||
| _customDomainEnabled = enabled.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase); | ||||||||||
| _initialized = true; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Returns true if the custom domain feature is enabled. | ||||||||||
| /// </summary> | ||||||||||
threddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| /// </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> |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -77,9 +77,11 @@ private static TracerProviderBuilder ConfigureInternal(IServiceProvider serviceP | |||
| var coreLogger = serviceProvider.GetService<ILogger<Agent365ExporterCore>>() ?? loggerFactory.CreateLogger<Agent365ExporterCore>(); | ||||
| var formatterLogger = serviceProvider.GetService<ILogger<ExportFormatter>>() ?? loggerFactory.CreateLogger<ExportFormatter>(); | ||||
|
|
||||
| var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>(); | ||||
|
|
||||
|
Comment on lines
+80
to
+81
|
||||
| var configuration = serviceProvider.GetService<Microsoft.Extensions.Configuration.IConfiguration>(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| // ------------------------------------------------------------------------------ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // ------------------------------------------------------------------------------ | ||
threddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| using Microsoft.Agents.A365.Observability.Runtime.Common; | ||
|
|
||
| namespace Microsoft.Agents.A365.Observability.Runtime.Tests.Common; | ||
|
|
||
|
|
||
| [TestClass] | ||
| public class Agent365EndpointDiscoveryTests | ||
|
||
| { | ||
| [TestMethod] | ||
| public void GetHost_Mapping_IsCorrect() | ||
| { | ||
| var expected = new Dictionary<string, string> | ||
| { | ||
| ["firstrelease"] = "agent365.svc.cloud.microsoft", | ||
| ["production"] = "agent365.svc.cloud.microsoft", | ||
| ["prod"] = "agent365.svc.cloud.microsoft", | ||
| }; | ||
|
|
||
| foreach (var kv in expected) | ||
| { | ||
| var disc = new Agent365EndpointDiscovery(kv.Key); | ||
| Assert.AreEqual(kv.Value, disc.GetHost()); | ||
| } | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void GetHost_IsCaseInsensitive() | ||
| { | ||
| var disc1 = new Agent365EndpointDiscovery("PRODUCTION"); | ||
| Assert.AreEqual("agent365.svc.cloud.microsoft", disc1.GetHost()); | ||
|
|
||
| var disc2 = new Agent365EndpointDiscovery("PROD"); | ||
| Assert.AreEqual("agent365.svc.cloud.microsoft", disc2.GetHost()); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void GetHost_ThrowsForUnknown() | ||
| { | ||
| var disc = new Agent365EndpointDiscovery("unknown-category"); | ||
| Assert.ThrowsException<System.ArgumentException>(() => disc.GetHost()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // ------------------------------------------------------------------------------ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // ------------------------------------------------------------------------------ | ||
|
|
||
| using FluentAssertions; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Agents.A365.Observability.Runtime.Common; | ||
|
|
||
| namespace Microsoft.Agents.A365.Observability.Runtime.Tests.Common | ||
| { | ||
| [TestClass] | ||
| public class EnvironmentUtilsTests | ||
| { | ||
| private static IConfiguration BuildConfig(IDictionary<string, string?>? values = null) | ||
| { | ||
| var builder = new ConfigurationBuilder(); | ||
| if (values != null) | ||
| { | ||
| builder.AddInMemoryCollection(values); | ||
| } | ||
| return builder.Build(); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void GetObservabilityAuthenticationScope_UsesAgent365Scope_WhenCustomDomainEnabled() | ||
| { | ||
| // Arrange | ||
| var configEnabled = BuildConfig(new Dictionary<string, string?> | ||
| { | ||
| { "EnableAgent365CustomDomain", "true" } | ||
| }); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: configEnabled, force: true); | ||
|
|
||
| // Act | ||
| var scope = EnvironmentUtils.GetObservabilityAuthenticationScope(); | ||
|
|
||
| // Assert | ||
| scope.Should().ContainSingle().Which.Should().Be("api://9b975845-388f-4429-889e-eab1ef63949c/.default"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void GetObservabilityAuthenticationScope_UsesProdScope_WhenCustomDomainDisabled() | ||
| { | ||
| // Arrange | ||
| var configDisabled = BuildConfig(new Dictionary<string, string?> | ||
| { | ||
| { "EnableAgent365CustomDomain", "false" } | ||
| }); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: configDisabled, force: true); | ||
|
|
||
| // Act | ||
| var scope = EnvironmentUtils.GetObservabilityAuthenticationScope(); | ||
|
|
||
| // Assert | ||
| scope.Should().ContainSingle().Which.Should().Be("https://api.powerplatform.com/.default"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void GetObservabilityAuthenticationScope_UsesProdScope_WhenCustomDomainSettingMissing() | ||
| { | ||
| // Arrange | ||
| var configMissing = BuildConfig(); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: configMissing, force: true); | ||
|
|
||
| // Act | ||
| var scope = EnvironmentUtils.GetObservabilityAuthenticationScope(); | ||
|
|
||
| // Assert | ||
| scope.Should().ContainSingle().Which.Should().Be("https://api.powerplatform.com/.default"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void IsCustomDomainEnabled_ReturnsTrue_WhenConfigValueTrue() | ||
| { | ||
| // Arrange | ||
| var config = BuildConfig(new Dictionary<string, string?> | ||
| { | ||
| { "EnableAgent365CustomDomain", "true" } | ||
| }); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: config, force: true); | ||
|
|
||
| // Act | ||
| var result = EnvironmentUtils.IsCustomDomainEnabled(); | ||
|
|
||
| // Assert | ||
| result.Should().BeTrue(); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void IsCustomDomainEnabled_ReturnsFalse_WhenConfigValueFalse() | ||
| { | ||
| // Arrange | ||
| var config = BuildConfig(new Dictionary<string, string?> | ||
| { | ||
| { "EnableAgent365CustomDomain", "false" } | ||
| }); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: config, force: true); | ||
|
|
||
| // Act | ||
| var result = EnvironmentUtils.IsCustomDomainEnabled(); | ||
|
|
||
| // Assert | ||
| result.Should().BeFalse(); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void IsCustomDomainEnabled_ReturnsFalse_WhenConfigValueMissing() | ||
| { | ||
| // Arrange | ||
| var config = BuildConfig(); | ||
|
|
||
| EnvironmentUtils.Initialize(configuration: config, force: true); | ||
|
|
||
| // Act | ||
| var result = EnvironmentUtils.IsCustomDomainEnabled(); | ||
|
|
||
| // Assert | ||
| result.Should().BeFalse(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference exception:
this.Configurationcan be null (as indicated by the nullable type on line 30), butEnvironmentUtils.Initialize()throwsArgumentNullExceptionif configuration is null (line 68-71 in EnvironmentUtils.cs).Either: