Added DiagnosticSource for filters #3363

ajaybhargavb · 2015-10-20T23:46:31Z

rynowak · 2015-10-21T00:06:39Z

src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs

+                        actionDescriptor = ActionContext.ActionDescriptor,
+                        filterProviderContext = context
+                    });
+            }


This is totally out of place, what is this for?

At this point the FilterProviderContext contains a list of FIlterItem which provides the FilterDescriptors and Filters

but what is this for?

I thought @avanderhoorn needed the list of registered filters separately.

This event on it's own is out of place, all of these other events already provide this.

ajaybhargavb · 2015-10-21T01:28:35Z

Updated.

avanderhoorn · 2015-10-21T02:12:54Z

In terms of working out ExceptionFilter vs ActionFilter vs ResultFilter etc is the recommended approach here to derive this from the type of the filter?

rynowak · 2015-10-21T04:14:16Z

In terms of working out ExceptionFilter vs ActionFilter vs ResultFilter etc is the recommended approach here to derive this from the type of the filter?

From the name of the event - if you want to consolidate that and track it as 'generic filter' feel free.

avanderhoorn · 2015-10-21T04:38:38Z

Thanks

ajaybhargavb · 2015-10-22T23:21:48Z

Updated (with extension methods)

rynowak · 2015-10-22T23:23:47Z

src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs

+                _diagnosticSource.BeforeOnAuthorizationAsync(
+                    _authorizationContext.ActionDescriptor,
+                    _authorizationContext,
+                    current.FilterAsync);


Simplify this method to just take the _authorizationContext and current.FilterAsync

rynowak · 2015-10-22T23:25:05Z

after removing a parameter from all of the methods 👍

dnfclas added the cla-already-signed label Oct 20, 2015

rynowak reviewed Oct 21, 2015
View reviewed changes

ajaybhargavb force-pushed the telemetry-filters branch from c6e2cfd to a938d7c Compare October 21, 2015 01:28

ajaybhargavb force-pushed the telemetry-filters branch from a938d7c to bb1a4e6 Compare October 22, 2015 23:21

rynowak reviewed Oct 22, 2015
View reviewed changes

ajaybhargavb force-pushed the telemetry-filters branch from bb1a4e6 to f2bc1c0 Compare October 22, 2015 23:53

[Fixes #3279] Added DiagnosticSource for filters

b6d7012

ajaybhargavb force-pushed the telemetry-filters branch from f2bc1c0 to b6d7012 Compare October 23, 2015 19:56

ajaybhargavb merged commit b6d7012 into dev Oct 23, 2015

ajaybhargavb deleted the telemetry-filters branch October 23, 2015 19:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added DiagnosticSource for filters #3363

Added DiagnosticSource for filters #3363

ajaybhargavb commented Oct 20, 2015

rynowak Oct 21, 2015

ajaybhargavb Oct 21, 2015

rynowak Oct 21, 2015

ajaybhargavb Oct 21, 2015

rynowak Oct 21, 2015

ajaybhargavb commented Oct 21, 2015

avanderhoorn commented Oct 21, 2015

rynowak commented Oct 21, 2015

avanderhoorn commented Oct 21, 2015

ajaybhargavb commented Oct 22, 2015

rynowak Oct 22, 2015

rynowak commented Oct 22, 2015

Added DiagnosticSource for filters #3363

Added DiagnosticSource for filters #3363

Conversation

ajaybhargavb commented Oct 20, 2015

rynowak Oct 21, 2015

Choose a reason for hiding this comment

ajaybhargavb Oct 21, 2015

Choose a reason for hiding this comment

rynowak Oct 21, 2015

Choose a reason for hiding this comment

ajaybhargavb Oct 21, 2015

Choose a reason for hiding this comment

rynowak Oct 21, 2015

Choose a reason for hiding this comment

ajaybhargavb commented Oct 21, 2015

avanderhoorn commented Oct 21, 2015

rynowak commented Oct 21, 2015

avanderhoorn commented Oct 21, 2015

ajaybhargavb commented Oct 22, 2015

rynowak Oct 22, 2015

Choose a reason for hiding this comment

rynowak commented Oct 22, 2015