Skip to content

Commit

Permalink
Fix activity reporting (#306)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AndreyTretyak
AndreyTretyak authored Feb 18, 2021
1 parent aafa073 commit 0ae2c9d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 94 deletions.
63 changes: 3 additions & 60 deletions src/Extensions/Activities/Internal/DiagnosticsListener/DiagnosticsObserversInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Microsoft.Omex.Extensions.Abstractions;
using Microsoft.Omex.Extensions.Abstractions.Activities.Processing;

namespace Microsoft.Omex.Extensions.Activities
{
Expand All @@ -24,24 +23,12 @@ namespace Microsoft.Omex.Extensions.Activities
/// </remarks>
internal sealed class DiagnosticsObserversInitializer : IHostedService, IObserver<DiagnosticListener>, IObserver<KeyValuePair<string, object?>>
{
/// <summary>
/// Ending of the Activity Start event
/// </summary>
internal static readonly string ActivityStartEnding = ".Start";

/// <summary>
/// Ending of the Activity Stop event
/// </summary>
internal static readonly string ActivityStopEnding = ".Stop";

/// <summary>
/// Ending of the exception events
/// </summary>
internal static readonly string ExceptionEventEnding = "Exception";

private static readonly string[] s_eventEndMarkersToListen = new[] {
ActivityStartEnding,
ActivityStopEnding,
ExceptionEventEnding,
// We need to listen for the "Microsoft.AspNetCore.Hosting.HttpRequestIn" event in order to signal Kestrel to create an Activity for the incoming http request.
// Searching only for RequestIn, in case any other requests follow the same pattern
Expand All @@ -67,19 +54,12 @@ private static bool IsEnabled(string eventName)
return false;
}

private readonly IActivityStartObserver[] m_activityStartObservers;
private readonly IActivityStopObserver[] m_activityStopObservers;
private readonly ILogger<DiagnosticsObserversInitializer> m_logger;
private readonly LinkedList<IDisposable> m_disposables;
private IDisposable? m_observerLifetime;

public DiagnosticsObserversInitializer(
IEnumerable<IActivityStartObserver> activityStartObservers,
IEnumerable<IActivityStopObserver> activityStopObservers,
ILogger<DiagnosticsObserversInitializer> logger)
public DiagnosticsObserversInitializer(ILogger<DiagnosticsObserversInitializer> logger)
{
m_activityStartObservers = activityStartObservers.ToArray();
m_activityStopObservers = activityStopObservers.ToArray();
m_logger = logger;
m_disposables = new LinkedList<IDisposable>();
}
Expand All @@ -106,55 +86,19 @@ public void OnCompleted() { }
public void OnError(Exception error) =>
m_logger.LogError(Tag.Create(), error, "Exception in diagnostic events provider");

public void OnNext(DiagnosticListener value)
{
if (m_activityStartObservers.Length != 0 || m_activityStopObservers.Length != 0)
{
m_disposables.AddLast(value.Subscribe(this, IsEnabled));
}
}
public void OnNext(DiagnosticListener value) => m_disposables.AddLast(value.Subscribe(this, IsEnabled));

public void OnNext(KeyValuePair<string, object?> value)
{
Activity? activity = Activity.Current;
string eventName = value.Key;

if (activity != null)
{
if (EventEndsWith(eventName, ActivityStartEnding))
{
OnActivityStarted(activity, value.Value);
return;
}
else if (EventEndsWith(eventName, ActivityStopEnding))
{
OnActivityStopped(activity, value.Value);
return;
}
}
if (EventEndsWith(eventName, ExceptionEventEnding))
{
OnException(eventName, value.Value);
return;
}
}

private void OnActivityStarted(Activity activity, object? payload)
{
foreach (IActivityStartObserver startHandler in m_activityStartObservers)
{
startHandler.OnStart(activity, payload);
}
}

private void OnActivityStopped(Activity activity, object? payload)
{
foreach (IActivityStopObserver stopHandler in m_activityStopObservers)
{
stopHandler.OnStop(activity, payload);
}
}

private void OnException(string eventName, object? payload) =>
m_logger.LogError(
Tag.Create(),
Expand Down Expand Up @@ -194,7 +138,6 @@ private void OnException(string eventName, object? payload) =>
}
}

private static readonly ConcurrentDictionary<Type, PropertyInfo?> s_exceptionProperties =
new ConcurrentDictionary<Type, PropertyInfo?>();
private static readonly ConcurrentDictionary<Type, PropertyInfo?> s_exceptionProperties = new();
}
}
28 changes: 20 additions & 8 deletions tests/Extensions/Activities.UnitTests/Internal/ActivityListenerInitializerServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@

