ApplicationInsights-dotnet
ApplicationInsights-dotnet copied to clipboard
Support interfaces for TelemetryConfiguration & TelemetryClient - primarily for testing & mocking needs
Problem: I was attempting to write a unit test today where I have code that is needing to resolve & reference the two types (TelemetryConfiguration and TelemetryClient) and found I was unable to mock them due to them being [sealed] classes and that there is no associated Interface for each.
Proposed Solution: Provide interfaces for these 2 types so that we can leverage mocking approaches in testing. Alternately, can they not be made [sealed] so that mocking tools (i.e. Moq is what I'm using) can
Alternatives Considered: None - we need to be able to mock which includes the having the ability to verify calls were made or not.
Context: In my particular case, I have code that is focused around the proper flushing & disposal of AppInsights resources; This stems from other well-known challenges with AppInsights and those areas of behavior. We wouldn't need the component in question and its test coverage if not for those behaviors needing to be applied. Addressing those root-cause issues (challenges) somewhat mitigates my needs here.
In my case we have a 2-stage lifecycle to account for (bootstrapping and the building/configuring the main host itself) as we use an initial bootstrap logger (Serilog) and configure it to write to the AppInsights sink. This bootstrapping process then includes an initial service provider instance for registering & resolving the AppInsights dependency for the bootstrap logger's AppInsight sink which requires either TelemetryConfiguration or TelemetryClient to be provided. It otherwise would use TelemetryConfiguration.Active which is being deprecated -or- it accepts an InstrumentationTkey (being deprecated) and does not support ConnectionStrings yet.
In our 'OnApplicationStopped' handler that I'm trying to write tests for, we ensure that we've flushed the TelemetryClient using the newer FlushAsync method and then we ensure that the TelemetryConfiguration is disposed of (for both sets of the objects.
The original author of this handler code was under the impression that all we needed to do was dispose of the TelemetryConfiguration but recent research was leading me to the impression that disposing of the configuration does not actually perform a flush (which has been requested by the community in another issue). My recent changes ensure both of these happen since we're not certain of what is really the "right way to handle flushing & disposing things" and want to ensure we're getting our full logging & telemetry contents sent to the server.
Instead of mocking TelemetryClient/TelemetryConfiguration, for unit tests, write yourself a custom telemetry channel, implementing ITelemetryChannel. For example:
using System;
using Microsoft.ApplicationInsights.Channel;
namespace YourCompnay.YourName.Tests
{
internal class MockTelemetryChannel : ITelemetryChannel
{
private readonly Action<ITelemetry> _sendAction;
public readonly Action _flushAction;
private readonly object _syncObject = new object();
public MockTelemetryChannel(Action<ITelemetry> sendAction, Action flushAction = null)
{
_sendAction = sendAction ?? throw new ArgumentNullException(nameof(sendAction));
_flushAction = flushAction;
}
public bool? DeveloperMode { get; set; }
public string EndpointAddress { get; set; }
public void Dispose()
{
}
public void Flush()
{
if (_flushAction != null)
{
lock (_syncObject)
{
_flushAction();
}
}
}
public void Send(ITelemetry item)
{
lock (_syncObject)
{
_sendAction(item);
}
}
}
}
and set it as the channel on the TelemetryConfiguration's TelemetryChannel instance. e.g.
var telemetryItems = new List<ITelemetryItem>();
bool flushCalled = false;
var telemetryChannel = new MockTelemetryChannel(telemetryItems.Add, () => flushCalled = true);
var telemetryConfiguration = new TelemetryConfiguration(yourInstrumentationKey, telemetryChannel);
var telemetryClient = new TelemetryClient(telemetryConfiguration);
Now, you can record all items and calls to Flush during your unit test and Assert the expected values. Does that help?
I appreciate the thought, but the code I need to test works against the TelemetryConfiguration & TelemetryClient and has no awareness or use of the TelemetryChannel. Its focus is on those 2 specific objects - and it needs to resolve them from the container where I'm managing them.
i.e.
// Arrange
var(bootstrapServiceProviderMock, bootstrapTelemetryConfigurationMock, bootstrapTelemetryClientMock) = CreateMocksForAppInsights();
var bootstrapContextMock = new Mock<IBootstrapContext>();
bootstrapContextMock.Setup(x => x.ServiceProvider).Returns(bootstrapServiceProviderMock.Object);
var(hostServiceProviderMock, hostTelemetryConfigurationMock, hostTelemetryClientMock) = CreateMocksForAppInsights();
// Act
var exception = Record.ExceptionAsync(
async () => await new OnApplicationStoppedHandlersForAzure().ExecuteHandlerForAppInsights(
bootstrapContextMock.Object,
hostServiceProviderMock.Object,
CancellationToken.None
).ConfigureAwait(false)
);
// Assert
exception.ShouldBeNull();
bootstrapTelemetryClientMock.Verify(client => client.FlushAsync(It.IsAny<CancellationToken>()), Times.Once());
bootstrapTelemetryConfigurationMock.Verify(config => config.Dispose(), Times.Once());
hostTelemetryClientMock.Verify(client => client.FlushAsync(It.IsAny<CancellationToken>()), Times.Once());
hostTelemetryConfigurationMock.Verify(config => config.Dispose(), Times.Once());
`
Also to clarify, I'm very specifically targeting use of the newer FlushAsync() method as from what I've read the old call to flush was not a blocking call and people were having to bake in arbitrary Sleep(..) calls which still didn't guarantee everything was flushed correctly. So TelemetryClient and the FlushAsync() are must haves for me.
That and in general I'm aiming for async end-to-end so I'm looking for the flush behavior to leverage a natively async method.
I'm not familiar with your code base, but presumably the necessary change will be in the CreateMocksForAppInsights method.
Note that TelemetryConfiguration.TelemetryChannel is writable. So, if you don't have direct control over the TelemetryConfiguration constructor, you can overwrite the TelemetryChannel after the fact.
To handle FlushAsync, your mock channel needs to implement IAsyncFlushable
Yes, that's the case - I'm accessing them from the service provider and with the 2-stage approach I have my original 'bootstrap context' which carries around the 1 instance of the services and then the host provides the 'real' set of services.
So the 'on stopped' handler is basically doing the resolves & then the flush/dispose on them - not the channel. The original author implemented the dispose handling on the TelemetryConfiguration and in my recent research I stumbled across the topics re: flushing via the TelemetryClient and Flush vs FlushAsync so I've tweaked the handler to now account for both. Is this just fundamentally wrong or any of it unnecessary?
The whole handling of flushing & disposing definitely seems to be a pain point with AI and compounded by anyone using a 2-stage initialization approach. Account for your logger/sinks and their flushing in a 2-stage approach to add more to the fun.
// Handle the 'bootstrap context' version of dependencies (if registered)
var bootstrapTelemetryClient = bootstrapContext.ServiceProvider.GetService<TelemetryClient>();
if (bootstrapTelemetryClient != null)
{
await bootstrapTelemetryClient.FlushAsync(cancellationToken).ConfigureAwait(false);
}
var bootstrapTelemetryConfiguration = bootstrapContext.ServiceProvider.GetService<TelemetryConfiguration>();
bootstrapTelemetryConfiguration?.Dispose();
// Handle the 'host' version of dependencies (if registered)
var hostTelemetryClient = services.GetService<TelemetryClient>();
if (hostTelemetryClient != null)
{
await hostTelemetryClient.FlushAsync(cancellationToken).ConfigureAwait(false);
}
var hostTelemetryConfiguration = services.GetService<TelemetryConfiguration>();
hostTelemetryConfiguration?.Dispose();
This issue is stale because it has been open 300 days with no activity. Remove stale label or this will be closed in 7 days. Commenting will instruct the bot to automatically remove the label.