Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ public DiagnosticListener(string name) { }
public static IObservable<DiagnosticListener> AllListeners { get { throw null; } }
public virtual void Dispose() { }
public override bool IsEnabled(string name) { throw null; }
public override bool IsEnabled(string name, object arg1, object arg2) { throw null; }
public System.IDisposable Subscribe(System.IObserver<System.Collections.Generic.KeyValuePair<string, object>> observer) { throw null; }
public virtual System.IDisposable Subscribe(System.IObserver<System.Collections.Generic.KeyValuePair<string, object>> observer, System.Predicate<string> isEnabled) { throw null; }
public virtual System.IDisposable Subscribe(System.IObserver<System.Collections.Generic.KeyValuePair<string, object>> observer, System.Func<string, object, object, bool> isEnabled) { throw null; }
public override void Write(string name, object parameters) { }
}
public abstract partial class DiagnosticSource {
protected DiagnosticSource() { }
public abstract bool IsEnabled(string name);
public virtual bool IsEnabled(string name, object arg1, object arg2) { throw null; }
public abstract void Write(string name, object value);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<ItemGroup>
<Compile Include="System.Diagnostics.DiagnosticSource.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetGroup)' == 'netstandard1.1'">
<Compile Include="System.Diagnostics.DiagnosticSourceActivity.cs" />
<ItemGroup Condition="'$(TargetGroup)' != 'netstandard1.1'">
<Compile Include="System.Diagnostics.DiagnosticSourceActivity.cs" />
<Reference Include="System.Runtime" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,21 @@ Thus the event names only need to be unique within a component.

* DO - always enclose the Write() call in a call to 'IsEnabled' for the same event name. Otherwise
a lot of setup logic will be called even if there is nothing listening for the event.

* DO NOT - make the DiagnosticListener public. There is no need to as subscribers will
use the AllListener property to hook up.

* CONSIDER - passing public named types instances to 'IsEnabled' overloads with object parameters
to keep IsEnabled as efficient as possible.

----------------------------------------
* CONSIDER - enclosing IsEnabled(string, object, object) calls with pure IsEnabled(string) call to avoid
extra cost of creating context in case consumer is not interested in such events at all.

* DO - when subscribing to DiagnosticSource with advanced filter for event name and extended context,
make sure filter returns true for null context properties if consumer is interested
in at least some events with context

----------------------------------------
## Consuming Data with DiagnosticListener.

Up until now, this guide has focused on how to instrument code to generate logging
Expand Down Expand Up @@ -337,8 +347,8 @@ ReflectEmit, but that is beyond the scope of this document.
#### Filtering

In the example above the code uses the IObservable.Subscribe to hook up the callback, which
causes all events to be given to the callback. However DiagnosticListener has a overload of
Subscribe that allows the controller to control which events get through.
causes all events to be given to the callback. However DiagnosticListener has a overloads of
Subscribe that allow the controller to control which events get through.

Thus we could replace the listener.Subscribe call in the previous example with the following
code
Expand All @@ -362,6 +372,43 @@ code
Which very efficiently only subscribes to the 'RequestStart' events. All other events will cause the DiagnosticSource.IsEnabled()
method to return false, and thus be efficiently filtered out.

##### Context-based Filtering
Some scenarios require advanced filtering based on extended context.
Producers may call DiagnosticSource.IsEnabled() overloads and supply additional event properties.

```C#
if (httpLogger.IsEnabled("RequestStart", aRequest, anActivity))
httpLogger.Write("RequestStart", new { Url="http://clr", Request=aRequest });
```

And consumers may use such properties to filter events more precisely.

```C#
// Create a predicate (asks only for Requests for certains URIs)
Func<string, object, bool> predicate = (string eventName, object context, object activity) =>
{
if (eventName == "RequestStart")
{
HttpRequestMessage request = context as HttpRequestMessage;
if (request != null)
{
return IsUriEnabled(request.RequestUri);
}
}
return false;
}

// Subscribe with a filter predicate
IDisposable subscription = listener.Subscribe(observer, predicate);
```

Note that producer is not aware of filter consumer has provided. DiagnosticListener
will invoke provided filter ommiting additional arguments if necessary, thus the filter
should expect to receive null context.
Producers should enclose IsEnabled call with event name and context with pure IsEnabled
call for event name, so consumers must ensure that filter allows events without context
to pass through.

----------------------------------------
## Consuming DiagnosticSource Data with with EventListeners and ETW

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,27 @@ public static IObservable<DiagnosticListener> AllListeners
/// Add a subscriber (Observer). If 'IsEnabled' == null (or not present), then the Source's IsEnabled
/// will always return true.
/// </summary>
virtual public IDisposable Subscribe(IObserver<KeyValuePair<string, object>> observer, Predicate<string> isEnabled)
public virtual IDisposable Subscribe(IObserver<KeyValuePair<string, object>> observer, Predicate<string> isEnabled)
{
// If we have been disposed, we silently ignore any subscriptions.
if (_disposed)
{
return new DiagnosticSubscription() { Owner = this };
}
DiagnosticSubscription newSubscription = new DiagnosticSubscription() { Observer = observer, IsEnabled = isEnabled, Owner = this, Next = _subscriptions };
while (Interlocked.CompareExchange(ref _subscriptions, newSubscription, newSubscription.Next) != newSubscription.Next)
newSubscription.Next = _subscriptions;
return newSubscription;
return SubscribeInternal(observer, isEnabled, (name, arg1, arg2) => isEnabled(name));
}

// Subscription implementation
/// <summary>
/// Add a subscriber (Observer). If 'IsEnabled' == null (or not present), then the Source's IsEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Add more description of what the arguments of 'isEnabled' are (the first is the name of the event, the next are context object (which may be null) that are optionally passed to the isEnabled call from the DiagnosticSource.

/// will always return true.
/// </summary>
public virtual IDisposable Subscribe(IObserver<KeyValuePair<string, object>> observer, Func<string, object, object, bool> isEnabled)
{
return SubscribeInternal(observer, name => IsEnabled(name, null, null), isEnabled);
}

/// <summary>
/// Same as other Subscribe overload where the predicate is assumed to always return true.
/// </summary>
public IDisposable Subscribe(IObserver<KeyValuePair<string, object>> observer)
{
return Subscribe(observer, null);
return SubscribeInternal(observer, null, null);
}

/// <summary>
Expand Down Expand Up @@ -183,6 +186,20 @@ public override bool IsEnabled(string name)
return false;
}

// NotificationSource implementation
/// <summary>
/// Override abstract method
/// </summary>
public override bool IsEnabled(string name, object arg1, object arg2)
{
for (DiagnosticSubscription curSubscription = _subscriptions; curSubscription != null; curSubscription = curSubscription.Next)
{
if (curSubscription.IsEnabledExt == null || curSubscription.IsEnabledExt(name, arg1, arg2))
return true;
}
return false;
}

/// <summary>
/// Override abstract method
/// </summary>
Expand All @@ -196,7 +213,9 @@ public override void Write(string name, object value)
private class DiagnosticSubscription : IDisposable
{
internal IObserver<KeyValuePair<string, object>> Observer;

internal Predicate<string> IsEnabled;
internal Func<string, object, object, bool> IsEnabledExt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename IsEnabled -> IsEnabled1Arg, and IsEnabled3Arg Also place a comment that any subscriber must set BOTH of these delegates (either they are both null or both are valid (typically one delegates to the other). ALso indicate the rationale (we do this so that if both the source and receiver agree on the number of args, dispatch is very efficient. Only in a mismatch is a 'fixup' thunk called.

internal DiagnosticListener Owner; // The DiagnosticListener this is a subscription for.
internal DiagnosticSubscription Next; // Linked list of subscribers

Expand Down Expand Up @@ -341,6 +360,27 @@ public void Dispose()
}
#endregion

private IDisposable SubscribeInternal(IObserver<KeyValuePair<string, object>> observer, Predicate<string> isEnabled, Func<string, object, object, bool> isEnabledExt)
{
// If we have been disposed, we silently ignore any subscriptions.
if (_disposed)
{
return new DiagnosticSubscription() { Owner = this };
}
DiagnosticSubscription newSubscription = new DiagnosticSubscription()
{
Observer = observer,
IsEnabled = isEnabled,
IsEnabledExt = isEnabledExt,
Owner = this,
Next = _subscriptions
};

while (Interlocked.CompareExchange(ref _subscriptions, newSubscription, newSubscription.Next) != newSubscription.Next)
newSubscription.Next = _subscriptions;
return newSubscription;
}

private volatile DiagnosticSubscription _subscriptions;
private DiagnosticListener _next; // We keep a linked list of all NotificationListeners (s_allListeners)
private bool _disposed; // Has Dispose been called?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,19 @@ public abstract partial class DiagnosticSource
/// </summary>
/// <param name="name">The name of the event being written.</param>
public abstract bool IsEnabled(string name);

/// <summary>
/// Optional: if there is expensive setup for the notification, you can call IsEnabled
/// before doing this setup with context
/// </summary>
/// <param name="name">The name of the event being written.</param>
/// <param name="arg1">An object that represents the additional context for IsEnabled</param>
/// <param name="arg2">An object that represents the additional context for IsEnabled</param>
/// <seealso cref="IsEnabled(string)"/>
public virtual bool IsEnabled(string name, object arg1, object arg2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a problem with using default args (arg2=null). I would expect this here, in the overload in DiagnosticListener, and in the reference assemblies.

Please put in the comment that arg1, and arg2 can be null.

{
return IsEnabled(name);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,8 @@ public void IdGenerationInternalParent()
var child1 = new Activity("child1");
var child2 = new Activity("child2");
//start 2 children in different execution contexts
var t1 = Task.Run(() => child1.Start());
var t2 = Task.Run(() => child2.Start());
Task.WhenAll(t1, t2).Wait();
Task.Run(() => child1.Start()).Wait();
Task.Run(() => child2.Start()).Wait();
#if DEBUG
Assert.Equal($"{parent.Id}.{child1.OperationName}_1", child1.Id);
Assert.Equal($"{parent.Id}.{child2.OperationName}_2", child2.Id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,47 @@ public void BasicIsEnabled()
using (listener.Subscribe(new ObserverToList<TelemData>(result), predicate))
{
Assert.False(source.IsEnabled("Uninteresting"));
Assert.False(source.IsEnabled("Uninteresting", "arg1", "arg2"));
Assert.True(source.IsEnabled("StructPayload"));

Assert.True(source.IsEnabled("StructPayload", "arg1", "arg2"));
Assert.True(seenUninteresting);
Assert.True(seenStructPayload);
}
}
}

/// <summary>
/// Simple tests for the IsEnabled method.
/// </summary>
[Fact]
public void IsEnabledMultipleArgs()
{
using (DiagnosticListener listener = new DiagnosticListener("Testing"))
{
DiagnosticSource source = listener;
var result = new List<KeyValuePair<string, object>>();
Func<string, object, object, bool> isEnabled = (name, arg1, arg2) =>
{
if (arg1 != null)
return (bool) arg1;
if (arg2 != null)
return (bool) arg2;
return true;
};

using (listener.Subscribe(new ObserverToList<TelemData>(result), isEnabled))
{
Assert.True(source.IsEnabled("event"));
Assert.True(source.IsEnabled("event", null, null));
Assert.True(source.IsEnabled("event", null, true));

Assert.False(source.IsEnabled("event", false, false));
Assert.False(source.IsEnabled("event", false, null));
Assert.False(source.IsEnabled("event", null, false));
}
}
}

/// <summary>
/// Test if it works when you have two subscribers active simultaneously
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,8 @@
<Compile Include="DiagnosticSourceEventSourceBridgeTests.cs" />
<Compile Include="DiagnosticSourceTests.cs" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetGroup)' != 'netstandard1.1'">
<Compile Include="ActivityTests.cs" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
</Project>