Skip to content
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

feat: trigger rate limit. Closes: #1087 #1318

Merged
merged 3 commits into from
Aug 23, 2021
Merged

Conversation

whynowy
Copy link
Member

@whynowy whynowy commented Aug 21, 2021

Signed-off-by: Derek Wang [email protected]

Trigger supports rate limit:

spec:
  triggers:
    - rateLimit:
        # Second, Minute or Hour, defaults to Second
        unit: Second
        # Requests per unit
        requestsPerUnit: 20

Closes: #1087

Checklist:

Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
func (sensorCtx *SensorContext) triggerOne(ctx context.Context, sensor *v1alpha1.Sensor, trigger v1alpha1.Trigger, eventsMapping map[string]*v1alpha1.Event, depNames, eventIDs []string, log *zap.SugaredLogger) error {
defer func(start time.Time) {
sensorCtx.metrics.ActionDuration(sensor.Name, trigger.Template.Name, float64(time.Since(start)/time.Millisecond))
}(time.Now())

defer sensorCtx.metrics.ActionDuration(sensor.Name, trigger.Template.Name, float64(time.Since(time.Now())/time.Millisecond))
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicate, incorrect metrics.


type RateLimit struct {
// Defaults to Second
Unit RateLimiteUnit `json:"unit,omitempty" protobuf:"bytes,1,opt,name=unit"`
Copy link
Member

Choose a reason for hiding this comment

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

can't you use Duration here?

Copy link
Member Author

@whynowy whynowy Aug 23, 2021

Choose a reason for hiding this comment

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

Using Duration like below seems to be weird:

spec:
  triggers:
    - rateLimit:
        duration: 5s
        requests: 20

Referencing to Istio and ambassador, both of them use unit:

Istio:

apiVersion: v1
kind: ConfigMap
metadata:
  name: ratelimit-config
data:
  config.yaml: |
    domain: productpage-ratelimit
    descriptors:
      - key: PATH
        value: "/productpage"
        rate_limit:
          unit: minute
          requests_per_unit: 1
      - key: PATH
        rate_limit:
          unit: minute
          requests_per_unit: 100

Ambassador:

apiVersion: getambassador.io/v2
kind: RateLimit
metadata:
  name: backend-rate-limit
spec:
  domain: ambassador
  limits:
   - pattern: [{generic_key: backend}]
     rate: 3
     unit: minute

@whynowy whynowy merged commit 4ba6543 into argoproj:master Aug 23, 2021
@whynowy whynowy deleted the ratelimit branch August 23, 2021 16:00
juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
* feat: trigger rate limit. Closes: argoproj#1087

Signed-off-by: Derek Wang <[email protected]>

* ok

Signed-off-by: Derek Wang <[email protected]>

* lint

Signed-off-by: Derek Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trigger Rate limit
2 participants