Skip to content

Enable OTLP exporting as part of development build #2006

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.

Sign up for GitHub

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

Open
alzimmermsft wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Enable OTLP exporting as part of development build #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab30f01
Enable OTLP exporting as part of development build
alzimmermsft Mar 10, 2026
6864da7
Fix typo
alzimmermsft Mar 10, 2026
6f95f09
Update OpenTelemetryExtension to use IConfiguration to load setup
alzimmermsft Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions ...re.Mcp.Core.UnitTests/Areas/Server/Commands/ServiceCollectionExtensionsSerializedTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void InitializeConfigurationAndOptions_Defaults()
var services = SetupBaseServices();

// Act
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);

// Assert
var provider = services.BuildServiceProvider();
Expand Down Expand Up @@ -64,7 +64,7 @@ public void InitializeConfigurationAndOptions_HttpTransport()
var services = SetupBaseServices().AddSingleton(Options.Create(serviceStartOptions));

// Act
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);
var provider = services.BuildServiceProvider();

// Assert
Expand All @@ -88,7 +88,7 @@ public void InitializeConfigurationAndOptions_Stdio()

// Act
Environment.SetEnvironmentVariable("AZURE_MCP_COLLECT_TELEMETRY", "false");
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);
var provider = services.BuildServiceProvider();

// Assert
Expand Down Expand Up @@ -121,7 +121,7 @@ public void InitializeConfigurationAndOptions_WithSupportLoggingFolder_DisablesT

// Act
Environment.SetEnvironmentVariable("AZURE_MCP_COLLECT_TELEMETRY", null);
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);
var provider = services.BuildServiceProvider();

// Assert
Expand All @@ -145,7 +145,7 @@ public void InitializeConfigurationAndOptions_WithSupportLoggingFolderAndEnvVarT

// Act
Environment.SetEnvironmentVariable("AZURE_MCP_COLLECT_TELEMETRY", "true");
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);
var provider = services.BuildServiceProvider();

// Assert
Expand All @@ -171,7 +171,7 @@ public void InitializeConfigurationAndOptions_WithEmptyOrWhitespaceSupportLoggin

// Act
Environment.SetEnvironmentVariable("AZURE_MCP_COLLECT_TELEMETRY", null);
ServiceCollectionExtensions.InitializeConfigurationAndOptions(services);
ServiceCollectionExtensions.InitializeConfigurationOptionsAndOpenTelemetry(services);
var provider = services.BuildServiceProvider();

// Assert
Expand Down
4 changes: 3 additions & 1 deletion .../Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Extensions/OpenTelemetryExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Licensed under the MIT License.

using Azure.Mcp.Core.Extensions;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Mcp.Core.Services.Telemetry;
using NSubstitute;
using Xunit;

namespace Azure.Mcp.Core.UnitTests.Extensions;
Expand All @@ -24,7 +26,7 @@ public void ConfigureOpenTelemetry_RegistersTelemetryService()
var services = new ServiceCollection();

// Act
services.ConfigureOpenTelemetry();
services.ConfigureOpenTelemetry(Substitute.For<IConfiguration>());
Copy link
Copy Markdown
Contributor

@jongio jongio Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Tests: Mock config doesn't verify new behavior

Substitute.For<IConfiguration>() returns null for all reads, so this test only proves the method doesn't throw with empty config. No test verifies that setting AZURE_MCP_ENABLE_OTLP_EXPORTER to true actually registers OTLP exporters, or that APPLICATIONINSIGHTS_CONNECTION_STRING from config enables the user-provided exporter.


