diff --git a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs index 96e8adedade5..67b7b6d3c95b 100644 --- a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs +++ b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.cs @@ -12,13 +12,16 @@ public DiagnosticListener(string name) { } public static IObservable 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 = null) { throw null; } public System.IDisposable Subscribe(System.IObserver> observer) { throw null; } public virtual System.IDisposable Subscribe(System.IObserver> observer, System.Predicate isEnabled) { throw null; } + public virtual System.IDisposable Subscribe(System.IObserver> observer, System.Func 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 = null) { throw null; } public abstract void Write(string name, object value); } } diff --git a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj index b053feb301bc..ff3eec1b15d3 100644 --- a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj +++ b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj @@ -10,8 +10,8 @@ - - + + diff --git a/src/System.Diagnostics.DiagnosticSource/src/DiagnosticSourceUsersGuide.md b/src/System.Diagnostics.DiagnosticSource/src/DiagnosticSourceUsersGuide.md index 6af22eb8f08a..800a6d395a62 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/DiagnosticSourceUsersGuide.md +++ b/src/System.Diagnostics.DiagnosticSource/src/DiagnosticSourceUsersGuide.md @@ -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 @@ -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 @@ -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 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 diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs index 8f9bb9e9f5cc..0e861c845945 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs @@ -2,12 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Threading; -using System.Diagnostics; using System.Collections.Generic; -using System.Diagnostics.Tracing; -using System.Runtime.CompilerServices; // TODO when we upgrade to C# V6 you can remove this. // warning CS0420: 'P.x': a reference to a volatile field will not be treated as volatile @@ -19,7 +15,7 @@ namespace System.Diagnostics { /// /// A DiagnosticListener is something that forwards on events written with DiagnosticSource. - /// It is an IObservable (has Subscribe method), and it also has a Subscribe overload that + /// It is an IObservable (has Subscribe method), and it also has a Subscribe overloads that /// lets you specify a 'IsEnabled' predicate that users of DiagnosticSource will use for /// 'quick checks'. /// @@ -59,24 +55,32 @@ public static IObservable AllListeners /// Add a subscriber (Observer). If 'IsEnabled' == null (or not present), then the Source's IsEnabled /// will always return true. /// - virtual public IDisposable Subscribe(IObserver> observer, Predicate isEnabled) + public virtual IDisposable Subscribe(IObserver> observer, Predicate 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)); + } + + /// + /// Add a subscriber (Observer). If 'IsEnabled' == null (or not present), then the Source's IsEnabled + /// will always return true. + /// + /// Subscriber (IObserver) + /// Filters events based on their name (string) and context objects that could be null. + /// Note that producer may first call filter with event name only and null context arguments and filter should + /// return true if consumer is interested in any of such events. Producers that support + /// context-based filtering will invoke isEnabled again with context for more prcise filtering. + /// Use Subscribe overload with name-based filtering if producer does NOT support context-based filtering + public virtual IDisposable Subscribe(IObserver> observer, Func isEnabled) + { + return SubscribeInternal(observer, name => IsEnabled(name, null, null), isEnabled); } + /// /// Same as other Subscribe overload where the predicate is assumed to always return true. /// public IDisposable Subscribe(IObserver> observer) { - return Subscribe(observer, null); + return SubscribeInternal(observer, null, null); } /// @@ -177,7 +181,21 @@ public override bool IsEnabled(string name) { for (DiagnosticSubscription curSubscription = _subscriptions; curSubscription != null; curSubscription = curSubscription.Next) { - if (curSubscription.IsEnabled == null || curSubscription.IsEnabled(name)) + if (curSubscription.IsEnabled1Arg == null || curSubscription.IsEnabled1Arg(name)) + return true; + } + return false; + } + + // NotificationSource implementation + /// + /// Override abstract method + /// + public override bool IsEnabled(string name, object arg1, object arg2 = null) + { + for (DiagnosticSubscription curSubscription = _subscriptions; curSubscription != null; curSubscription = curSubscription.Next) + { + if (curSubscription.IsEnabled3Arg == null || curSubscription.IsEnabled3Arg(name, arg1, arg2)) return true; } return false; @@ -196,7 +214,21 @@ public override void Write(string name, object value) private class DiagnosticSubscription : IDisposable { internal IObserver> Observer; - internal Predicate IsEnabled; + + // IsEnabled1Arg and IsEnabled3Arg represent IsEnabled callbacks. + // - IsEnabled1Arg invoked for DiagnosticSource.IsEnabled(string) + // - IsEnabled3Arg invoked for DiagnosticSource.IsEnabled(string, obj, obj) + // Subscriber MUST set both IsEnabled1Arg and IsEnabled3Arg or none of them: + // when Predicate is provided in DiagosticListener.Subscribe, + // - IsEnabled1Arg is set to predicate + // - IsEnabled3Arg falls back to predicate ignoring extra arguments. + // similarly, when Func is provided, + // IsEnabled1Arg falls back to IsEnabled3Arg with null context + // Thus, dispatching is very efficient when producer and consumer agree on number of IsEnabled arguments + // Argument number mismatch between producer/consumer adds extra cost of adding or omitting context parameters + internal Predicate IsEnabled1Arg; + internal Func IsEnabled3Arg; + internal DiagnosticListener Owner; // The DiagnosticListener this is a subscription for. internal DiagnosticSubscription Next; // Linked list of subscribers @@ -215,7 +247,7 @@ public void Dispose() var cur = newSubscriptions; while (cur != null) { - Debug.Assert(!(cur.Observer == Observer && cur.IsEnabled == IsEnabled), "Did not remove subscription!"); + Debug.Assert(!(cur.Observer == Observer && cur.IsEnabled1Arg == IsEnabled1Arg && cur.IsEnabled3Arg == IsEnabled3Arg), "Did not remove subscription!"); cur = cur.Next; } #endif @@ -233,14 +265,16 @@ private static DiagnosticSubscription Remove(DiagnosticSubscription subscription return null; } - if (subscriptions.Observer == subscription.Observer && subscriptions.IsEnabled == subscription.IsEnabled) + if (subscriptions.Observer == subscription.Observer && + subscriptions.IsEnabled1Arg == subscription.IsEnabled1Arg && + subscriptions.IsEnabled3Arg == subscription.IsEnabled3Arg) return subscriptions.Next; #if DEBUG // Delay a bit. This makes it more likely that races will happen. for (int i = 0; i < 100; i++) GC.KeepAlive(""); #endif - return new DiagnosticSubscription() { Observer = subscriptions.Observer, Owner = subscriptions.Owner, IsEnabled = subscriptions.IsEnabled, Next = Remove(subscriptions.Next, subscription) }; + return new DiagnosticSubscription() { Observer = subscriptions.Observer, Owner = subscriptions.Owner, IsEnabled1Arg = subscriptions.IsEnabled1Arg, IsEnabled3Arg = subscriptions.IsEnabled3Arg, Next = Remove(subscriptions.Next, subscription) }; } } @@ -341,6 +375,27 @@ public void Dispose() } #endregion + private IDisposable SubscribeInternal(IObserver> observer, Predicate isEnabled1Arg, Func isEnabled3Arg) + { + // If we have been disposed, we silently ignore any subscriptions. + if (_disposed) + { + return new DiagnosticSubscription() { Owner = this }; + } + DiagnosticSubscription newSubscription = new DiagnosticSubscription() + { + Observer = observer, + IsEnabled1Arg = isEnabled1Arg, + IsEnabled3Arg = isEnabled3Arg, + 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? diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs index d7e25c628e1f..17e9d5383cae 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSource.cs @@ -42,5 +42,24 @@ public abstract partial class DiagnosticSource /// /// The name of the event being written. public abstract bool IsEnabled(string name); + + /// + /// Optional: if there is expensive setup for the notification, you can call IsEnabled + /// before doing this setup with context + /// + /// The name of the event being written. + /// An object that represents the additional context for IsEnabled. + /// Consumers should expect to receive null which may indicate that producer called pure + /// IsEnabled(string) to check if consumer wants to get notifications for such events at all. + /// Based on it, producer may call IsEnabled(string, object, object) again with non-null context + /// Optional. An object that represents the additional context for IsEnabled. + /// Null by default. Consumers shoud expect to receive null which may indicate that producer + /// called pure IsEnabled(string) or producer passed all necessary context in arg1 + /// + public virtual bool IsEnabled(string name, object arg1, object arg2 = null) + { + return IsEnabled(name); + } + } } diff --git a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 6e02480de74f..675956e87bd2 100644 --- a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -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); diff --git a/src/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceTests.cs b/src/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceTests.cs index bd7c01b9ea81..518dce0c1987 100644 --- a/src/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceTests.cs +++ b/src/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceTests.cs @@ -124,14 +124,47 @@ public void BasicIsEnabled() using (listener.Subscribe(new ObserverToList(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); } } } + /// + /// Simple tests for the IsEnabled method. + /// + [Fact] + public void IsEnabledMultipleArgs() + { + using (DiagnosticListener listener = new DiagnosticListener("Testing")) + { + DiagnosticSource source = listener; + var result = new List>(); + Func isEnabled = (name, arg1, arg2) => + { + if (arg1 != null) + return (bool) arg1; + if (arg2 != null) + return (bool) arg2; + return true; + }; + + using (listener.Subscribe(new ObserverToList(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)); + } + } + } + /// /// Test if it works when you have two subscribers active simultaneously /// diff --git a/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj b/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj index eae7c7599e51..928effb00f37 100644 --- a/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj +++ b/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj @@ -16,5 +16,8 @@ + + + \ No newline at end of file