Proposal: A clear and standard syntax for instrumentation

[msfcolombo]

[adapted from Roslyn Issue 16896]

Background

It's common in the implementation of logging, retry handlers and other helpers, to have a method that observes and manages execution of a code block. This is called manual or explicit "instrumentation", and in C# is usually performed with lambdas:

Instrument("MyAction", () =>
{
    // action code
} );

Lambdas imply the following:

• They cannot interact with parent control flow, such as breaking or continuing a for statement, or immediately return a value for the declaring method. They force the developer to return a flag or temporary object to the observer method, which then returns to the caller, which then must be evaluated and processed. That's a lot of boilerplate just to add instrumentation.
• The observer interface and implementation typically require multiple versions: one for sync and other for async code, one for void and other for returned values. Providers can forget to implement one version.
• The stacktrace contains two additional entries: the Observe call and an internally generated method for the lambda.
• Lambdas often require an additional heap object to provide access to variables outside the lambda body.

Some people suggest the using keyword as way of avoiding lambda issues, but using doesn't notify the IDisposable object about exceptions. This prevents observation of certain outcomes. Also, the IDisposable interface is designed for release of unmanaged resources. There is a general assumption that an object cannot be reused after it's disposed, and this assumption doesn't hold for observers. And finally, using cannot transparently issue a retry attempt.

Proposal

The proposal is to make C# support a clear form of instrumentation:

with (object-Expr)
{
    // action code
}

The object-Expr must return an instance of IInstrument. The action code is free to return a value for the declaring method, or to interact with parent flow statements (ie, it can have break and continue). This makes instrumentation much cleaner and less invasive and disruptive than a lambda. There are clear benefits in readability and editability, not to mention simpler stacktraces and mildly improved performance.

The above code is compiled roughly as if the developer had written:

var _temp = object-Expr;
for (var _i = 0; ; ++_i) {
    _temp.NotifyAttempt(_i);
    try
    {
        // action code
    }
    catch (Exception ex)
    {
        if (_temp.NotifyException(_i, ex))
            throw; // Instrument wants exception to be forwarded to caller.
        continue; // Instrument wants a retry.
    }
    _temp.NotifySuccess(_i);
    break;
}

With this feature, a provider of IInstrument implementation can:

• Determine the time when the action started and finished, regardless of outcome.
• Decide if an exception is abortive or if a retry should be attempted.
• Retry as many times as wanted, knowing the number of each iteration.
• Cache and reuse the object especified by object-Expr.
• Throw an exception if the action took longer than expected, at the NotifySuccess call.

The provider can also back off (for instance, sleep) during execution of NotifyAttempt. In order to support async code, the IInstrument interface should also declare the async version NotifyAttemptAsync. The compiler decides which method to call from the style of parent method. This brings the additional benefits:

• The provider of an observer cannot forget to support asynchronous code.
• The user will have a single syntax for both sync and async code: with (foo) { ... }.

[HaloFour]

I have some qualms about C# supporting both using and with to both perform unit-of-work operations. Intention aside, they both largely do the same thing. This seems to mostly just expand on what things it could do.

I'd be fine with the use of an interface that offers all of the supported members, but I would also prefer the compiler support instrumentation by convention. That way as long as the instrumentation value exposes the correct accessible members (or they can be resolved via extension methods), the code would continue to compile. I would also allow for partial implementations of the convention.

One thing that seems missing is a way to define a back-off algorithm for retries. At best I could see the instrumentation value exposing a public property with the attempt and to have the action manually implement the back-off strategy, but I could see that being built into the instrumentation value.

[jnm2]

I think using (instrument) would be my first choice for readability. It would look for either IDisposable or IInstrument-like types. Only pitfall there is you may actually think an instrument is a disposable and not realize it could execute your code twice.

[scottdorman]

I'm fine with either approach, adding a with block or expanding using to work with something more than just IDisposable.

I don't care of the IInstrument name for the interface as I think it's too context specific around a specific use case scenario.

As for using with as the new keyword, I wonder if we can come up with something better? Taking a cue from VB, a with block was used to simplify setting a large number of properties on an object. this is similar to what we can do now with object initializers, but allows them to be performed at any time not just at construction. I'd hate to loose the ability to do that in C# at some point through a with block, which makes a lot of syntactic sense in that context because we're already using it for something completely different. That being said, I'm not sure what other term we could use since with makes a lot of sense for either use. Maybe scoped?

@jnm2's point about accidentally thinking an instrument (still don't like that term) is disposable when it isn't is valid and is a drawback to allowing using to work with both.

[msfcolombo]

Perhaps for is a valid alternative for the keyword?

Currently, for imposes the syntax for ( [initializer] ; [condition] ; [iterator] ). The two semicolons are mandatory.

It looks viable to support a new syntax, for ( obj ), which can be detected by the lack of semicolons. It also gives the notion that the body can be executed multiple times, at discretion of obj.

[scottdorman]

@msfcolombo

It also gives the notion that the body can be executed multiple times

Yes, it gives the notion that it can be executed multiple times, but it also gives the implication that it's a loop, which it isn't.

[scottdorman]

As context for a better keyword, consider the following cursor change class:

public class WaitCursor : IDisposable
{
    public WaitCursor()
    {
        IsWaitCursor = true;
    }

    public void Dispose()
    {
        IsWaitCursor = false;
    }

    public bool IsWaitCursor
    {
        get
        {
            return Application.UseWaitCursor;
        }
        set
        {
            if (Application.UseWaitCursor != value)
            {
                Application.UseWaitCursor = value;
                Cursor.Current = value ? Cursors.WaitCursor : Cursors.Default;
            }
        }
    }
}