using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Diagnostics;
using Microsoft.Omex.Extensions.Activities;
using Moq;
using Microsoft.Extensions.Options;
using System;
using Microsoft.Omex.Extensions.Abstractions.Activities.Processing;
using System.Threading.Tasks;
using System.Threading;
Expand All @@ -20,8 +18,8 @@ public class ActivityListenerInitializerServiceTests
[TestMethod]
public async Task ActivityListeners_ControlsActivityCreation()
{
Mock<IActivityStartObserver> mockStartObserver = new Mock<IActivityStartObserver>();
Mock<IActivityStopObserver> mockStopObserver = new Mock<IActivityStopObserver>();
Mock<IActivityStartObserver> mockStartObserver = new();
Mock<IActivityStopObserver> mockStopObserver = new();

IOptionsMonitor<OmexActivityListenerOptions>? optionsMonitor = DefaultActivityListenerConfiguratorTests.CreateOptionsMonitor();
OmexActivityListenerOptions options = optionsMonitor.CurrentValue;
Expand All @@ -30,12 +28,12 @@ public async Task ActivityListeners_ControlsActivityCreation()
options.Sample = ActivitySamplingResult.AllDataAndRecorded;
options.SampleUsingParentId = ActivitySamplingResult.AllDataAndRecorded;

ActivityListenerInitializerService service = new ActivityListenerInitializerService(
ActivityListenerInitializerService service = new(
new IActivityStartObserver[] { mockStartObserver.Object },
new IActivityStopObserver[] { mockStopObserver.Object },
new DefaultActivityListenerConfigurator(optionsMonitor));

ActivitySource source = new ActivitySource(nameof(ActivityListeners_ControlsActivityCreation));
ActivitySource source = new(nameof(ActivityListeners_ControlsActivityCreation));

Assert.IsNull(source.StartActivity("ActivityBeforeStart"));

Expand All @@ -51,6 +49,20 @@ public async Task ActivityListeners_ControlsActivityCreation()

AssertActivityReporting(source.StartActivity("ActivityWhenConfigEnable"), mockStartObserver, mockStopObserver);

// check that Activity from Diagnostic listener also captured
using (DiagnosticListener listener = new(nameof(ActivityListeners_ControlsActivityCreation)))
{
Activity activity = new("DiagnosticsListenerActivity");

listener.StartActivity(activity, null);
mockStartObserver.Verify(s => s.OnStart(activity, null), Times.Once);
mockStartObserver.Reset();

listener.StopActivity(activity, null);
mockStopObserver.Verify(s => s.OnStop(activity, null), Times.Once);
mockStopObserver.Reset();
}

await service.StopAsync(CancellationToken.None);

Assert.IsNull(source.StartActivity("ActivityAfterStop"));
Expand All @@ -62,12 +74,12 @@ private void AssertActivityReporting(
Mock<IActivityStopObserver> mockStopObserver)
{
NullableAssert.IsNotNull(activity);
mockStartObserver.Verify(s => s.OnStart(It.IsAny<Activity>(), null), Times.Once);
mockStartObserver.Verify(s => s.OnStart(activity, null), Times.Once);
mockStartObserver.Reset();

activity.Dispose();

mockStopObserver.Verify(s => s.OnStop(It.IsAny<Activity>(), null), Times.Once);
mockStopObserver.Verify(s => s.OnStop(activity, null), Times.Once);
mockStopObserver.Reset();
}
}
Expand Down
33 changes: 7 additions & 26 deletions tests/Extensions/Activities.UnitTests/Internal/DiagnosticsObserversInitializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Omex.Extensions.Abstractions.Activities.Processing;
using Microsoft.Omex.Extensions.Activities;
using Microsoft.Omex.Extensions.Activities.UnitTests;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

Expand Down Expand Up @@ -44,56 +46,35 @@ public void ExtractExceptionFromPayload_HandleExceptionProperty()
public async Task DiagnosticsObserversInitializer_InvokedProperly()
{
string name = nameof(DiagnosticsObserversInitializer_InvokedProperly);
MockLogger logger = new MockLogger();
Mock<IActivityStartObserver> startObserver = new Mock<IActivityStartObserver>();
Mock<IActivityStopObserver> stopObserver = new Mock<IActivityStopObserver>();
DiagnosticsObserversInitializer initializer = new DiagnosticsObserversInitializer(
new[] { startObserver.Object },
new[] { stopObserver.Object },
logger);
MockLogger logger = new();
DiagnosticsObserversInitializer diagnosticsInitializer = new(logger);

try
{
await initializer.StartAsync(CancellationToken.None).ConfigureAwait(false);
await diagnosticsInitializer.StartAsync(CancellationToken.None).ConfigureAwait(false);

using DiagnosticListener listener = new DiagnosticListener(name);

AssertEnabledFor(listener, HttpRequestOutEventName);
AssertEnabledFor(listener, HttpRequestInEventName);
AssertEnabledFor(listener, ExceptionEventName);

AssertEnabledFor(listener, MakeStartName(name));
AssertEnabledFor(listener, MakeStopName(name));

Assert.IsFalse(listener.IsEnabled(name, "Should be disabled for other events"));

Activity activity = new Activity(name);
object obj = new object();

listener.StartActivity(activity, obj);
startObserver.Verify(obs => obs.OnStart(activity, obj), Times.Once);

listener.StopActivity(activity, obj);
stopObserver.Verify(obs => obs.OnStop(activity, obj), Times.Once);

Exception exception = new ArithmeticException();
listener.Write(ExceptionEventName, exception);
Assert.IsTrue(logger.Exceptions.Contains(exception), "Should log exception event");
}
catch
{
await initializer.StopAsync(CancellationToken.None).ConfigureAwait(false);
await diagnosticsInitializer.StopAsync(CancellationToken.None).ConfigureAwait(false);
throw;
}
}

private void AssertEnabledFor(DiagnosticListener listener, string eventName) =>
Assert.IsTrue(listener.IsEnabled(eventName), "Should be enabled for '{0}'", eventName);

private string MakeStartName(string name) => name + DiagnosticsObserversInitializer.ActivityStartEnding;

private string MakeStopName(string name) => name + DiagnosticsObserversInitializer.ActivityStopEnding;

private const string ExceptionEventName = "System.Net.Http.Exception";

private const string HttpRequestOutEventName = "System.Net.Http.HttpRequestOut";
Expand All @@ -102,7 +83,7 @@ private void AssertEnabledFor(DiagnosticListener listener, string eventName) =>

private class MockLogger : ILogger<DiagnosticsObserversInitializer>
{
public List<Exception> Exceptions { get; } = new List<Exception>();
public List<Exception> Exceptions { get; } = new();

public IDisposable BeginScope<TState>(TState state) => throw new NotImplementedException();

Expand Down

0 comments on commit 0ae2c9d

Please sign in to comment.