Outline steps to add "limiter" extension component

[jmacd]

Component(s)

extension/memorylimiter

Describe the issue you're reporting

Following a proposal roughly sketched in #9591 (comment), this is a concrete set of steps to upgrade the Collector's capabilities with regards to rate-limiting and admission-limiting requests.

This proposal has the following major steps:

1. Add extension/xextension/limiter package, where limiter.Extension and limiter.Limiter types are modeled on the x/storage package
2. Add config/configlimiter package, similar to the configauth package in exporting a type for naming extension components, performing a map lookup and type check over host extensions
3. Modify configgrpc adding Limiters []configlimiter.Limitation

• (a) w/ unittests in configgrpc
• (b) end-to-end test in extension/admissionlimiterextension

4. Modify confighttp adding Limiters []configlimiter.Limitation

• (a) w/ unittests in confighttp
• (b) end-to-end test in extension/admissionlimiterextension

5. Update memorylimiterextension to support the new limiter.Extension interface
6. Add admissionlimiterextension supporting the new limiter.Extension interface modeled on the OTel-Arrow admission controller

• (a) Initial skeleton
• (b) Copy implementation from https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/otelarrow/admission2/README.md

7. Remove internal/otelarrow/admission2 from collector-contrib, update otelarrowreceiver to use limiter extensions.

Here are the major steps outlined as rough drafts:

Includes step (1) and (2). These can be easily separated into two PRs, first extension then config.
#12599

Includes (3) w/o tests plus the content above, look mainly at configgrpc.go
#12600

Includes (5) w/o tests plus the first, #12601

Includes (6a) w/o tests plus the first, #12602

I am looking for maintainers and approvers to sign off on the general approach. I would send part (1) as a stand-alone change and proceed in roughly the order presented above.

[mx-psi]

Is there a possibility to design some parts in a more generic way to cover the use cases discussed in #7441?

[bogdandrutu]

If I understand correctly the proposal from @mx-psi. I think we can expand this to something like (where interceptor may be a better name than Limiter) that is generic enough for all our cases:

type HTTPServerInterceptor interface {
  GetHTTPHandler(...) (http.Handler, error) {}
}

type GRPCServerInterceptor interface {
  GetGRPCUnaryServerInterceptor(...) (grpc.UnaryClientInterceptor, error) {}
  GetGRPCStreamServerInterceptor(...) (grpc.StreamClientInterceptor, error) {}
}

@mx-psi: Can the auth be an "Interceptor" as well then?

EDIT: Initially, I showed the example wrong on client side, everything should be about server side.

[mattsains]

@bogdandrutu , I agree that this admission pattern may have opportunity for generalization, but I think the interface you suggested actually specializes the interface to working only on receivers that use http or grpc servers, rather then generalizes it. Other receiver types, like those that read from files, or some inter-process communication, would be written out of this interface by using HTTP/GRPCClientInterceptors.

I interpret this limiter extension interface as an interface for admission control that prevents the controller from "biting off more than it can chew" and I think starting here with mebibytes is a good first step, because I can't think of any other useful data you can derive from incoming requests that could be used for resource utilization control. Anything else I can think of (eg., headers, resource attributes, etc) start to step into the realm of auth extensions, which I see as a different thing.

[bogdandrutu]

Apparently there are use-cases where users may want number of concurrent requests or bytes per a path (url) or per tenant (in http/grpc headers). In order to support that we need to extend to add more details about the request. Also, I was thinking 10 mins ago exactly about scrapers situation and we should cover that as well but would be a different interface for that.

[mattsains]

I think concurrent requests is a valid use case for this issue, but is covered by jmacd's interface simply by the number of times you call Limiter.Acquire.

Bytes per path also makes sense to me, but I am not sure how to model it except by using request interceptors.

Bytes per tenant to me sounds like an auth extension - we allow/disallow something based on who you are as a requester or data you have provided. I see this feature more about deciding something based on collector state rather than by request headers or anything like that.

[jmacd]

