Skip to content

Add rate limiter context HttpContext feature#45652

Closed
MadL1me wants to merge 1 commit intodotnet:mainfrom
MadL1me:MadL1me/rate-limiter-middleware/context-feature
Closed

Add rate limiter context HttpContext feature#45652
MadL1me wants to merge 1 commit intodotnet:mainfrom
MadL1me:MadL1me/rate-limiter-middleware/context-feature

Conversation

@MadL1me
Copy link
Copy Markdown
Contributor

@MadL1me MadL1me commented Dec 17, 2022

Add rate limiter context HttpContext feature

Added HttpContext feature which has access to Global and Endpoint limiters with availability to fetch rate limiters statistics and get Lease metadata. Based on #44140

Description

  • Added new HttpContext feature, which allows to get access rate limiters and its statistics in custom middlewares or controllers
  • Added unit tests for changes
  • Still not sure about the API, so I didn't add any XML documentation yet

Motivation

In #44140 issue, one of The dream scenario for a better ASP.NET Core rate limiter middleware as @maartenba says, would be able Have a feature on the current HttpContext that gives access to the current rate limit context, so these details can also be returned on successful requests, or added to telemetry.. This PR addresses this concern.

I thought it would be much better to make multiple atomic, but feature-complete PR's to one issue, so it would be faster and easier to review

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Dec 17, 2022
@ghost
Copy link
Copy Markdown

ghost commented Dec 17, 2022

Thanks for your PR, @MadL1me. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.


namespace Microsoft.AspNetCore.RateLimiting.Features;

public interface IRateLimiterContextFeature
Copy link
Copy Markdown
Contributor Author

@MadL1me MadL1me Dec 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: there is no xml docs, cause i'm not sure about the API

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New APIs need to be approved. This needs an API proposal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new APIs also need to be added to https://github.com/dotnet/aspnetcore/blob/main/src/Middleware/RateLimiting/src/PublicAPI.Unshipped.txt. There's a suggestion hint in VS to do this for you.

@MadL1me
Copy link
Copy Markdown
Contributor Author

MadL1me commented Dec 17, 2022

I think this PR is in @wtgodbe area

@MadL1me
Copy link
Copy Markdown
Contributor Author

MadL1me commented Jan 11, 2023

Decided to make proposed API (#45658) with 2 separate PR's for each feature, also, this PR is too differ from proposed API, so I'm Closing this PR. All work is moved to #46028

@MadL1me MadL1me closed this Jan 11, 2023
@ghost
Copy link
Copy Markdown

ghost commented Jan 11, 2023

Hi @MadL1me. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants