-
Notifications
You must be signed in to change notification settings - Fork 180
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
[Access] Response limit tracker #6814
base: master
Are you sure you want to change the base?
[Access] Response limit tracker #6814
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6814 /- ##
==========================================
Coverage 41.18% 41.19% 0.01%
==========================================
Files 2109 2109
Lines 185660 185664 4
==========================================
Hits 76460 76489 29
Misses 102788 102766 -22
Partials 6412 6409 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@Guitarheroua please address merge conflicts |
…isarchuk/6640-responce-limit-tracker
s.T().Run("Enforces response rate limit", func(t *testing.T) { | ||
totalMessages := 5 // Number of messages to simulate. | ||
|
||
// Step 1: Create a mock WebSocket connection. | ||
conn := connmock.NewWebsocketConnection(t) | ||
conn.On("SetWriteDeadline", mock.Anything).Return(nil).Times(totalMessages) | ||
|
||
// Step 2: Configure the WebSocket controller with a rate limit. | ||
config := NewDefaultWebsocketConfig() | ||
config.MaxResponsesPerSecond = 2 // 2 messages per second. | ||
controller := NewWebSocketController(s.logger, config, conn, nil) | ||
|
||
// Step 3: Simulate sending messages to the controller's `multiplexedStream`. | ||
go func() { | ||
for i := 0; i < totalMessages; i { | ||
controller.multiplexedStream <- map[string]interface{}{ | ||
"message": i, | ||
} | ||
} | ||
close(controller.multiplexedStream) | ||
}() | ||
|
||
// Step 4: Collect timestamps of message writes for verification. | ||
var timestamps []time.Time | ||
conn.On("WriteJSON", mock.Anything).Run(func(args mock.Arguments) { | ||
timestamps = append(timestamps, time.Now()) | ||
}).Return(nil).Times(totalMessages) | ||
|
||
// Invoke the `writeMessages` method to process the stream. | ||
_ = controller.writeMessages(context.Background()) | ||
|
||
// Step 5: Verify that all messages are processed. | ||
require.Len(t, timestamps, totalMessages, "All messages should be processed") | ||
|
||
// Calculate the expected delay between messages based on the rate limit. | ||
expectedDelay := time.Second / time.Duration(config.MaxResponsesPerSecond) | ||
const tolerance = 5 * time.Millisecond // Allow up to 5ms deviation. | ||
|
||
// Step 6: Assert that the delays respect the rate limit with tolerance. | ||
for i := 1; i < len(timestamps); i { | ||
delay := timestamps[i].Sub(timestamps[i-1]) | ||
assert.GreaterOrEqual(t, delay, expectedDelay-tolerance, "Messages should respect the minimum rate limit") | ||
assert.LessOrEqual(t, delay, expectedDelay tolerance, "Messages should respect the maximum rate limit") | ||
} | ||
}) |
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.
s.T().Run("Enforces response rate limit", func(t *testing.T) { | |
totalMessages := 5 // Number of messages to simulate. | |
// Step 1: Create a mock WebSocket connection. | |
conn := connmock.NewWebsocketConnection(t) | |
conn.On("SetWriteDeadline", mock.Anything).Return(nil).Times(totalMessages) | |
// Step 2: Configure the WebSocket controller with a rate limit. | |
config := NewDefaultWebsocketConfig() | |
config.MaxResponsesPerSecond = 2 // 2 messages per second. | |
controller := NewWebSocketController(s.logger, config, conn, nil) | |
// Step 3: Simulate sending messages to the controller's `multiplexedStream`. | |
go func() { | |
for i := 0; i < totalMessages; i { | |
controller.multiplexedStream <- map[string]interface{}{ | |
"message": i, | |
} | |
} | |
close(controller.multiplexedStream) | |
}() | |
// Step 4: Collect timestamps of message writes for verification. | |
var timestamps []time.Time | |
conn.On("WriteJSON", mock.Anything).Run(func(args mock.Arguments) { | |
timestamps = append(timestamps, time.Now()) | |
}).Return(nil).Times(totalMessages) | |
// Invoke the `writeMessages` method to process the stream. | |
_ = controller.writeMessages(context.Background()) | |
// Step 5: Verify that all messages are processed. | |
require.Len(t, timestamps, totalMessages, "All messages should be processed") | |
// Calculate the expected delay between messages based on the rate limit. | |
expectedDelay := time.Second / time.Duration(config.MaxResponsesPerSecond) | |
const tolerance = 5 * time.Millisecond // Allow up to 5ms deviation. | |
// Step 6: Assert that the delays respect the rate limit with tolerance. | |
for i := 1; i < len(timestamps); i { | |
delay := timestamps[i].Sub(timestamps[i-1]) | |
assert.GreaterOrEqual(t, delay, expectedDelay-tolerance, "Messages should respect the minimum rate limit") | |
assert.LessOrEqual(t, delay, expectedDelay tolerance, "Messages should respect the maximum rate limit") | |
} | |
}) | |
totalMessages := 5 // Number of messages to simulate. | |
// Step 1: Create a mock WebSocket connection. | |
conn := connmock.NewWebsocketConnection(t) | |
conn.On("SetWriteDeadline", mock.Anything).Return(nil).Times(totalMessages) | |
// Step 2: Configure the WebSocket controller with a rate limit. | |
config := NewDefaultWebsocketConfig() | |
config.MaxResponsesPerSecond = 2 // 2 messages per second. | |
controller := NewWebSocketController(s.logger, config, conn, nil) | |
// Step 3: Simulate sending messages to the controller's `multiplexedStream`. | |
go func() { | |
for i := 0; i < totalMessages; i { | |
controller.multiplexedStream <- map[string]interface{}{ | |
"message": i, | |
} | |
} | |
close(controller.multiplexedStream) | |
}() | |
// Step 4: Collect timestamps of message writes for verification. | |
var timestamps []time.Time | |
conn.On("WriteJSON", mock.Anything).Run(func(args mock.Arguments) { | |
timestamps = append(timestamps, time.Now()) | |
}).Return(nil).Times(totalMessages) | |
// Invoke the `writeMessages` method to process the stream. | |
_ = controller.writeMessages(context.Background()) | |
// Step 5: Verify that all messages are processed. | |
require.Len(t, timestamps, totalMessages, "All messages should be processed") | |
// Calculate the expected delay between messages based on the rate limit. | |
expectedDelay := time.Second / time.Duration(config.MaxResponsesPerSecond) | |
const tolerance = 5 * time.Millisecond // Allow up to 5ms deviation. | |
// Step 6: Assert that the delays respect the rate limit with tolerance. | |
for i := 1; i < len(timestamps); i { | |
delay := timestamps[i].Sub(timestamps[i-1]) | |
assert.GreaterOrEqual(t, delay, expectedDelay-tolerance, "Messages should respect the minimum rate limit") | |
assert.LessOrEqual(t, delay, expectedDelay tolerance, "Messages should respect the maximum rate limit") | |
} |
// Step 4: Collect timestamps of message writes for verification. | ||
var timestamps []time.Time | ||
conn.On("WriteJSON", mock.Anything).Run(func(args mock.Arguments) { | ||
timestamps = append(timestamps, time.Now()) |
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.
Lets also check that we indeed receive expected messages in expected order at this place.
// Step 6: Assert that the delays respect the rate limit with tolerance. | ||
for i := 1; i < len(timestamps); i { | ||
delay := timestamps[i].Sub(timestamps[i-1]) | ||
assert.GreaterOrEqual(t, delay, expectedDelay-tolerance, "Messages should respect the minimum rate limit") |
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.
You could possibly simplify next statement using: https://github.com/stretchr/testify/blob/master/require/require.go#L869
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.
Looks good. Left a few comments tests related.
Closes: #6640
NOTE: this PR should be merged after #6798
Implements rate limiter for responses, that controller returns to client.