farfetch / loadshedding Goto Github PK

Is your request related to a problem you have?

N/A

Describe the solution you'd like

What about supporting Diagnostics.Metrics instead of prometheus-net? Using Metrics we are able to consume from OpenTelemetry for example

Are you able to help bring it to life and contribute with a Pull Request?

Yes

Additional context

No response

Is your request related to a problem you have?

When a task doesn't go to the queue, the zero waiting time is not reported for this task, which causes the queue time histogram to have periods without data and not represent the completed queue time data.

If we want to obtain the histogram of queue times, it would be possible to exclude bucket zero and thus obtain a new histogram only with buckets > 0.

Additionally, the queue counter is never reported as 0 for an application that has never had an element in the queue, as this metric is only reported in the dequeue event.

Describe the solution you'd like

The queue time of zero should be reported in the histogram in all processed events, as well as the queue counter. So in the first task it would already report that the queue is zero and the waiting time is zero.

Are you able to help bring it to life and contribute with a Pull Request?

Yes

Additional context

I've already done a local POC and I have the code that I can push to the server for comments from maintainers.

Prerequisites

• I have searched issues to ensure it has not already been reported

Description

The http_requests_queue_items_total metric is being used as a counter to report the total task waiting in the queue.

Sometimes after the queue is empty the gauge reports more than 0 items in the queue.

Steps to reproduce

1. Open the Solution
2. Go to AdaptativeConcurrencyLimiterTests test class
3. Change the assertions in AssertMetrics method as described below
4. Run the GetAsync_WithReducedLimitAndQueueSize_SomeRequestsAreRejected test until failure (use the Test Explorer Run Until Failure)
5. After 5 attempts the tests should fail

Assert.Contains("http_requests_concurrency_items_total{method=\"GET\",priority=\"normal\"} 0", content);
Assert.Contains("http_requests_concurrency_limit_total", content);
Assert.Contains("http_requests_task_processing_time_seconds", content);
Assert.Contains("http_requests_queue_items_total{method=\"GET\",priority=\"normal\"} 0", content);
Assert.Contains("http_requests_queue_limit_total", content);
Assert.Contains("http_requests_queue_time_seconds_sum{method=\"GET\",priority=\"normal\"}", content);
Assert.Contains("http_requests_queue_time_seconds_count{method=\"GET\",priority=\"normal\"}", content);
Assert.Contains("http_requests_queue_time_seconds_bucket{method=\"GET\",priority=\"normal\",le=\"0.0005\"}", content);
Assert.Contains("http_requests_rejected_total{method=\"GET\",priority=\"normal\",reason=\"max_queue_items\"}", content);

Expected behavior

After processing all the tasks the metrics http_requests_queue_items_total and http_requests_concurrency_items_total must be 0 for all methods and priorities

Actual behavior

When the test fails, the http_requests_queue_items_total is 1, however it must be 0.

I saw one test failing which method where http_requests_queue_items_total{method=GET} 1 and http_requests_queue_items_total{method=UNKNOWN} -1

LoadShedding version

1.0.0

Prerequisites

• I have searched issues to ensure it has not already been reported

Description

I have a .Net 8 WebAPI that I've been building out and I added LoadShedding to it today.

I've noticed now that any time an exception would be thrown, returning a 500 error to the client it instead returns an empty 200 response instead.

Steps to reproduce

1. Create a basic .Net 8 WebAPI (Controllers) application
2. Create a controller as below.
3. Add load shedding with defaults
4. Run the application and use a tool like Postman to hit the endpoint

[ApiController]
[Route("[controller]")]
public sealed class EchoController : ControllerBase
{
    [HttpGet("test/error")]
    public async Task<IActionResult> TestError(CancellationToken cancellationToken = default)
    {
        throw new Exception("Something went wrong");
    }
}

The same issue happens for Unauthorized responses:

    [HttpGet("test/auth")]
    [Authorize]
    public async Task<IActionResult> TestAuth(CancellationToken cancellationToken = default)
    {
        return Ok("Authorized");
    }

If you hit this endpoint while not authenticated you also get a blank 200 response.

Expected behavior

I'd expect error responses to be preserved so that client applications can respond correctly.

Actual behavior

A blank 200 response is returned for errors.

LoadShedding version

1.0.0

Is your request related to a problem you have?

The feature would introduce the ability to configure specific thresholds for system metrics (e.g., CPU usage, memory usage, request rate), beyond which the middleware would start shedding loads. This approach allows for finer control over when load shedding should occur, ensuring critical resources are preserved for the most important operations and complementing auto-scaling by providing an additional layer of performance management.

Describe the solution you'd like

In a scenario where an ASP.NET Core application experiences sudden spikes in traffic, auto-scaling might not immediately accommodate the increased load, leading to potential performance degradation. By setting a CPU usage threshold at 75%, the load shedding can begin shedding less critical requests, maintaining the responsiveness of the application until auto-scaling can bring additional resources online.

Are you able to help bring it to life and contribute with a Pull Request?

No

Additional context

No response