Redesign/remove IOperationLogger

[Tratcher]

reverse-proxy/src/ReverseProxy.Core/Middleware/LoadBalancingMiddleware.cs

Lines 47 to 49 in 00c4e7c

	var endpoint = _operationLogger.Execute(
	"ReverseProxy.PickEndpoint",
	() => _loadBalancer.PickEndpoint(endpoints, in loadBalancingOptions));

Using the IOperationLogger allocates a closure per call per request. We need to re-think how to write these start/stop/failed logs.

[analogrelay]

Perhaps if there's Execute<T>(string, Action<T> callback, T state)? That would allow the func passed in to be static.

[davidfowl]

We should avoid this pattern in general.

[halter73]

What about a using based pattern with IDisposable structs/pooled objects? The downside is it wouldn't be able to log errors.

Or do we just give up on the idea of having a helper for logging the start and end of an operation and have everyone do it manually?

[davidfowl]

What about a using based pattern with IDisposable structs/pooled objects? The downside is it wouldn't be able to log errors.

It should be explicit.

Or do we just give up on the idea of having a helper for logging the start and end of an operation and have everyone do it manually?

I'd prefer that.

The issue with callbacks are the ones we all hate:

• Breaks control flow
• Creates closures, which then need a state object to be worked around
• Need async versions of the callback, now it's harder to optimize for the non-async code paths

[Tratcher]

@MihaZupan this directly relates to your diagnostics work this milestone, I'll assign it to you for tracking.

[MihaZupan]

Closing this issue as the problem discussed above ("Redesign/remove IOperationLogger") has been dealth with by #501.

Opening a new issue for the user story: #660