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.

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

Replace ITelemetryActivator with ITelemetryModule #3009

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Replace ITelemetryActivator with ITelemetryModule #3009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92d0340
initial commit
bachuv Jan 13, 2025
1876307
Merge branch 'dev' into vabachu/replace-itlemetryactivator
bachuv Jan 14, 2025
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
80 changes: 4 additions & 76 deletions src/WebJobs.Extensions.DurableTask/Correlation/TelemetryActivator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ namespace Microsoft.Azure.WebJobs.Extensions.DurableTask.Correlation
/// <summary>
/// TelemetryActivator initializes Distributed Tracing. This class only works for netstandard2.0.
/// </summary>
public class TelemetryActivator : ITelemetryActivator, IAsyncDisposable, IDisposable
public class TelemetryActivator : ITelemetryModule, IAsyncDisposable, IDisposable
{
private readonly DurableTaskOptions options;
private readonly INameResolver nameResolver;
private EndToEndTraceHelper endToEndTraceHelper;
private TelemetryClient telemetryClient;
private IAsyncDisposable telemetryModule;

Expand Down Expand Up @@ -57,7 +56,7 @@ public void Dispose()
/// <summary>
/// Initialize is initialize the telemetry client.
/// </summary>
public void Initialize(ILogger logger)
public void Initialize(TelemetryConfiguration configuration)
{
if (this.options.Tracing.DistributedTracingEnabled)
{
Expand All @@ -66,21 +65,18 @@ public void Initialize(ILogger logger)
return;
}

this.endToEndTraceHelper = new EndToEndTraceHelper(logger, this.options.Tracing.TraceReplayEvents);
TelemetryConfiguration telemetryConfiguration = this.SetupTelemetryConfiguration();

if (this.options.Tracing.Version == Options.DurableDistributedTracingVersion.V2)
{
DurableTelemetryModule module = new DurableTelemetryModule();
module.Initialize(telemetryConfiguration);
module.Initialize(configuration);
this.telemetryModule = module;
}
else
{
this.SetUpV1DistributedTracing();
if (CorrelationSettings.Current.EnableDistributedTracing)
{
this.SetUpTelemetryClient(telemetryConfiguration);
this.SetUpTelemetryClient(configuration);

if (CorrelationSettings.Current.EnableDistributedTracing)
{
Expand Down Expand Up @@ -128,75 +124,7 @@ private void SetUpTelemetryCallbacks()

private void SetUpTelemetryClient(TelemetryConfiguration telemetryConfiguration)
{
this.endToEndTraceHelper.ExtensionInformationalEvent(
hubName: this.options.HubName,
functionName: string.Empty,
instanceId: string.Empty,
message: "Setting up the telemetry client...",
writeToUserLogs: true);

this.telemetryClient = new TelemetryClient(telemetryConfiguration);
}

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(
hubName: this.options.HubName,
functionName: string.Empty,
instanceId: string.Empty,
message: "Both 'APPINSIGHTS_INSTRUMENTATIONKEY' and 'APPLICATIONINSIGHTS_CONNECTION_STRING' are defined in the current environment variables. Please specify one. We recommend specifying 'APPLICATIONINSIGHTS_CONNECTION_STRING'.");
}

if (!instrumentationKeyProvided && !connectionStringProvided)
{
this.endToEndTraceHelper.ExtensionWarningEvent(
hubName: this.options.HubName,
functionName: string.Empty,
instanceId: string.Empty,
message: "'APPINSIGHTS_INSTRUMENTATIONKEY' or 'APPLICATIONINSIGHTS_CONNECTION_STRING' were not defined in the current environment variables, but distributed tracing is enabled. Please specify one. We recommend specifying 'APPLICATIONINSIGHTS_CONNECTION_STRING'.");
}

if (instrumentationKeyProvided)
{
this.endToEndTraceHelper.ExtensionInformationalEvent(
hubName: this.options.HubName,
functionName: string.Empty,
instanceId: string.Empty,
message: "Reading APPINSIGHTS_INSTRUMENTATIONKEY...",
writeToUserLogs: true);

#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(
hubName: this.options.HubName,
functionName: string.Empty,
instanceId: string.Empty,
message: "Reading APPLICATIONINSIGHTS_CONNECTION_STRING...",
writeToUserLogs: true);

config.ConnectionString = resolvedConnectionString;
}

return config;
}
}
}
3 changes: 2 additions & 1 deletion src/WebJobs.Extensions.DurableTask/DurableTaskJobHostConfigurationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.ContextImplementations;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.Correlation;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.Options;
Expand Down Expand Up @@ -88,7 +89,7 @@ public static IWebJobsBuilder AddDurableTask(this IWebJobsBuilder builder)
serviceCollection.TryAddSingleton<IMessageSerializerSettingsFactory, MessageSerializerSettingsFactory>();
serviceCollection.TryAddSingleton<IErrorSerializerSettingsFactory, ErrorSerializerSettingsFactory>();
serviceCollection.TryAddSingleton<IApplicationLifetimeWrapper, HostLifecycleService>();
serviceCollection.AddSingleton<ITelemetryActivator, TelemetryActivator>();
serviceCollection.AddSingleton<ITelemetryModule, TelemetryActivator>();
serviceCollection.TryAddSingleton<IDurableClientFactory, DurableClientFactory>();
#pragma warning disable CS0612, CS0618 // Type or member is obsolete
serviceCollection.TryAddSingleton<IConnectionStringResolver, WebJobsConnectionStringProvider>();
Expand Down
103 changes: 0 additions & 103 deletions test/FunctionsV2/CorrelationEndToEndTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,109 +219,6 @@ internal async Task<Tuple<List<OperationTelemetry>, List<ExceptionTelemetry>>>
return new Tuple<List<OperationTelemetry>, List<ExceptionTelemetry>>(result.CorrelationSort(), exceptionTelemetryList);
}