// Assert - Verify that the telemetry service descriptor is registered
Assert.Contains(services, sd => sd.ServiceType == typeof(ITelemetryService));
Expand Down
15 changes: 9 additions & 6 deletions core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,13 @@ public static IServiceCollection AddAzureMcpServer(this IServiceCollection servi
/// Using <see cref="IConfiguration"/> configures <see cref="McpServerConfiguration"/>.
/// </summary>
/// <param name="services">Service Collection to add configuration logic to.</param>
public static void InitializeConfigurationAndOptions(this IServiceCollection services)
public static void InitializeConfigurationOptionsAndOpenTelemetry(this IServiceCollection services)
Copy link
Copy Markdown
Member

@conniey conniey Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we have to configure open telemetry in this method as well? This method now has multiple responsibilities rather than just setting options.

Copy link
Copy Markdown
Contributor Author

@alzimmermsft alzimmermsft Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a cleaner way to get the IConfiguration singleton bound here when we called OpenTelemetryExtensions.ConfigureOpenTelemetry. If you know of a way to cleanly pull that out of the IServiceCollection I'd be more than happy to revert this change as I'm not a huge fan of it either.

Copy link
Copy Markdown
Contributor

@jongio jongio Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Breaking change: Public API renamed without backward-compat shim

InitializeConfigurationAndOptions is a public extension method. Renaming it to InitializeConfigurationOptionsAndOpenTelemetry (plus changing ConfigureOpenTelemetry's signature to require IConfiguration) is a source and binary break for any external consumer of Microsoft.Mcp.Core. Consider adding [Obsolete] forwarders that delegate to the new methods for one release, or confirm no external callers exist.

{
#if DEBUG
Copy link
Copy Markdown
Member

@conniey conniey Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for having DOTNET_ENVIRONMENT is so we don't have to recompile to make test changes. We can rerun the application with different settings. We shouldn't have to if/def things based on the build type.

Copy link
Copy Markdown
Member

@conniey conniey Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://learn.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-10.0

Copy link
Copy Markdown
Contributor Author

@alzimmermsft alzimmermsft Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was finding the DOTNET_ENVIRONMENT was blank when running debug builds, so we fell back to Production. If you know of a better way to fallback to Development when running a debug build do let me know.

var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Development";
#else
var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Production";
#endif
var configuration = new ConfigurationBuilder()
.AddJsonFile("appsettings.json", optional: false)
.AddJsonFile($"appsettings.{environment}.json", optional: true)
Expand All @@ -272,11 +276,8 @@ public static void InitializeConfigurationAndOptions(this IServiceCollection ser

// Assembly.GetEntryAssembly is used to retrieve the version of the server application as that is
// the assembly that will run the tool calls.
var entryAssembly = Assembly.GetEntryAssembly();
if (entryAssembly == null)
{
throw new InvalidOperationException("Entry assembly must be a managed assembly.");
}
var entryAssembly = Assembly.GetEntryAssembly()
?? throw new InvalidOperationException("Entry assembly must be a managed assembly.");

options.Version = AssemblyHelper.GetAssemblyVersion(entryAssembly);

Expand All @@ -293,5 +294,7 @@ public static void InitializeConfigurationAndOptions(this IServiceCollection ser
// over any other settings.
options.IsTelemetryEnabled = rootConfiguration.GetValue("AZURE_MCP_COLLECT_TELEMETRY", true);
});

services.ConfigureOpenTelemetry(configuration);
}
}
13 changes: 7 additions & 6 deletions core/Microsoft.Mcp.Core/src/Extensions/OpenTelemetryExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Runtime.InteropServices;
using Azure.Monitor.OpenTelemetry.Exporter;
using Microsoft.Extensions.Azure;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand All @@ -25,7 +26,7 @@ public static class OpenTelemetryExtensions
/// </summary>
private const string MicrosoftOwnedAppInsightsConnectionString = "InstrumentationKey=21e003c0-efee-4d3f-8a98-1868515aa2c9;IngestionEndpoint=https://centralus-2.in.applicationinsights.azure.com/;LiveEndpoint=https://centralus.livediagnostics.monitor.azure.com/;ApplicationId=f14f6a2d-6405-4f88-bd58-056f25fe274f";

public static void ConfigureOpenTelemetry(this IServiceCollection services)
public static void ConfigureOpenTelemetry(this IServiceCollection services, IConfiguration configuration)
Copy link
Copy Markdown
Contributor

@jongio jongio Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] ConfigureOpenTelemetry signature is also a breaking change

Adding a required IConfiguration parameter changes this public extension method's signature. External consumers calling services.ConfigureOpenTelemetry() won't compile. Same concern as the InitializeConfigurationAndOptions rename - needs either an [Obsolete] parameterless overload or a default parameter value.

{
services.AddSingleton<ITelemetryService, TelemetryService>();

Expand All @@ -46,10 +47,10 @@ public static void ConfigureOpenTelemetry(this IServiceCollection services)
services.AddSingleton<IMachineInformationProvider, DefaultMachineInformationProvider>();
}

EnableAzureMonitor(services);
EnableAzureMonitor(services, configuration);
}

private static void EnableAzureMonitor(this IServiceCollection services)
private static void EnableAzureMonitor(this IServiceCollection services, IConfiguration configuration)
{
#if DEBUG
services.AddSingleton(sp =>
Expand Down Expand Up @@ -80,7 +81,7 @@ private static void EnableAzureMonitor(this IServiceCollection services)
.AddTelemetrySdk();
});

var userProvidedAppInsightsConnectionString = Environment.GetEnvironmentVariable("APPLICATIONINSIGHTS_CONNECTION_STRING");
var userProvidedAppInsightsConnectionString = configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"];
Copy link
Copy Markdown
Contributor

@jongio jongio Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Config: Connection string now readable from checked-in JSON

Switching from Environment.GetEnvironmentVariable to configuration[...] means APPLICATIONINSIGHTS_CONNECTION_STRING can now come from appsettings.json or appsettings.Development.json - not just env vars. If someone adds a connection string to a checked-in config file, it becomes a committed secret. The env-var-only approach was a natural guard against this. Consider keeping this particular value as env-var-only, or document that connection strings should only be set via env vars / user-secrets.


