-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Limiter extension interface for memorylimiterextension draft #12601
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Storage | ||
|
|
||
| **Status: under development** | ||
|
|
||
| A limit extension supports flexible methods for admission and rate | ||
| control. Other components, typically receivers, can request a limit | ||
| client from the limit extension and use it to admit requests. | ||
|
|
||
| This interface is byte-weight oriented, enabling limiting for | ||
| individual payloads within a stream-oriented RPC. For request-level | ||
| limiting, consider using the Auth extension instead. | ||
|
|
||
| The `limit.Extension` interface extends `component.Extension` by | ||
| adding the following method: | ||
|
|
||
| ``` | ||
| GetClient(context.Context, component.Kind, component.ID, string) (Client, error) | ||
| ``` | ||
|
|
||
| After the context argument are two component-level identifiers and a | ||
| string, which is the name of the extension. Typically, components | ||
| will support a list of named limiters to apply in sequence, e.g., | ||
|
|
||
| ``` | ||
| receivers: | ||
| someprotocol: | ||
| limiters: | ||
| - rate | ||
| - memory | ||
| ``` | ||
|
|
||
| The `limit.Client` interface contains the following method: | ||
| ``` | ||
| Acquire(ctx context.Context, weight uint64) (ReleaseFunc, error) | ||
| ``` | ||
|
|
||
| The weight parameter specifies how large of an request to consider in | ||
| the limiter, which should estimate the amount of real memory used by | ||
| to represent the data after it is uncompressed. | ||
|
|
||
| The result is either a non-nil release function and nil error, or a | ||
| nil release function and non-nil error. The limiter may block or fail | ||
| fast, at its discretion. When a non-nil release function is returned, | ||
| the component is responsible for calling the release function after | ||
| the memory is no longer used. | ||
|
|
||
| Note: It is the responsibility of each component to `Close` a storage | ||
| client that it has requested. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Package limit implements an extension that can limit requests by | ||
| // blocking or failing them through calls to acquire and release. | ||
| package limit // import "go.opentelemetry.io/collector/extension/xextension/limit" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package limit // import "go.opentelemetry.io/collector/extension/xextension/limit" | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "go.opentelemetry.io/collector/component" | ||
| "go.opentelemetry.io/collector/extension" | ||
| ) | ||
|
|
||
| // Extension is the interface that storage extensions must implement | ||
| type Extension interface { | ||
| extension.Extension | ||
|
|
||
| // GetClient will create a client for use by the specified | ||
| // component. The component can use the client to limit | ||
| // admission to the pipeline. | ||
| GetClient(ctx context.Context, kind component.Kind, id component.ID) (Client, error) | ||
| } | ||
|
Comment on lines
+14
to
+21
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some questions/asks here:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes more sense to me, without further context, to use the approach on my PR: if multiple instances of the client are required then the user can define multiple extensions (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't mean to be inconsistent with #12574, sorry. That approach looks fine to me. As noted, the "extra" interface appears valuable to me in the sense that static context (e.g., component metadata) can be used to specialize the limiting conditions at start time similar to See I opened several drafts--the copy you are commenting on, here, is an older iteration of mine, where I tried to follow the |
||
|
|
||
| // Client implements a limiter for byte-weighted request admission. | ||
| type Client interface { | ||
| // Acquire asks the controller to admit the caller. | ||
| // | ||
| // The weight parameter specifies how large of an request to | ||
| // consider in the limiter. This should be a measure of the | ||
| // size of the request after it is uncompressed. One way to | ||
| // compute the weight of a request is byte its OTLP encoding | ||
| // size using the appropriate pdata `Sizer` type. However, | ||
| // components are encouraged to use an approximation, not | ||
| // necessarily based on the OTLP encoding size, if there is an | ||
| // efficient substitute. | ||
| // | ||
| // The goal is for the estimated weight to approximate the | ||
| // amount of real memory that will be occupied while the | ||
| // request is in flight. If the component uses a protocol | ||
| // with less repetition than OTLP, it is possible for the | ||
| // Sizer to overestimate the amount of real memory used, | ||
| // because Golang's immutable string value allows | ||
| // it. Therefore, components should prefer an inexpensive, | ||
| // approximate method to determine weight in bytes. | ||
| // | ||
| // The limiter is permitted to block the request. After making | ||
| // its decision, the return value is exclusively a function | ||
| // value or an error. Admit will return when one of the | ||
| // following events occurs: | ||
| // | ||
| // (1) admission is allowed, or | ||
| // (2) the provided ctx becomes canceled, or | ||
| // (3) the limiter determines that the request should fail. | ||
|
Comment on lines
+51
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify, in this case returned err is not nil. |
||
| // | ||
| // In case (1), the return value will be a non-nil | ||
| // ReleaseFunc. The caller must invoke it after it is finished | ||
| // with the resource being guarded by the admission | ||
| // controller. | ||
| // | ||
| // In case (2), the return value should be similar to: | ||
| // - gRPC: Cancelled or DeadlineExceeded error | ||
| // - HTTP: Status 408. | ||
| // | ||
| // In case (3), the return value should be similar to: | ||
| // - gRPC: ResourceExhausted error | ||
| // - HTTP: Status 429. | ||
| Acquire(ctx context.Context, weight uint64) (ReleaseFunc, error) | ||
| } | ||
|
|
||
| // ReleaseFunc is returned by Acquire when the Acquire() was admitted. | ||
| type ReleaseFunc func() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package limit | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestNopLimiter(t *testing.T) { | ||
| ctx := context.Background() | ||
| nop := NewNopClient() | ||
| rel, err := nop.Acquire(ctx, 1000) | ||
| require.NoError(t, err) | ||
| rel() | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| type: xextension | ||
| github_project: open-telemetry/opentelemetry-collector | ||
|
|
||
| status: | ||
| class: extension | ||
| codeowners: | ||
| active: | ||
| - jmacd | ||
| stability: | ||
| development: [traces, metrcs, logs, profiles] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package limit // import "go.opentelemetry.io/collector/extension/xextension/limit" | ||
|
|
||
| import "context" | ||
|
|
||
| type nopClient struct{} | ||
|
|
||
| var nopClientInstance Client = &nopClient{} | ||
|
|
||
| // NewNopClient returns a nop client | ||
| func NewNopClient() Client { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually have this in a |
||
| return nopClientInstance | ||
| } | ||
|
|
||
| // Acquire implements Client. | ||
| func (nopClient) Acquire(_ context.Context, _ uint64) (ReleaseFunc, error) { | ||
| return func() {}, nil | ||
| } | ||
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.
Based on auth case, I think this should be consistent and be named
extensionlimit