chore: Warn about unsupported async return types

[chynesNR]

As per our documentation, we only support instrumenting async methods that return a Task. If we find another return type, log a Warning. This should make it easier to track down problems caused by using e.g. ValueTask. The logging will only occur once since the Wrapper is cached.

Sample line:

2024-06-18 20:12:10,856 NewRelic   WARN: [pid: 37628, tid: 24] Instrumenting async methods that return a type other than Task or Task<> is not supported and may result in inconsistent data. 'MyAsyncMethod' has a return type of 'ValueTask'.

[tippmar-nr]

Nice solution!

[nrcventura]

This solution seems like a good one. Chris and I talked through some things that I think makes sense to call out more broadly.

• This approach will warn when trying to instrument async void, async ValueTask, and async ValueTask<T> return types.
• This approach will cover new instrumentation, custom instrumentation, and attribute-based instrumentation because all instrumentation is routed through that class.
• The reflection is necessary because we do not have access to the return type of the method until the method completes.

Chris is going to take a look at an alternative approach in the Delegates class where we already have code that checks if the return type is a Task, to see which implementation may be better.

[chynesNR]

This solution seems like a good one. Chris and I talked through some things that I think makes sense to call out more broadly.

• This approach will warn when trying to instrument async void, async ValueTask, and async ValueTask<T> return types.
• This approach will cover new instrumentation, custom instrumentation, and attribute-based instrumentation because all instrumentation is routed through that class.
• The reflection is necessary because we do not have access to the return type of the method until the method completes.

Chris is going to take a look at an alternative approach in the Delegates class where we already have code that checks if the return type is a Task, to see which implementation may be better.

After talking it over with Chris and looking at the code together, we think the current solution is probably the less-impactful one. We do have access to the return object (and therefore its type) in the afterWrappedMethodDelegate created by Delegates.GetDelegateFor<Task>, but while this wouldn't incur the reflection cost, it would run after every invocation of the method. That feels like it will be more expensive in the long run.