AAD: Handling Azure.Core.TokenCredential

[TimothyMothra]

Adding support for Azure.Core.TokenCredential to the TelemetryConfiguration

For more information see the parent item: #2190.

Introduction

We have a unique challenge of needing to support Azure.Core.TokenCredential while having incompatible Frameworks.

To address this I'm making the assumption that if a customer's application is able to create an instance of TokenCredential, then they also have the Azure.Core assembly in their runtime.
Going off of this assumption, we're able to use reflection to access the methods of TokenCredential and return types available in Azure.Core.
This approach is validated in unit tests.

Implementation

TelemetryConfiguration.SetCredential

We need a way for a customer to provide an implementation of Azure.Core.TokenCredential.
For this, I've introduced a new method TelemetryConfiguration.SetCredential.

ApplicationInsights-dotnet/BASE/src/Microsoft.ApplicationInsights/Extensibility/TelemetryConfiguration.cs

Lines 408 to 412 in aac78ed

	/// <summary>
	/// Set a TokenCredential for this configuration.
	/// </summary>
	/// <param name="tokenCredential">An instance of Azure.Core.TokenCredential.</param>
	public void SetCredential(object tokenCredential) => this.CredentialEnvelope = new ReflectionCredentialEnvelope(tokenCredential);

We accept Object and validate that the provided object inherits TokenCredential.
We store this in the new property TelemetryConfiguration.CredentialEnvelope which is accessible by all internal classes in the SDK.

CredentialEnvelope

We hold the instance of TokenCredential in a new class ReflectionCredentialEnvelope which as the name suggests, uses Reflection to interact with the Object that was provided.

ApplicationInsights-dotnet/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Authentication/ReflectionCredentialEnvelope.cs

Lines 21 to 37 in aac78ed

	/// <summary>
	/// Create an instance of <see cref="ReflectionCredentialEnvelope"/>.
	/// </summary>
	/// <param name="tokenCredential">An instance of Azure.Core.TokenCredential.</param>
	public ReflectionCredentialEnvelope(object tokenCredential)
	{
	this.tokenCredential = tokenCredential ?? throw new ArgumentNullException(nameof(tokenCredential));
	if (tokenCredential.GetType().IsSubclassOf(Type.GetType("Azure.Core.TokenCredential, Azure.Core")))
	{
	this.tokenRequestContext = AzureCore.MakeTokenRequestContext(scopes: CredentialConstants.GetScopes());
	}
	else
	{
	throw new ArgumentException($"The provided {nameof(tokenCredential)} must inherit Azure.Core.TokenCredential", nameof(tokenCredential));
	}
	}

[cijothomas]

Consider making this internal, if end users are not expected to read this back.

[TimothyMothra]

@pharring, We're planning on making the CredentialEnvelope internal.
We want to get your library added to our InternalsVisibleTo here.

[pharring]

@xiaomi7732 for https://github.com/microsoft/ApplicationInsights-Profiler-AspNetCore

Snapshot Collector is Microsoft.ApplicationInsights.SnapshotCollector, but note that it has a different public key.

[xiaomi7732]

Thanks for tagging me on this. AI.SDK and Profiler have different build cycles - and we have mixed references. How are we going to make that work?

For example, our desktop build (no StrongName) assemblies might reference public/stable version of app insights SDK, in which case, the InternvalVisibileTo (to a strongNamed assembly) won't work. What is that I missed?

[pharring]

I don't think you need to worry about version numbers. InternalsVisibleTo uses just the assembly's name and public key. The question really is whether you expect to take a dependency on a version of the core SDK that has CredentialEnvelope, or would you stick with an older version and use reflection to get CredentialEnvelope.

I still haven't decided for Snapshot Collector. There's going to be some reflection anyway in order to use the CredentialEnvelope.

[xiaomi7732]

On this topic, I think I am asked to provide assembly names and public keys so that in Core SDK, it knows to expose the internal classes.

The deal is, during the development, our assemblies aren't going to be strongnamed - that means we won't be able to see the internal classes in the Core SDK anyway.

On the other hand, reflection will work, with or without InternalVisibleTo.

So, what is the point to add the InternalVisibleTo for our assemblies? And that is what I am asking, in case I missed something.

[TimothyMothra]

We're changing this property to internal.
Optionally, if you want to utilize this without reflection you can submit a PR to add your SDK to our InternalsVisibleTo.

[TimothyMothra]

@cijothomas , Is ConfigureAwait(true) the correct usage for SdkInternalOperationsMonitor ?

[TimothyMothra]

I think I answered my own question.
ProfileServiceWrapper calls Async methods inside an InternalOperation.

ApplicationInsights-dotnet/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/ApplicationId/ProfileServiceWrapper.cs

Lines 86 to 107 in e1ff87e

	private async Task<string> SendRequestAsync(string instrumentationKey)
	{
	try
	{
	SdkInternalOperationsMonitor.Enter();
	var resultMessage = await this.GetAsync(instrumentationKey).ConfigureAwait(false);
	if (resultMessage.IsSuccessStatusCode)
	{
	return await resultMessage.Content.ReadAsStringAsync().ConfigureAwait(false);
	}
	else
	{
	this.FailedRequestsManager.RegisterFetchFailure(instrumentationKey, resultMessage.StatusCode);
	return null;
	}
	}
	finally
	{
	SdkInternalOperationsMonitor.Exit();
	}
	}

[cijothomas]

as a follow up, we could modify the method comment to be more descriptive, and can warn that this throws exception when object is not of type Azure.Core.TokenCredential.

[cijothomas]

an alternate name could be - SetTokenCredential or SetAzureTokenCredential, to make it more clear that this is an azure only thing.

[cijothomas]

it may be better to leave the job of entering/exiting internal operation to the consumer (like Transmission class)

[TimothyMothra]

Will investigate this in second PR.