[ASP.NET Core] Option to prevent getting unviewed profilers

[hdt80]

Occasionally when starting a new profiler, the step named MiniProfiler Prep takes much longer than what is being profiled to complete. For example, a request to one of my API endpoints took 129.3ms to complete, with 127.50ms of that being MiniProfiler Prep (https://github.com/MiniProfiler/dotnet/blob/master/src/MiniProfiler.AspNetCore/MiniProfilerMiddleware.cs#L77).

I think this is due to the call to SetHeadersAndState (https://github.com/MiniProfiler/dotnet/blob/master/src/MiniProfiler.AspNetCore/MiniProfilerMiddleware.cs#L142), which gets the IDs of the unviewed profiles from the underlying Storage, which I have set to an SqlServerStorage, and this query occasionally takes a while to execute. This adds a lot of time to perform an API request, which is a bit longer than I'd prefer.

There are two solutions I have thought of to work around this problem.

1. Implement MiniProfilerOptions.ResultsAuthorize and implement middleware to handle authorization instead.

   • By implementing .ResultsAuthorize to only return true when actually viewing the results of a profiler, getting the unviewed IDs in .SetHeadersAndState is bypassed as isAuthorized would be false (https://github.com/MiniProfiler/dotnet/blob/master/src/MiniProfiler.AspNetCore/MiniProfilerMiddleware.cs#L150)
   • The additional middleware is implemented to prevent unauthorized access to the profiler data.

2. Created a wrapper IAsyncStorage around SqlServerStorage.

   • The wrapper IAsyncStorage would internally use an SqlServerStorage for all methods (ex: public async Task SaveAsync(MiniProfiler p) => _InternalStorage.SaveAsync(p)), except for GetUnviewedIds, it would just return an empty List<Guid>, and not execute the SQL query.

While I don't see any issues with both of these methods (Haven't actually tried them yet), I would much prefer just having an option during .ConfigureServices to not get the past profilers. For example:

services.AddMiniProfiler(options => {
    options.GrabPastProfilers = false;
});

This would be used in MiniProfilerOptions.SetHeadersAndState:

From:

var profilerIds = (isAuthorized ? await Options.ExpireAndGetUnviewedAsync(current.User).ConfigureAwait(false) : null)
    ?? new List<Guid>(1);

To:

var profilerIds = (isAuthorized && Options.GrabPastProfilers ? await Options.ExpireAndGetUnviewedAsync(current.User).ConfigureAwait(false) : null)
    ?? new List<Guid>(1);

Thank you for taking the time to read this, let me know if you have any questions and I'll get back to you as soon as I can.

[NickCraver]

It sounds like maybe this is attacking the wrong problem. If we're not intended to view the profilers: why run them in the first place? It seems like maybe plugging MiniProfilerOptions.ShouldProfile and not recording them in the first place would be a better approach. Or, you can call .Stop(discardResults: true) to throw away a profiler if you don't know until after it's started. Or, you can call <storage>.SetViewed(profiler) (or .SetViewedAsync) to mark it as viewed if you indeed want it marked that way.

In short...we have several alternatives here I think are worth fixing the problem far upstream. Still, I had this in mind when tweaking bits in #472. The MiniProfiler Init step (previously MiniProfiler Prep) has a few sub-steps (reported if they take longer than 0.1ms) to better illustrate where the time is going.

[TheCloudlessSky]

If we're not intended to view the profilers: why run them in the first place?

@NickCraver if you're profiling all requests in production (#499) wouldn't the desired behavior be to only allow privileged users (e.g. ResultsAuthorize(request) == true) to be able to SetViewed(), SetUnviewed(), and GetUnviewed()? What benefit would this have, for example, for non-staff/developer users of the application? Maybe I'm missing something.