if (!string.IsNullOrWhiteSpace(userProvidedAppInsightsConnectionString))
{
Expand All @@ -92,7 +93,7 @@ private static void EnableAzureMonitor(this IServiceCollection services)
#if RELEASE
Copy link
Copy Markdown
Member

@conniey conniey Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, I think we could have avoided this by using default appsettings.json = false and used the running environment to set it or not. Because now if we want to test this in a debug environment, it's impossible.

// This environment variable can be used to disable Microsoft telemetry collection.
// By default, Microsoft telemetry is enabled.
var microsoftTelemetry = Environment.GetEnvironmentVariable("AZURE_MCP_COLLECT_TELEMETRY_MICROSOFT");
var microsoftTelemetry = configuration["AZURE_MCP_COLLECT_TELEMETRY_MICROSOFT"];

bool shouldCollectMicrosoftTelemetry = string.IsNullOrWhiteSpace(microsoftTelemetry) || (bool.TryParse(microsoftTelemetry, out var shouldCollect) && shouldCollect);

Expand All @@ -102,7 +103,7 @@ private static void EnableAzureMonitor(this IServiceCollection services)
}
#endif

var enableOtlp = Environment.GetEnvironmentVariable("AZURE_MCP_ENABLE_OTLP_EXPORTER");
var enableOtlp = configuration["AZURE_MCP_ENABLE_OTLP_EXPORTER"];
if (!string.IsNullOrEmpty(enableOtlp) && bool.TryParse(enableOtlp, out var shouldEnable) && shouldEnable)
{
otelBuilder.WithTracing(tracing => tracing.AddOtlpExporter())
Expand Down
3 changes: 1 addition & 2 deletions servers/Azure.Mcp.Server/src/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ internal static void ConfigureServices(IServiceCollection services)
{
var thisAssembly = typeof(Program).Assembly;

services.InitializeConfigurationAndOptions();
services.ConfigureOpenTelemetry();
services.InitializeConfigurationOptionsAndOpenTelemetry();

services.AddMemoryCache();
services.AddSingleton<IExternalProcessService, ExternalProcessService>();
Expand Down
8 changes: 4 additions & 4 deletions servers/Azure.Mcp.Server/src/appsettings.Development.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
// Generally, AZURE_MCP_COLLECT_TELEMETRY will be set as an environment variable.
// For Development purposes, we never want to publish telemetry so it is explicitly
// set here.
"AZURE_MCP_COLLECT_TELEMETRY": "false",
// Disable AZURE_MCP_COLLECT_TELEMETRY_MICROSOFT to prevent development telemetry from being collected by Microsoft
// instrumentation. This is to avoid polluting production telemetry with development data.
"AZURE_MCP_COLLECT_TELEMETRY_MICROSOFT": "false",
Copy link
Copy Markdown
Contributor

@jongio jongio Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Inconsistency: Fabric and Template dev configs not updated

Both servers/Fabric.Mcp.Server/src/appsettings.Development.json and servers/Template.Mcp.Server/src/appsettings.Development.json still have the old AZURE_MCP_COLLECT_TELEMETRY: false and lack the new AZURE_MCP_COLLECT_TELEMETRY_MICROSOFT and AZURE_MCP_ENABLE_OTLP_EXPORTER keys. Their Program.cs files were updated to call the renamed method, but their dev configs are stale. Devs running Fabric or Template servers in Development will get different telemetry behavior than Azure.Mcp.Server.

"AZURE_MCP_ENABLE_OTLP_EXPORTER": "true",
"Logging": {
"LogLevel": {
"Default": "Debug",
Expand Down
3 changes: 1 addition & 2 deletions servers/Fabric.Mcp.Server/src/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ private static void WriteResponse(CommandResponse response)
/// <param name="services">A service collection.</param>
internal static void ConfigureServices(IServiceCollection services)
{
services.InitializeConfigurationAndOptions();
services.ConfigureOpenTelemetry();
services.InitializeConfigurationOptionsAndOpenTelemetry();

services.AddMemoryCache();
services.AddSingleton<IExternalProcessService, ExternalProcessService>();
Expand Down
3 changes: 1 addition & 2 deletions servers/Template.Mcp.Server/src/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ private static void WriteResponse(CommandResponse response)
/// <param name="services">A service collection.</param>
internal static void ConfigureServices(IServiceCollection services)
{
services.InitializeConfigurationAndOptions();
services.ConfigureOpenTelemetry();
services.InitializeConfigurationOptionsAndOpenTelemetry();

services.AddMemoryCache();
services.AddSingleton<IExternalProcessService, ExternalProcessService>();
Expand Down