/*
* End to end test that checks if a warning is logged when distributed tracing is
* enabled, but APPINSIGHTS_INSTRUMENTATIONKEY isn't set. The test also checks
* that the warning isn't logged when the environment variable is set.
*/
[Theory]
[Trait("Category", PlatformSpecificHelpers.TestCategory)]
[InlineData(false, false, false)]
[InlineData(false, false, true)]
[InlineData(true, false, false)]
[InlineData(true, false, true)]
[InlineData(false, true, false)]
[InlineData(false, true, true)]
[InlineData(true, true, false)]
[InlineData(true, true, true)]
public void TelemetryClientSetup_AppInsights_Warnings(bool instrumentationKeyIsSet, bool connStringIsSet, bool extendedSessions)
{
TraceOptions traceOptions = new TraceOptions()
{
DistributedTracingEnabled = true,
DistributedTracingProtocol = "W3CTraceContext",
};

DurableTaskOptions options = new DurableTaskOptions();
options.Tracing = traceOptions;

string instKeyEnvVarName = "APPINSIGHTS_INSTRUMENTATIONKEY";
string connStringEnvVarName = "APPLICATIONINSIGHTS_CONNECTION_STRING";
string environmentVariableValue = "test value";
string connStringValue = "InstrumentationKey=xxxx;IngestionEndpoint =https://xxxx.applicationinsights.azure.com/;LiveEndpoint=https://xxxx.livediagnostics.monitor.azure.com/";

var mockNameResolver = GetNameResolverMock(new[] { (instKeyEnvVarName, string.Empty), (connStringEnvVarName, string.Empty) });

if (instrumentationKeyIsSet && connStringIsSet)
{
mockNameResolver = GetNameResolverMock(new[] { (instKeyEnvVarName, environmentVariableValue), (connStringEnvVarName, connStringValue) });
}
else if (instrumentationKeyIsSet)
{
mockNameResolver = GetNameResolverMock(new[] { (instKeyEnvVarName, environmentVariableValue), (connStringEnvVarName, string.Empty) });
}
else if (connStringIsSet)
{
mockNameResolver = GetNameResolverMock(new[] { (instKeyEnvVarName, string.Empty), (connStringEnvVarName, connStringValue) });
}

using (var host = TestHelpers.GetJobHost(
this.loggerProvider,
"SingleOrchestration",
extendedSessions,
nameResolver: mockNameResolver.Object,
options: options))
{
string bothSettingsSetWarningMessage = "Both 'APPINSIGHTS_INSTRUMENTATIONKEY' and 'APPLICATIONINSIGHTS_CONNECTION_STRING' are defined in the current environment variables. Please specify one. We recommend specifying 'APPLICATIONINSIGHTS_CONNECTION_STRING'.";
var bothSettingsSetWarningLogMessage = this.loggerProvider.GetAllLogMessages().Where(l => l.FormattedMessage.StartsWith(bothSettingsSetWarningMessage));

string neitherSettingsSetWarningMessage = "'APPINSIGHTS_INSTRUMENTATIONKEY' or 'APPLICATIONINSIGHTS_CONNECTION_STRING' were not defined in the current environment variables, but distributed tracing is enabled. Please specify one. We recommend specifying 'APPLICATIONINSIGHTS_CONNECTION_STRING'.";
var neitherSettingsSetWarningLogMessage = this.loggerProvider.GetAllLogMessages().Where(l => l.FormattedMessage.StartsWith(neitherSettingsSetWarningMessage));

string settingUpTelemetryClientMessage = "Setting up the telemetry client...";
var settingUpTelemetryClientLogMessage = this.loggerProvider.GetAllLogMessages().Where(l => l.FormattedMessage.StartsWith(settingUpTelemetryClientMessage));

string readingInstrumentationKeyMessage = "Reading APPINSIGHTS_INSTRUMENTATIONKEY...";
var readingInstrumentationKeyLogMessage = this.loggerProvider.GetAllLogMessages().Where(l => l.FormattedMessage.StartsWith(readingInstrumentationKeyMessage));

string readingConnStringMessage = "Reading APPLICATIONINSIGHTS_CONNECTION_STRING...";
var readingConnStringLogMessage = this.loggerProvider.GetAllLogMessages().Where(l => l.FormattedMessage.StartsWith(readingConnStringMessage));

if (instrumentationKeyIsSet && connStringIsSet)
{
Assert.Single(bothSettingsSetWarningLogMessage);
Assert.Empty(neitherSettingsSetWarningLogMessage);
Assert.Single(settingUpTelemetryClientLogMessage);
Assert.Single(readingInstrumentationKeyLogMessage);
Assert.Single(readingConnStringLogMessage);
}
else if (instrumentationKeyIsSet && !connStringIsSet)
{
Assert.Empty(bothSettingsSetWarningLogMessage);
Assert.Empty(neitherSettingsSetWarningLogMessage);
Assert.Single(settingUpTelemetryClientLogMessage);
Assert.Single(readingInstrumentationKeyLogMessage);
Assert.Empty(readingConnStringLogMessage);
}
else if (!instrumentationKeyIsSet && connStringIsSet)
{
Assert.Empty(bothSettingsSetWarningLogMessage);
Assert.Empty(neitherSettingsSetWarningLogMessage);
Assert.Single(settingUpTelemetryClientLogMessage);
Assert.Empty(readingInstrumentationKeyLogMessage);
Assert.Single(readingConnStringLogMessage);
}
else
{
Assert.Empty(bothSettingsSetWarningLogMessage);
Assert.Single(neitherSettingsSetWarningLogMessage);
Assert.Single(settingUpTelemetryClientLogMessage);
Assert.Empty(readingInstrumentationKeyLogMessage);
Assert.Empty(readingConnStringLogMessage);
}
}
}

