[WIP] Support IsEnabled with addtional arguments #15984 #15985
Conversation
|
/cc @karolz-ms @brahmnes |
| /// <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) |
There was a problem hiding this comment.
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.
|
|
||
| // Subscription implementation | ||
| /// <summary> | ||
| /// Add a subscriber (Observer). If 'IsEnabled' == null (or not present), then the Source's IsEnabled |
There was a problem hiding this comment.
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.
| internal IObserver<KeyValuePair<string, object>> Observer; | ||
|
|
||
| internal Predicate<string> IsEnabled; | ||
| internal Func<string, object, object, bool> IsEnabledExt; |
There was a problem hiding this comment.
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.
|
I have left some feedback to document the code a bit more, but substance of the code looks good. |
…is_enabled [WIP] Support IsEnabled with addtional arguments dotnet/corefx#15984 Commit migrated from dotnet/corefx@d388751
…is_enabled [WIP] Support IsEnabled with addtional arguments dotnet/corefx#15984 Commit migrated from dotnet/corefx@d388751
4 participants
This change introduces IsEnabled(string, object, object) obverload in DiagnosticSource for advanced events filtering based on additional event context.
It also adds DiagnosticListener.Subscribe overload capable of filtering events based on such context.
See #15984 for more details