-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Feature/rate limiter http #2055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Feature/rate limiter http #2055
Conversation
* add traceID and logs in cron * resolve linter * Added 404 response code in case of no rows retrieved from QueryRowContext * Added support for CQL * Added support for Mongo * Fixed formatting * refactor file cron.go and add file cron_scheduler.go * Updated .gitignore file to exclude go.work and go.work.sum files * Updated .gitignore file to exclude go.work and go.work.sum files * fix: allow sending multiple query parameters with the same name * fixed docker container naming docs/quick-start/connecting-mysql/page.md : fixed docker container naming for mysql * add mutex protection for float64Gauge and concurrent test * Revert "add mutex protection for float64Gauge and concurrent test" This reverts commit 24f8859. * docs: remove redundant "code" from CONTRIBUTING * build(deps): bump golang.org/x/sync from 0.14.0 to 0.15.0 Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.14.0 to 0.15.0. - [Commits](golang/sync@v0.14.0...v0.15.0) --- updated-dependencies: - dependency-name: golang.org/x/sync dependency-version: 0.15.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump github.com/XSAM/otelsql from 0.38.0 to 0.39.0 Bumps [github.com/XSAM/otelsql](https://github.com/XSAM/otelsql) from 0.38.0 to 0.39.0. - [Release notes](https://github.com/XSAM/otelsql/releases) - [Changelog](https://github.com/XSAM/otelsql/blob/main/CHANGELOG.md) - [Commits](XSAM/otelsql@v0.38.0...v0.39.0) --- updated-dependencies: - dependency-name: github.com/XSAM/otelsql dependency-version: 0.39.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump github.com/redis/go-redis/extra/redisotel/v9 Bumps [github.com/redis/go-redis/extra/redisotel/v9](https://github.com/redis/go-redis) from 9.9.0 to 9.10.0. - [Release notes](https://github.com/redis/go-redis/releases) - [Changelog](https://github.com/redis/go-redis/blob/master/CHANGELOG.md) - [Commits](redis/go-redis@v9.9.0...v9.10.0) --- updated-dependencies: - dependency-name: github.com/redis/go-redis/extra/redisotel/v9 dependency-version: 9.10.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump ls-lint/action in the actions group Bumps the actions group with 1 update: [ls-lint/action](https://github.com/ls-lint/action). Updates `ls-lint/action` from 2.3.0 to 2.3.1 - [Commits](ls-lint/action@v2.3.0...v2.3.1) --- updated-dependencies: - dependency-name: ls-lint/action dependency-version: 2.3.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> * Updated implementation as per feedback from gofr team * Removed no longer required change * Re-added incorrectly removed test * Fixed import sequence * Revert "Updated .gitignore file to exclude go.work and go.work.sum files" This reverts commit 64d3fb7. * added StatusCode method and tests * Reverted .gitignore changes * build(deps): bump google.golang.org/grpc in /pkg/gofr/datasource/dgraph Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.72.2 to 1.73.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.72.2...v1.73.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.73.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump google.golang.org/api from 0.235.0 to 0.236.0 Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.235.0 to 0.236.0. - [Release notes](https://github.com/googleapis/google-api-go-client/releases) - [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md) - [Commits](googleapis/google-api-go-client@v0.235.0...v0.236.0) --- updated-dependencies: - dependency-name: google.golang.org/api dependency-version: 0.236.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump github.com/alicebob/miniredis/v2 from 2.34.0 to 2.35.0 Bumps [github.com/alicebob/miniredis/v2](https://github.com/alicebob/miniredis) from 2.34.0 to 2.35.0. - [Release notes](https://github.com/alicebob/miniredis/releases) - [Changelog](https://github.com/alicebob/miniredis/blob/master/CHANGELOG.md) - [Commits](alicebob/miniredis@v2.34.0...v2.35.0) --- updated-dependencies: - dependency-name: github.com/alicebob/miniredis/v2 dependency-version: 2.35.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump golang.org/x/text from 0.25.0 to 0.26.0 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.25.0 to 0.26.0. - [Release notes](https://github.com/golang/text/releases) - [Commits](golang/text@v0.25.0...v0.26.0) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-version: 0.26.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump google.golang.org/grpc from 1.72.2 to 1.73.0 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.72.2 to 1.73.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.72.2...v1.73.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.73.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump modernc.org/sqlite from 1.37.1 to 1.38.0 Bumps [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) from 1.37.1 to 1.38.0. - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.37.1...v1.38.0) --- updated-dependencies: - dependency-name: modernc.org/sqlite dependency-version: 1.38.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): bump github.com/redis/go-redis/v9 from 9.9.0 to 9.10.0 Bumps [github.com/redis/go-redis/v9](https://github.com/redis/go-redis) from 9.9.0 to 9.10.0. - [Release notes](https://github.com/redis/go-redis/releases) - [Changelog](https://github.com/redis/go-redis/blob/master/CHANGELOG.md) - [Commits](redis/go-redis@v9.9.0...v9.10.0) --- updated-dependencies: - dependency-name: github.com/redis/go-redis/v9 dependency-version: 9.10.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * update redisMock * fixed otel error in case of supabase dialect * fixing code climate issue * update release version to v1.41.0 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Akshat Singhal <akshat.singhal@zopsmart.com> Co-authored-by: Akshat Singhal <65562230+akshat-kumar-singhal@users.noreply.github.com> Co-authored-by: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Co-authored-by: Saurav Dharwadkar <dharwadkarsaurav@gmail.com> Co-authored-by: Aryan Mehrotra <aryan.mehrotra@gofr.dev> Co-authored-by: olxandr <31043927+olxandr@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Vikash Kumar <vikash.iitb@gmail.com> Co-authored-by: Divya <divi229moc@gmail.com> Co-authored-by: Divya Darshana <98943137+coolwednesday@users.noreply.github.com>
Release/v1.42.0
Release/v1.42.1
Release/v1.42.2
Release/v1.42.3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There are no tests added for the new code.
- Documentation is not provided.
- Removes the current godoc comments at many places.
- Version changes are unncessary and should be removed.
- The way we are injecting Rate Limiter doesn't comply with what we are currently following for other HTTP options. Let's try to stick to the previous way only.
@@ -1,3 +1,3 @@ | |||
package version | |||
|
|||
const Framework = "dev" | |||
const Framework = "v1.42.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not required. Please revert it.
@@ -24,50 +24,35 @@ type httpService struct { | |||
} | |||
|
|||
type HTTP interface { | |||
// HTTP is embedded as HTTP would be able to access it's clients method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but any particular reason for removing these godoc comments??
for _, o := range options { | ||
if rlOption, ok := o.(*WithRateLimiter); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need to add this option manually instead of doing:
svc = o.AddOption(svc)
i think the implementation is wrong. For the other configs/features we are not additionally providing the logger metrics etc then why for rate limiter??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I’ll review the implementation again and make the necessary updates. Thanks for the feedback!
type Options interface { | ||
AddOption(h HTTP) HTTP | ||
AddOption(HTTP) HTTP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
I think sticking to :
a.AddHTTPService("cat-facts", "https://catfact.ninja",
service.NewAPIKeyConfig("some-random-key"),
service.NewBasicAuthConfig("username", "password"),
&service.CircuitBreakerConfig{
Threshold: 4,
Interval: 1 * time.Second,
},
&service.DefaultHeaders{Headers: map[string]string{"key": "value"}},
&service.HealthConfig{
HealthEndpoint: "breeds",
},
service.NewOAuthConfig("clientID", "clientSecret",
"https://tokenurl.com", nil, nil, 0),
&service.RetryConfig{
MaxRetries: 5
},
)
Is better?
Hey! @archit-2003. Thankyou for your contribution. Currently your PR has the below linter issues. Please resolve all the linters to completely pass the workflow. ![]() |
This PR introduces a built-in rate limiting capability for interservice HTTP calls within Gofr.
Tasks Completed:
RateLimiterConfig
struct: This new struct allows configuration of rate limits viaRequestsPerSecond
andBurst
parameters.AddHTTPService
: Aservice.WithRateLimiter
option is now available to apply rate limiting when defining HTTP services.WithRateLimiter
gets its own independent rate limiter instance, ensuring isolation.Logger
interface.OpenTelemetry
event is added to the active span for rate-limited requests.Prometheus
counter (app_http_service_rate_limit_exceeded_total
) is incremented upon rate limit triggers.Technical Details / Files Changed:
pkg/gofr/service/ratelimiter.go
: New file definingRateLimiterConfig
and the core rate limiting logic.pkg/gofr/service/options.go
: Modified to include theWithRateLimiter
option.pkg/gofr/service/new.go
: Modified to correctly initialize and inject dependencies into the rate limiter.examples/using-http-service/main.go
): Modified for local testing demonstration.