Add overload to prevent delegate allocation for ConfigureScope

[KnapSac]

Problem Statement

I use the following code to configure a scope, capturing a value to be set as a tag on the scope. Unfortunately, this always results in a delegate allocation because of the capture.

string tagValue = "value";
SentrySdk.ConfigureScope(scope => scope.SetTag("tag_name", tagValue));

Solution Brainstorm

Perhaps there already is a better way to set a tag on the current scope, but otherwise I'd propose adding the following methods:

public static partial class SentrySdk
{
+    public static void ConfigureScope<TArg>(Action<Scope, TArg> configureScope, TArg arg);
+    public static Task ConfigureScopeAsync<TArg>(Func<Scope, TArg, Task> configureScope, TArg arg);
}

These methods could then be used like this:

string tagValue = "value";
SentrySdk.ConfigureScope(
    static (scope, arg) => scope.SetTag("tag_name", arg),
    tagValue);

I'd be happy to provide the implementation if this is something you're interested in.

[jamescrosswell]

@bruno-garcia maybe you've got some context around this.

In the Hub we have this internally:

sentry-dotnet/src/Sentry/Internal/Hub.cs

Line 34 in 34d8490

private Scope CurrentScope => ScopeManager.GetCurrent().Key;

Why is it we only expose that via a ConfigureScope callback (instead of surfacing it directly via the IHub interface or some public extension method)?

[bruno-garcia]

@bruno-garcia maybe you've got some context around this.

In the Hub we have this internally:

sentry-dotnet/src/Sentry/Internal/Hub.cs

Line 34 in 34d8490

private Scope CurrentScope => ScopeManager.GetCurrent().Key;
Why is it we only expose that via a ConfigureScope callback (instead of surfacing it directly via the IHub interface or some public extension method)?

The idea is that once you return the reference explicitly you've lost ownership over it. One can hold it in a static field, access it concurrently etc. So having that state be accessed only behind our public API (SetTag, AddBreadcrumb, etc) allows us to control access, including having it be threadlocal, or asynclocal, or keep it per HTTP request (aspnet classic) etc.

[bruno-garcia]

We should start with adding SentrySdk.SetTag, surprised we actualy don't have that yet.

Also not against adding overloads that allow for 0 allocations of course, but I do think we can add some stuff to our public API like SetTag and other common things.

[KnapSac]

That would be even better, as you wouldn't have to go through the hassle of using a callback.

Would you be open to PRs for adding both (i.e. SentrySdk.SetTag and these 0 allocation overloads)?

[bruno-garcia]

That'd be awesome. Would you start with SetTag please? Think that's slam dunk. The other one we might want to discuss further (just want to hear from @james.crosswell and @bitsandfoxes before we go ahead)

[jamescrosswell]

SentrySdk.SetTag sounds like a great idea 🙌🏻 - would love a PR for that!

In principle, the overloads leveraging static lambdas are really nice as well. Having them there relies on people using them appropriately though. I think if we have methods like SentrySdk.SetTag then people are more likely to build code that is naturally performant (even if they don't know about or understand static lambda optimisation). I'm thinking if we don't have these overloads, that forces a conversation about other methods we could potentially add to SentrySdk. SentrySdk.SetExtra would be the other obvious one... but are there more?

On the other hand, having the overloads would provide ultimate flexibility. Maybe we do both then - maybe we add SentrySdk.SetTag and SentrySdk.SetExtra and also add the overloads.