envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.34k stars 4.7k forks source link

ext_proc - sampling support #30147

Open rshriram opened 9 months ago

rshriram commented 9 months ago

Title: ext_proc - sampling support

Description: We need the ability to sample ext_proc calls. A simple sample_rate field (0.0 to 1.0) within ext_proc filter that activates the filter and makes the external call only when necessary. This becomes especially useful when we finish implementing the async support, allowing users to selectively sample a fraction of traffic for observation.

Once sampled, the stream should be active for the entire request, honoring all the processing modes, etc.

cc @tyxia @htuch

kyessenov commented 9 months ago

Should the sampling be inside the unified matcher? There's a common need to sample logs statefully and probabilistically, for example in https://github.com/envoyproxy/envoy/issues/30006.

htuch commented 9 months ago

Yeah, preference is this is outside of ext_proc and part of the match model.

tyxia commented 9 months ago

/assign @tyxia

github-actions[bot] commented 8 months ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 8 months ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

yanjunxiang-google commented 3 months ago

I am a little bit confused on the comments.

The original issue description is for adding sampling support for ext_proc to makes the external call only when necessary. To implement that, we can simply toss a coin in the ext_proc filter and make the external call based on the coin toss result.

@kyessenov @htuch could you please elaborate the idea to implement it in the unified matcher?

yanjunxiang-google commented 3 months ago

/assign @yanjunxiang-google

htuch commented 2 months ago

I think @kyessenov is suggesting we just have the coin flip in the matcher, which will be more generic, as there are other legit use cases for sampling.

tyxia commented 2 months ago

I think it can be understood as: Performing the sampling at the higher level and centralized place. This will benefit all delegated filters (like ext_proc, ext_authz, etc) as the delegated filters are invoked based on sampling result