private static Mock<INameResolver> GetNameResolverMock((string Key, string Value)[] settings)
{
var mock = new Mock<INameResolver>();
Expand Down
3 changes: 2 additions & 1 deletion test/FunctionsV2/PlatformSpecificHelpers.FunctionsV2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.ApplicationInsights.Channel;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.Correlation;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.Options;
using Microsoft.Azure.WebJobs.Extensions.DurableTask.Storage;
Expand Down Expand Up @@ -89,7 +90,7 @@ public static ITestHost CreateJobHost(

if (onSend != null)
{
serviceCollection.AddSingleton<ITelemetryActivator>(serviceProvider =>
serviceCollection.AddSingleton<ITelemetryModule>(serviceProvider =>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jviau - after updating this line to register an ITelemetryModule for tests, I noticed that the lambda expression here doesn't get evaluated anymore and therefore the TelemetryActivators constructor and Intialize() method don't get called. I thought it was because the new Initialize() method takes a TelemetryConfiguration as an argument so I registered a TelemetryConfiguration as a singleton before line 91, but it's still not initializing TelemetryActivator. Do you know if there are any additional services I should register to get TelemetryActivator to register?

For context, the changes in this PR work when I manually test them in a sample DF app. The tests are just not passing because of the issue I described earlier.

Copy link
Contributor

@jviau jviau Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

ITelemetryModule's are resolved and invoked by the AppInsights SDK. Does the test code have the AppInsights SDK hooked up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The test code references Microsoft.ApplicationInsights through the package reference to the WebJobs extension. It doesn't have any code to hook up Application Insights though. I tried adding services.AddApplicationInsightsTelemetry(), but that ended up giving me an error that it needs an IHostingEnvironment so I wasn't sure if that was correct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jviau, were you thinking of an approach different from services.AddApplicationInsightsTelemetry()?

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly? I haven't looked at the exact test failures yet, but all of this is just a convenience part of the AppInsights SDK to have a defined hook for code to run as part of the SDK starting up. If that doesn't work for your tests, doing the same thing manually -- instantiating and calling the ITelemetryModule with the correct config instance is perfectly acceptable. What that looks like exactly I can't say without taking a deeper look.

{
var durableTaskOptions = serviceProvider.GetService<IOptions<DurableTaskOptions>>();
var nameResolver = serviceProvider.GetService<INameResolver>();
Expand Down
Loading