@bogdandrutu to your suggestion about expansion. I was aware of #7441, and I understand how if middleware and limiters are to co-exist, they should be listed and executed as one series interleaving the various activities.

As @mattsains suggested, this is less general purpose. Perhaps this specialization is necessary, anyway. First, I would state that the OTel-Arrow streaming receiver, where my proposal originates from, would benefit from the streaming interceptor as illustrated in my draft PR #12600; however, the OTAP protocol carries a compressed segment inside the gRPC data stream, which it decodes and expands into pdata objects. The OTAP receiver requires direct access to the Limiter as in my draft, because the gRPC-level footprint is artificially low--moreover pipeline data that is acquired during Send() is not released when the call returns. I would suggest that pull-based receivers and special cases like mine require access to the sort of weight-based limiter in my proposal where gRPC and HTTP interceptors are not applicable.

I made these drafts as narrow as possible, on purpose. Here are some observations about additional information that would be useful:

• The xextension/storage interface has a GetClient() call which accepts component.Kind and component.ID of the requesting component. These would be useful in allowing limiters the ability to prioritize by the requesting component. However, the call to interpret configgrpc.ServerConfig does not currently receive this information, so I could not automatically configure limiters with these parameters. I figured these could be passed through the context if we decide it's important: the Collector would set a component.Metadata value in Context before calling component Start() so that GetLimiter(ctx) can see which component is requesting a limiter for which signal type.
• As discussed, I've tried to avoid doing things an Auth extension could also do. Critically, the Limiters in my draft are called after the Auth extension and after the client.Metadata object is set in the context. Therefore, at runtime the Limiters have access to the result of auth and more through context, provided the receiver configures include_metadata.
• The interceptors have access to request metadata, such as gRPC method name. I omitted this on purpose. I can see protocol metadata being useful to a limiter, for example to support prioritizing by prototocol name for receiver components that support multiple protocols.

Here's how I propose to move forward.

1. For configuration purposes, instead of a package configlimiter, with embedded Limiters []configlimiter.Limtation in several places, we will use a package configmiddleware and something like Middleware []configmiddleware.Middleware, so that the concept of limiters and interceptors are combined into a single sequence. This is a small dependency, like configauth, that redirects to the corresponding extension(s).
2. The xextension/limiter interface would remain distinct, and pull-based receivers or complex use-cases like OTel-Arrow can still use it directly. These components will call a method like (configmiddleware.Middleware).GetLimter(ctx) limiter.Extension to get limiters.
3. For Add support for "middleware extensions" #7441, a new extension or extensions like in @bogdandrutu's comment would support HTTP- and gRPC-specific middleware.
4. The configgrpc and confighttp server structs would contain an ordered list of middleware components, some would be limiters, some would be arbitrary middleware.
5. There would be an internal helper package, to adapt from a Limiter extension component to a Middleware extension component; limiter extensions that don't care about HTTP vs gRPC can just use the helper library to construct the appropriate middleware on their behalf. The configgrpc and confighttp code will use calls like (configmiddleware.Middleware).GetHTTPMiddleware(ctx) httpmiddleware.Extension.

I'm a little worried that the scope of the change described above is much larger than #9591: I have only to support server-side middleware, for example. I think it would be nice, to move ahead with #9591 expediently, not to introduce the middleware extensions in the same work stream as the limiters. We start with configmiddleware, similar to configauth, but the package would only support Middleware.GetLimiter(). As separate work (lesser priority), new middleware extensions can be added, six ways even: (Client, Server) x (HTTP, gRPC-unary, gRPC-stream); as these are added, the configmiddleware package will gain new interfaces (e.g., Middleware.GetHTTPServerMiddleware()) to obtain the correct underlying types.

[axw]

Mentioned on the Collector SIG, but for posterity: we've built a rate limiter processor that I think would fit well with this proposal, and which we would be happy to contribute: https://github.com/elastic/opentelemetry-collector-components/blob/main/processor/ratelimitprocessor

