[API Proposal]: InstrumentRecorder - notification of new measurements

[JamesNK]

Background and motivation

InstrumentRecorder is an API new in .NET 8 to make testing metrics easier:

var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 

var response = await httpClient.GetAsync("/");

var measurements = instrumentRecorder.GetMeasurements();
// Assert contents of measurements

However, there are some cases where there are race conditions because measurements happen on a different thread. For example, with the request-duration counter above, the server can return the failing response to the client before the metric is recorded. GetMeasurements may or may not have an item. The test is flaky.

It would be useful if someone using InstrumentRecorder could get notifications of new measurements.

API Proposal

namespace System.Diaganostics.Metrics;

public class InstumentRecorder<T>
{
    // Wait for any number of measurements.
    public Task<IEnumerable<Measurement<T>> WaitForMeasurementsAsync(bool clear = false)
        => WaitForMeasurementsAsync(minimumCount: 1, clear);

    // Wait for measurements greater than count.
    public Task<IEnumerable<Measurement<T>> WaitForMeasurementsAsync(int minimumCount, bool clear = false);
}

WaitForMeasurementsAsync returns when one or more measurements are available. If there are already measurements, the method returns immediately.

Random thoughts:

• It might be worth having a count overload on the method, allowing someone to wait for an expected number of measurements, e.g. WaitForMeasurementsAsync(minimumCount: 5). Edit: Added above.
• Are parallel calls to WaitForMeasurementsAsync allowed? What happens if there are parallel calls to WaitForMeasurementsAsync, and they have clear: true?
• If someone wants to wait multiple times, they must clear the measurements (or increase the wait count). The example below shows this.

API Usage

var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 

var response1 = await httpClient.GetAsync("/");
var measurements1 = await instrumentRecorder.WaitForMeasurementsAsync(clear: true);
// Assert contents of measurements

var response2 = await httpClient.GetAsync("/");
var measurements2 = await instrumentRecorder.WaitForMeasurementsAsync(clear: true);
// Assert contents of measurements

Alternative Designs

An API to register a callback that executes for each measurement:

var tcs = new TaskCompletionSource();
var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 
instrumentRecorder.Register(m => 
{ 
    tcs.TrySetResult(); 
});

// Do stuff

await tcs.Task;
var measurements = await instrumentRecorder.GetMeasurements();
// Assert contents of measurements

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

InstrumentRecorder is an API new in .NET 8 to make testing metrics easier:

var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 

var response = await httpClient.GetAsync("/");

var measurements = instrumentRecorder.GetMeasurements();
// Assert contents of measurements

However, there are some cases where there are race conditions because measurements happen on a different thread. For example, in the counter above, the server can return the failing response to the client before the metric is recorded. GetMeasurements may or may not have an item.

It would be useful if someone using InstrumentRecorder could get notifications of new measurements.

API Proposal

namespace System.Diaganostics.Metrics;

public class InstumentRecorder<T>
{
    public Task<IEnumerable<Measurement<T>> WaitForMeasurementsAsync(bool clear = false);
}

WaitForMeasurementsAsync returns when one or more measurements are available. If there are already measurements, the method returns immediately.

If someone wants to wait again then they would need to clear the measurements when they wait/get measurements.

API Usage

var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 

var response = await httpClient.GetAsync("/");

var measurements = await instrumentRecorder.WaitForMeasurementsAsync();
// Assert contents of measurements

Alternative Designs

An API to register a callback that executes for each measurement:

var tcs = new TaskCompletionSource();
var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 
instrumentRecorder.Register(m => 
{ 
    tcs.SetResult(); 
});

// Do stuff

await tcs.Task;
var measurements = await instrumentRecorder.GetMeasurements();
// Assert contents of measurements

Risks

No response

Author:	JamesNK
Assignees:	-
Labels:	api-suggestion, area-System.Net, untriaged
Milestone:	-

[JamesNK]

cc @tarekgh @noahfalk

[noahfalk]

I think we'd want some timeout or cancellation on the wait, otherwise tests that fail to generate the expected measurement might run a very long time. Other than that it seems fine to me.

[tarekgh]

The alternative proposal utilizing the Register feature appears superior since it offers greater flexibility, not limited solely to the number of measurements passed to the Wait methods.

var instrumentRecorder = new InstrumentRecorder<double>(meterRegistry, "Microsoft.AspNetCore.Hosting", "request-duration"); 

var tcs = new TaskCompletionSource<bool>();

instrumentRecorder.Register(recorder, measurement, state => { 
	// can check the current measurments or even get all measurments by calling recorder.GetMeasurements()

    // can call this when done getting and checking all needed measurments
	(state as TaskCompletionSource<bool>).TrySetResult(true);
}, tcs);

Assert.Equal(0, Task.WaitAny(tcs.Task, Task.Delay(10000))); // Ensure to check timing out the operation

[JamesNK]

Tarek and I discussed this offline. I'll summarize my thoughts:

• I see InstrumentRecorder is a convenience helper type for recording and testing measurements.
• I see InstrumentRecorder.WaitForMeasurementsAsync is a convenience method for waiting for measurements to be available.
• Registering a callback on the recorder is really flexible. It allows someone to wait for measurements, but it's more code. The code is relatively complex.
• The recorder doesn't need ultimate flexibility. We have MeterListener for that.
• There may be a place for both APIs. Does adding WaitForMeasurementsAsync now prevent a callback from being added in the future?

[tarekgh]

I am thinking in the following shape of the API

public Task<IEnumerable<Measurement<T>> WaitForMeasurementsAsync(
                                            int minimumCount = 1, 
                                            bool clear = false, 
                                            TimeSpan timeout = TimeSpan.InfiniteTimeSpan, 
                                            CancellationToken cancellationToken = default);

We need to think about how this API is going to work with the Observable instruments (that work as pull instruments). There is some suggestion from @noahfalk to add a new API to InstrumentRecorder to force getting the measurement from the observable instrument. #86567 (comment). If we add such API, WaitForMeasurementsAsync can work nicely as it will wait the number of times minimumCount calling the new API. Without the new API, WaitForMeasurementsAsync can be a problematic for the observable instruments.

[tarekgh]

We removed the InstrumentRecorder from the runtime. Users can use MetricCollector