-
Notifications
You must be signed in to change notification settings - Fork 287
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
AAD: with ServerTelemetryChannel #2292
Changes from 2 commits
25a4881
48b6546
adad52c
f4a6eed
834f859
eb5d445
fe955cb
363454c
96fc075
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
#if NET461 || NETCOREAPP2_1 || NETCOREAPP3_1 || NET5_0 | ||
namespace Microsoft.ApplicationInsights.TestFramework.Helpers | ||
{ | ||
using System; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
using Azure.Core; | ||
|
||
|
||
/// <remarks> | ||
/// Copied from (https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core.TestFramework/src/MockCredential.cs). | ||
/// </remarks> | ||
public class MockCredential : TokenCredential | ||
{ | ||
public override ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) | ||
{ | ||
return new ValueTask<AccessToken>(GetToken(requestContext, cancellationToken)); | ||
} | ||
|
||
public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) | ||
{ | ||
return new AccessToken("TEST TOKEN " + string.Join(" ", requestContext.Scopes), DateTimeOffset.MaxValue); | ||
} | ||
} | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
#if !NET452 && !NET46 | ||
namespace Microsoft.ApplicationInsights.TestFramework.Implementation | ||
{ | ||
using Microsoft.ApplicationInsights.Channel; | ||
using Microsoft.ApplicationInsights.Extensibility; | ||
using Microsoft.ApplicationInsights.TestFramework.Helpers; | ||
using Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel; | ||
using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
||
/// <summary> | ||
/// These tests verify that <see cref="TelemetryConfiguration"/> can receive and store an instance of <see cref="Azure.Core.TokenCredential"/>. | ||
/// </summary> | ||
/// <remarks> | ||
/// These tests do not run in NET452 OR NET46. | ||
/// In these cases, the test runner is NET452 or NET46 and Azure.Core.TokenCredential is NOT SUPPORTED in these frameworks. | ||
/// This does not affect the end user because we REQUIRE the end user to create their own instance of TokenCredential. | ||
/// This ensures that the end user is consuming the AI SDK in one of the newer frameworks. | ||
/// </remarks> | ||
[TestClass] | ||
[TestCategory("AAD")] | ||
public class ServerTelemetryChannelCredentialEnvelopeTests | ||
{ | ||
[TestMethod] | ||
public void VerifySetCredential_CorrectlySetsTelemetryChannel_CredentialFirst() | ||
{ | ||
// SETUP | ||
var tc = TelemetryConfiguration.CreateDefault(); | ||
Assert.IsInstanceOfType(tc.TelemetryChannel, typeof(InMemoryChannel)); | ||
Assert.IsTrue(tc.TelemetryChannel.EndpointAddress.Contains("v2")); // defaults to old api | ||
|
||
// ACT | ||
// set credential first | ||
tc.SetAzureTokenCredential(new MockCredential()); | ||
Assert.IsTrue(tc.TelemetryChannel.EndpointAddress.Contains("v2.1")); // api switch | ||
|
||
// test new channel | ||
var channel = new ServerTelemetryChannel(); | ||
Assert.IsNull(channel.EndpointAddress); // new channel defaults null | ||
|
||
// change config channel | ||
tc.TelemetryChannel = channel; | ||
Assert.IsTrue(channel.EndpointAddress.Contains("v2.1")); // configuration sets new api | ||
} | ||
|
||
[TestMethod] | ||
public void VerifySetCredential_CorrectlySetsTelemetryChannel_TelemetryChannelFirst() | ||
{ | ||
// SETUP | ||
var tc = TelemetryConfiguration.CreateDefault(); | ||
Assert.IsInstanceOfType(tc.TelemetryChannel, typeof(InMemoryChannel)); | ||
Assert.IsTrue(tc.TelemetryChannel.EndpointAddress.Contains("v2")); // defaults to old api | ||
|
||
// ACT | ||
// set new channel first | ||
var channel = new ServerTelemetryChannel(); | ||
Assert.IsNull(channel.EndpointAddress); // new channel defaults null | ||
|
||
// change config channel | ||
tc.TelemetryChannel = channel; | ||
Assert.IsTrue(channel.EndpointAddress.Contains("v2")); // configuration sets new api | ||
|
||
// set credential second | ||
tc.SetAzureTokenCredential(new MockCredential()); | ||
Assert.IsTrue(tc.TelemetryChannel.EndpointAddress.Contains("v2.1")); // api switch | ||
} | ||
} | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
namespace Microsoft.ApplicationInsights.Channel | ||
{ | ||
using System; | ||
using System.ComponentModel; | ||
using System.Diagnostics; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
@@ -15,7 +14,7 @@ | |
/// Represents a communication channel for sending telemetry to Application Insights via HTTPS. There will be a buffer that will not be persisted, to enforce the | ||
/// queued telemetry items to be sent, <see cref="ITelemetryChannel.Flush"/> should be called. | ||
/// </summary> | ||
public class InMemoryChannel : ITelemetryChannel, IAsyncFlushable | ||
public class InMemoryChannel : ITelemetryChannel, IAsyncFlushable, ISupportCredentialEnvelope | ||
{ | ||
private readonly TelemetryBuffer buffer; | ||
private readonly InMemoryTransmitter transmitter; | ||
|
@@ -96,6 +95,20 @@ public TimeSpan SendingInterval | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Gets or sets the <see cref="CredentialEnvelope"/> which is used for AAD. | ||
/// FOR INTERNAL USE. Customers should use <see cref="TelemetryConfiguration.SetAzureTokenCredential"/> instead. | ||
/// </summary> | ||
/// <remarks> | ||
/// <see cref="ISupportCredentialEnvelope.CredentialEnvelope"/> on <see cref="InMemoryChannel"/> sets <see cref="InMemoryTransmitter.CredentialEnvelope"/> | ||
/// which is used to set <see cref="Transmission.CredentialEnvelope"/> just before calling <see cref="Transmission.SendAsync"/>. | ||
/// </remarks> | ||
CredentialEnvelope ISupportCredentialEnvelope.CredentialEnvelope | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Access modifier is missing here. If the plan is to mark this method as internal, we could simplify the summary section removing "For internal use". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the reason this looks weird is because this is an explicitly implemented interface. The only way to avoid this is to make the new interface public. |
||
{ | ||
get => this.transmitter.CredentialEnvelope; | ||
set => this.transmitter.CredentialEnvelope = value; | ||
} | ||
|
||
/// <summary> | ||
/// Gets or sets the HTTP address where the telemetry is sent. | ||
/// </summary> | ||
|
@@ -126,20 +139,6 @@ public int BacklogSize | |
set { this.buffer.BacklogSize = value; } | ||
} | ||
|
||
/// <summary> | ||
/// Gets or sets the <see cref="CredentialEnvelope"/> which is used for AAD. | ||
/// FOR INTERNAL USE. Customers should use <see cref="TelemetryConfiguration.SetAzureTokenCredential"/> instead. | ||
/// </summary> | ||
/// <remarks> | ||
/// <see cref="InMemoryChannel.CredentialEnvelope"/> sets <see cref="InMemoryTransmitter.CredentialEnvelope"/> | ||
/// which is used to set <see cref="Transmission.CredentialEnvelope"/> just before calling <see cref="Transmission.SendAsync"/>. | ||
/// </remarks> | ||
internal CredentialEnvelope CredentialEnvelope | ||
{ | ||
get => this.transmitter.CredentialEnvelope; | ||
set => this.transmitter.CredentialEnvelope = value; | ||
} | ||
|
||
internal bool IsDisposed => this.isDisposed; | ||
|
||
/// <summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
namespace Microsoft.ApplicationInsights.Extensibility.Implementation.Authentication | ||
{ | ||
/// <summary> | ||
/// This interface defines a class that accepts the <see cref="Authentication.CredentialEnvelope"/> as a property. | ||
/// </summary> | ||
internal interface ISupportCredentialEnvelope | ||
{ | ||
/// <summary> | ||
/// Gets or sets the <see cref="Authentication.CredentialEnvelope"/>. | ||
/// </summary> | ||
CredentialEnvelope CredentialEnvelope { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ internal class ResponseStatusCodes | |
public const int InternalServerError = 500; | ||
public const int BadGateway = 502; | ||
public const int ServiceUnavailable = 503; | ||
public const int GatewayTimeout = 504; | ||
public const int GatewayTimeout = 504; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove this as there seems to no code using this. |
||
public const int Unauthorized = 401; // AAD | ||
public const int Forbidden = 403; // AAD | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woudl this work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found a way to use wildcards in the csproj.
The
NETCOREAPP
key word is only available as a preprocessor.link: https://docs.microsoft.com/en-us/dotnet/core/tutorials/libraries#how-to-multitarget
At line 24 specifically, this wouldn't work because these dependencies are for netcoreapp 2.1 only. These aren't shared with netcoreapp 3.1.