This processor started life as an auth extension in order to rate limit either the receiver (admission) or exporter (so we could rate limit post-processing). Eventually we made it a processor to satisfy pull-based receivers, e.g. see open-telemetry/opentelemetry-collector-contrib#35204

I've had in mind that push receivers could use the limiter as middleware to control admission, and for pull/scraper receivers we could use a processor, but in the latter case this does mean that the scraper may allocate memory before it knows it will be limited - could be a problem for a file scraper with many files.

In open-telemetry/opentelemetry-collector-contrib#35204 (comment) I describe a possible solution for rate limiting the filelog receiver by k8s pod labels. Again that assumes the use of a processor. I think we could do something similar but with some additional config on the filelog receiver to invoke the xextension/limiter directly.

I'm imagining the following kind of config for the OTLP scenario:

extensions:
  ratelimiter:
    metadata_keys: [x-tenant-id]

receivers:
  otlp:
    protocols:
      http:
        endpoint: :4318
        include_metadata: true # adds "x-tenant-id" header to client metadata in context
        middleware: [ratelimiter]

For the k8s filelog scenario:

extensions:
  k8s_observer:
  ratelimiter/k8s:
    metadata_keys: [organization]

receivers:
  receiver_creator:
    watch_observers: [k8s_observer]
    receivers:
      filelog:
        rule: type == "pod.container"
        limiter: ratelimiter/k8s
        metadata:
          - key: organization
            value: `pod.labels["service_org"]`
        config:
          include:
            - /var/log/pods/`pod.namespace`_`pod.name`_`pod.uid`/`container_name`/*.log
          include_file_name: false
          include_file_path: true
          operators:
            - id: container-parser
              type: container

(metadata would be new config added to receivercreator, and limiter would be a new config added to filelog & other scraper receivers)

[bogdandrutu]

@jmacd regarding "Here's how I propose to move forward." I am 100% aligned with your proposal, except the last step 5, which I will show you an alternative proposal when we get there.

[jmacd]

@axw Let's focus on this line:

        limiter: ratelimiter/k8s

I'm having trouble reconciling something here. For the gRPC/HTTP middleware cases, we understand it makes sense to configure a sequence of components, e.g.,

        middleware: [one, two, three]

where the components may be limiters or arbitrary middleware extensions. For a file-based receiver as you illustrated, I don't think we want to call it "middleware" for non-HTTP/gRPC-based systems, since not all forms of middleware are useful. My question is whether we should support more than one limiter in this case. If a user wants to limit based on memory and per-tenant rate limits, for example, how would you configure it? I was thinking

        limiters: [ratelimiter/k8s, admissionlimiter]

which indicates first check the rate limit, then the admission limit.

[jmacd]

@axw and @bogdandrutu please see a second draft incorporating the feedback above.
#12633

[axw]

My question is whether we should support more than one limiter in this case. If a user wants to limit based on memory and per-tenant rate limits, for example, how would you configure it? I was thinking

    limiters: [ratelimiter/k8s, admissionlimiter]

which indicates first check the rate limit, then the admission limit.

The use case I had in mind is specifically for admission control, based on open-telemetry/opentelemetry-collector-contrib#35204. The goal there is to avoid noisy data sources from overwhelming others. You could do that either at admission time (if the receiver can be taught how to identify of the data source, as I described in open-telemetry/opentelemetry-collector-contrib#35204 (comment)), or in a processor after receiving.

I don't have a very strong opinion on whether the interface for pull-based receivers should support exactly one or a list of limiters. I suppose you might want to have both a data source identity-based rate limiter (for fairness) and a process-global memory limiter (to prevent OOM in general).

[jmacd]

Please see the complete proposal in #12700.

Note that I've allowed the limitermiddlewareextension to present as a limiter Provider impl, this makes it so that for push-based receivers, what I can do is scan the middleware list to find an ordered set of limiters. Pull-based receivers could either list middleware or only limiters, probably middleware is more appropriate, and only special-cases limitermiddlewareextension or non-HTTP/non-GRPC protocols will use configlimiter.Limiter fields directly.