The proposed choices for syntax are:

// Current working syntax
using (var waitCursor = new WaitCursor())
{
}

// Proposed new syntax options
with (var waitCursor = new WaitCursor())
{
}

scoped (var waitCursor = new WaitCursor())
{
}

for (var waitCursor = new WaitCursor())
{
}

instrument (var waitCursor = new WaitCursor())
{
}

[HaloFour]

@scottdorman

My preference is using as I don't think that the verb implies anything about intent and it already effectively behaves in this manner. I'd be open to expanding using to fit more unit of work scenarios including informing the resource as to result (success/fail) of the embedded statement, either through some interface or via convention.

[msfcolombo]

@scottdorman

it also gives the implication that it's a loop, which it isn't.

I think you are being over ideological. To a programmer, for and while keywords denotes possible repetition, not certain repetition. In fact, many retry logic today uses for or while (I prefer for because while is an English term closer to a loop). Stronger: the English word for is also close to "at discretion of", which is exactly what we want. The ability to capture exceptions and issue retries is key for this feature, as perceived in many real world scenarios.

[scottdorman]

@msfcolombo

The ability to capture exceptions and issue retries is key for this feature, as perceived in many real world scenarios.

Yes, that's useful but only in a narrowly defined use case, specifically that of instrumenting the code. A general purpose "scoped unit of work" block wouldn't provide any explicit mechanism to capture exceptions, handle retries, etc. It's much more equivalent to a using statement and would expand to very similar code (probably identical except for the code in the finally block, which would cast to some interface other than IDisposable and call the appropriate other method.)

@HaloFour I'm good with expanding using as well. It's one of the keywords that makes the most contextual sense to me. I'm concerned about baking in some mechanism to inform the calling code of success/failure though as it then starts to deviate too much from how using works.

[scottdorman]

@HaloFour The other difference is that I can currently write code like this:

using (new WaitCursor())
{
}

If we introduced a new keyword, it could require that obj-expression is an assignment (and be of a specific interface), such that we could then write:

using (var wc = new WaitCursor())
{
   if (!wc.Success)
   {
      // handle the failure condition
   }
}

[msfcolombo]

@scottdorman

A general purpose "scoped unit of work" block wouldn't provide any explicit mechanism to capture exceptions, handle retries, etc.

This is what using provides and is not enough. We would still have to use lambdas for instrumentation, with all issues explained in this proposal (returning flags, multiple versions, poorly readable stacktraces, etc).

If you want to slightly improve using, I suggest #93. Beware it won't reduce instrumentation costs by any means.

[scottdorman]

@msfcolombo

This is what using provides and is not enough.

Correct, which is why I think having a different mechanism to do this would, ultimately, be better. That way, using and this aren't considered equivalent, since they aren't. It really seems like there are two things that need to be addressed:

1. A general purpose "scoped unit of work" block, similar in nature to using, that doesn't provide any explicit mechanism to capture exceptions, handle retries, etc. but that has no relationship to using IDisposable as it's constrained type.
2. A more specific "transactional/instrumented unit of work" block (this proposal) that does provide some way for the code inside the block to be notified (or to notify the constrained type) of exceptions, potentially handle retries, etc.

This proposal, as it stands, really sounds like wanting language support for the old patterns and practices transient fault handling block. I think (it's been a while since I've looked at it) that code handled everything in this proposal, although it used lambdas rather than actual language syntax.

[pharring]

Just want to suggest an improvement to the compiler-generated code to use an exception filter for handling the exception case (see below).

var _temp = object-Expr;
for (var _i = 0; ; ++_i) {
    _temp.NotifyAttempt(_i);
    try
    {
        // action code
    }
    catch (Exception ex) when (!_temp.NotifyException(_i, ex))
    {
        continue; // Instrument wants a retry.
    }
    _temp.NotifySuccess(_i);
    break;
}

[scottdorman]

As long as using the exception filter doesn't introduce more overhead than without it. What does the exception filter expand to? I couldn't find the proposal.

[tannergooding]

@ManishJayaswal, this has some overlap with both generators and dynamic analyzers.

[pharring]

Exception filters are a CLR feature exposed by C#6. There's no expansion. The key thing is it avoids if (expr) throw;. Probably less relevant (but still an observable difference), NotifyException will be called before any finally blocks inside "action code".

[scottdorman]

@pharring Thanks for that. I had forgotten that they were already a CLR feature just being exposed by language syntax. I think that's a worthwhile syntax change to make and the difference you mentioned about NotifyException being called before any finally blocks is probably a desirable change as well.

[Thaina]

I would support for because it will not add keyword. While contextual it would separate from for(;;)

[lachbaer]

The using expression could be slightly extended to support more implementations.

using (var foo = new Foo() in IDisposable) { ... }    // the "standard"
using (var foo = new Foo() in IInstrumentation) { ... }  

to be read as "... in IDisposable context".

On step further this could allow for custom implementations.

internal using interface IMyDisposable {
    void Dispose();
    void using(IMyDisposable subject) {
        try {
            using;
        }
        finally { subject.Dispose(); }
    }
}

It's a bit much using 😉 using interface defines the wrapper for the usercode and it's interface members. void using(IUsingInterface subject) { ... } (appearance of method declaration) defines the actual wrapper and using; is the (one time only!) statement where the usercode is inserted. The whole wrapper source code is effectively copied in the place of the custom using statement.

There can be more using(...) methods in the interface that differentiate by full signature (async void using(), CReturn using(), etc.) The most specific should be used. This idea is a compile time feature only, because the compiler must have access to the direct wrapper source.

[dmitriyse]

Another one attempt to solve add general instrumentation: #3025