Skip to content
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

Durable Tracing V2 issue when using Application Insights with Entra Authentication #2870

Open
Tracked by #2660
MahrRah opened this issue Jul 5, 2024 · 3 comments · May be fixed by #3009
Open
Tracked by #2660

Durable Tracing V2 issue when using Application Insights with Entra Authentication #2870

MahrRah opened this issue Jul 5, 2024 · 3 comments · May be fixed by #3009
Labels

Comments

@MahrRah
Copy link

MahrRah commented Jul 5, 2024

Description

We have encountered a problem regarding the combination of Durable Tracing V2 and using ManagedIdentity with Application Insights.

We found that when disabling local authentication in our Application Insights (see Microsoft Entra authentication for Application Insights) and relying on the ManagedIdentity of the Azure Function Host to authenticate to Application Insights, we start losing telemetry data related to the V2 distributed tracing.

Looking into the code, it seems like the Durable Function Extension configures its own TelemetryClient which does not take the ManagedIdentity of the Function Host into account.

Expected behavior

When using Managed Identity there should be a way to set the Managed Identity credentials of the Azure Function Hosts when this TelemetryClient is created, similar to how this is handled in the azure-webjobs-sdk with the APPLICATIONINSIGHTS_AUTHENTICATION_STRING (see code here)

Actual behavior

This TelemetryClient only sets the APPINSIGHTS_INSTRUMENTATIONKEY and the APPLICATIONINSIGHTS_CONNECTION_STRING, but does not have a way to set credentials when creating the TelemetryConfiguration (see full code reference here or simplified snippet below)

Relevant source code snippets

private TelemetryConfiguration SetupTelemetryConfiguration()
        {
            TelemetryConfiguration config = TelemetryConfiguration.CreateDefault();
            if (this.OnSend != null)
            {
                config.TelemetryChannel = new NoOpTelemetryChannel { OnSend = this.OnSend };
            }

            string resolvedInstrumentationKey = this.nameResolver.Resolve("APPINSIGHTS_INSTRUMENTATIONKEY");
            string resolvedConnectionString = this.nameResolver.Resolve("APPLICATIONINSIGHTS_CONNECTION_STRING");

            bool instrumentationKeyProvided = !string.IsNullOrEmpty(resolvedInstrumentationKey);
            bool connectionStringProvided = !string.IsNullOrEmpty(resolvedConnectionString);

            if (instrumentationKeyProvided && connectionStringProvided)
            {
                this.endToEndTraceHelper.ExtensionWarningEvent(...);
            }

            if (!instrumentationKeyProvided && !connectionStringProvided)
            {
                this.endToEndTraceHelper.ExtensionWarningEvent(...);
            }

            if (instrumentationKeyProvided)
            {
                this.endToEndTraceHelper.ExtensionInformationalEvent(...);

#pragma warning disable CS0618 // Type or member is obsolete
                config.InstrumentationKey = resolvedInstrumentationKey;
#pragma warning restore CS0618 // Type or member is obsolete
            }

            if (connectionStringProvided)
            {
                this.endToEndTraceHelper.ExtensionInformationalEvent(...);

                config.ConnectionString = resolvedConnectionString;
            }

            return config;
        }

App Details

  • Durable Functions extension version (e.g. v1.8.3): >=2.4.1
  • Azure Functions runtime version (1.0 or 2.0): 2.0
  • Programming language used: Python

tagging: @ransonjb

@cgillum
Copy link
Member

cgillum commented Jul 5, 2024

FYI @AnatoliB and @lilyjma, this might be an important issue to prioritize as it is security compliance related, and we may start seeing more asks for this.

@bachuv
Copy link
Collaborator

bachuv commented Jul 30, 2024

I discussed this with @jviau offline and he recommended making the following fix:

  • Rewrite ITelemetryActivator as an ITelemetryModule so we get TelemetryConfiguration without having to instantiate it ourselves. With this approach, TelemetryConfiguration will have the necessary auth information.

Jacob, feel free to add any details that I missed.

@jviau
Copy link
Contributor

jviau commented Jul 30, 2024

@bachuv, yes it is a very straight forward change of removing our own ITelemetryActivator contract and use ITelemetryModule instead. Register our implementation as a singleton:

Implementation of our module (using existing TelemetryActivator implementation):

/// <inheritdoc/>
public void Initialize(TelemetryConfiguration configuration)
{
    if (this.options.Tracing.Version == Options.DurableDistributedTracingVersion.V2)
    {
        DurableTelemetryModule module = new DurableTelemetryModule();
        module.Initialize(configuration);
        this.telemetryModule = module;
    }
    else
    {
        this.SetUpV1DistributedTracing();
        if (CorrelationSettings.Current.EnableDistributedTracing)
        {
            this.SetUpTelemetryClient(configuration);
            if (CorrelationSettings.Current.EnableDistributedTracing)
            {
                this.SetUpTelemetryCallbacks();
            }
        }
    }
}

Registration:

services.AddSingleton<ITelemetryModule, TelemetryActivator>();

@bachuv bachuv linked a pull request Jan 14, 2025 that will close this issue
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants