feat: add ext_proc filter crate with Envoy-compatible config by usize · Pull Request #182 · praxis-proxy/praxis

usize · 2026-05-10T22:38:20Z

Introduce praxis-proxy-ext-proc as a standalone workspace crate at filter/ext-proc/, alongside filter/proto/. Isolated from the filter builtins to contain the gRPC/tonic/proto dependency surface.

The config mirrors Envoy's ExternalProcessor proto shape for easy migration from Envoy deployments:

target, failure_mode, message_timeout_ms
processing_mode (header/body/trailer send modes)
allow_mode_override, observability_mode, disable_immediate_response, max_message_timeout_ms

Features not yet implemented are parsed but rejected during validation with a clear error. The filter is not registered in FilterRegistry::with_builtins(); consumers register it explicitly via registry.register() or register_filters!.

Opus 4.6 aided in codebase exploration and feature implementation.

usize · 2026-05-10T22:39:28Z

Experimenting with saving summarized agent session for prs: https://gist.github.com/usize/ebb03b9963383cf01f433bd5bf7719e3

usize · 2026-05-14T23:22:48Z

@shaneutt Aside from config parsing there's not much happening here. Except it does establish the structure of the filter/ext-proc crate. I wanted to check your thoughts on the structure.

praxis-bot · 2026-05-14T23:28:02Z

Converted to draft: required checks failing.

praxis-bot · 2026-05-19T16:45:16Z

PR too large

This PR adds 1199 lines (limit: 500).

Large PRs are difficult to review and more likely to introduce subtle bugs. Please break this contribution into smaller, focused PRs that each address a single concern.

See our coding conventions for guidance.

If this PR legitimately requires a large change, a maintainer can add the skip/pr-hygiene label to skip this check.

twghu · 2026-05-21T09:09:02Z

+        )
+        .into());
+    }
+    if cfg.message_timeout_ms == 0 {


Should this have an upper bound validation?

As a matter of taste, while there is definitely a reasonable limit on how long a timeout should be... it's more a matter of taste than a lower bound. Should someone have a timeout of 3600 * 1000? Well.. no. Probably not. How about 300 * 1000? Still high, but seems murkier.

twghu · 2026-05-21T09:12:27Z

+license.workspace = true
+repository.workspace = true
+readme = "../../README.md"
+publish = true


This is premature until all approved

Suggested change

publish = true

publish = false

twghu · 2026-05-21T09:24:29Z

+        r#"
+target: "http://127.0.0.1:50051"
+message_timeout_ms: 500
+max_message_timeout_ms: 5000


With the change suggested for from_config() in lib.rs this should validate timeout.

twghu · 2026-05-21T09:27:22Z

+    let yaml: serde_yaml::Value = serde_yaml::from_str(
+        r#"
+target: "http://127.0.0.1:50051"
+deferred_close_timeout_ms: 10000


accepts_custom_status_on_error and accepts_custom_deferred_close_timeout only assert filter.name()

These tests verify that certain configs are accepted, but the only assertion is filter.name() == "ext_proc", which is true for any successfully built filter. They're really just "doesn't error" tests. That's a limitation of the private field / Box interface rather than a test bug, but it's worth noting the tests don't actually confirm the values were stored.

That's true. I think that for a skeleton.. this seems okay? As the implementation proceeds, integration tests will hit the private fields. Or we can revisit and see if it's worth making it possible to inspect here.

twghu

Some nits and suggestions

twghu · 2026-05-21T10:08:51Z

+    status_on_error: u16,
+
+    /// gRPC endpoint URI (retained for diagnostics).
+    target: String,


Is this field in the right location?

I think it's worth having for logging purposes?

shaneutt · 2026-05-21T20:57:54Z

+    }
+
+    async fn on_request(&self, _ctx: &mut HttpFilterContext<'_>) -> Result<FilterAction, FilterError> {
+        tracing::trace!(


Should be warn! instead, so if it ever gets triggered (for some reason) it's seen

Suggested change

tracing::trace!(

tracing::warn!(

shaneutt · 2026-05-21T21:00:38Z

+license.workspace = true
+repository.workspace = true
+readme = "../../README.md"
+publish = true


Suggested change

publish = true

publish = false

Introduce `praxis-proxy-ext-proc` as a standalone workspace crate at filter/ext-proc/, alongside filter/proto/. Isolated from the filter builtins to contain the gRPC/tonic/proto dependency surface. The config mirrors Envoy's ExternalProcessor proto shape for easy migration from Envoy deployments: - target, failure_mode, message_timeout_ms - processing_mode (header/body/trailer send modes) - allow_mode_override, observability_mode, disable_immediate_response, max_message_timeout_ms Features not yet implemented are parsed but rejected during validation with a clear error. The filter is not registered in FilterRegistry::with_builtins(); consumers register it explicitly via registry.register() or register_filters!. Opus 4.6 aided in codebase exploration and feature implementation. Signed-off-by: usize <mofoster@redhat.com>

Reject status_on_error values outside the valid HTTP status code range (100..=599) and message_timeout_ms of 0, which would cause immediate gRPC timeouts. Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

Error messages now show the snake_case YAML value the user typed (e.g. 'buffered') instead of the Rust enum variant ('Buffered'). Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

Use 'filter\/ext-proc' instead of bare 'ext-proc' to avoid matching unrelated workspace members in the future. Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

tonic's Channel requires a tokio reactor during Drop, causing panics when tests share threads without a runtime. Convert all tests that call ExtProcFilter::from_config to #[tokio::test]. Also replace invalid_failure_mode_errors with invalid_target_uri_errors since #[serde(default)] silently defaults unknown enum variants in serde_yaml::from_value, making the original assertion untestable. Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

failure_mode is a pipeline-level concern owned by FilterEntry, not a filter-specific config field. strip_structural_keys() already removes it from the YAML before it reaches from_config, so the field always silently defaulted regardless of user input. Remove the field from both the config struct and the filter struct. The pipeline executor reads failure_mode from FilterEntry and applies it via PipelineFilter, matching all other filters. Also removes the now-unused praxis-core direct dependency. Signed-off-by: usize <mofoster@redhat.com>

Adds tests for validation paths that had no coverage: - response_header_mode: skip rejection - response_trailer_mode: send rejection - all response_body_mode variants (streamed, buffered, buffered_partial, full_duplex_streamed) - allowed_override_modes with non-empty entries Replaces the single-variant body mode tests with comprehensive loops that cover all four BodySendMode variants for both request and response directions. Signed-off-by: usize <mofoster@redhat.com>

Proves that failure_mode flows through the pipeline infrastructure rather than being parsed by the filter itself: - failure_mode in filter YAML is stripped by parse_filter_config - FilterEntry captures failure_mode: open and closed correctly - FilterEntry defaults failure_mode to Closed when omitted - FilterPipeline::build succeeds with ext_proc entries carrying different failure_mode values Signed-off-by: usize <mofoster@redhat.com>

- Remove unused praxis-proto dependency from filter/ext-proc - Update praxis-ext-proc version from 0.2.0 to 0.3.1 in workspace dependencies to match the workspace version Signed-off-by: usize <mofoster@redhat.com>

This is a workspace-internal crate, not intended for crates.io. Signed-off-by: usize <mofoster@redhat.com>

If the unimplemented on_request path ever fires in a real deployment it should be visible in logs, not buried at trace level. Signed-off-by: usize <mofoster@redhat.com>

Describe the rejection condition rather than just naming the field. Signed-off-by: usize <mofoster@redhat.com>

The project convention requires inline format args. Field access expressions are bound to locals first since format! does not support dotted paths directly. Signed-off-by: usize <mofoster@redhat.com>

The input type is inferred; only the return type annotation is needed for the .into() conversion. Signed-off-by: usize <mofoster@redhat.com>

Enables diagnostic formatting for the filter struct. All fields including tonic::Channel implement Debug. Signed-off-by: usize <mofoster@redhat.com>

All fields are Copy enums (HeaderSendMode, BodySendMode), so the containing struct can be Copy too. Signed-off-by: usize <mofoster@redhat.com>

Use explicit "not yet implemented" instead of "(skeleton)" which was unclear to reviewers. Signed-off-by: usize <mofoster@redhat.com>

Make the ExternalProcessorClient relationship explicit so the next contributor knows praxis-proto needs to be added to Cargo.toml when implementing the gRPC callout. Signed-off-by: usize <mofoster@redhat.com>

Reject max_message_timeout_ms of 0 and values less than message_timeout_ms, which would make the upper bound meaningless. Negative values are caught by serde (u64), but a test guards against future type changes. Signed-off-by: usize <mofoster@redhat.com>

The Copy derive on ProcessingModeConfig triggered trivially_copy_pass_by_ref — pass by value instead of reference. Also reformats chained method calls per nightly rustfmt. Signed-off-by: usize <mofoster@redhat.com>

The workspace migrated from serde_yaml to yaml_serde in praxis-proxy#381. Cargo.lock for ext-proc needs to reflect the new package name. Signed-off-by: usize <mofoster@redhat.com>

usize added the conventions/bypass label May 10, 2026

usize force-pushed the feat/ext-proc-skeleton branch 3 times, most recently from 1619fd4 to 031cb54 Compare May 14, 2026 23:16

usize marked this pull request as ready for review May 14, 2026 23:19

usize requested a review from a team May 14, 2026 23:19

praxis-bot marked this pull request as draft May 14, 2026 23:28

usize marked this pull request as ready for review May 15, 2026 00:16

shaneutt self-assigned this May 15, 2026

shaneutt added this to AI Gateway May 15, 2026

shaneutt moved this to Review in AI Gateway May 15, 2026

shaneutt added this to the v0.4.0 milestone May 15, 2026

shaneutt requested a review from twghu May 15, 2026 12:15

shaneutt requested changes May 15, 2026

View reviewed changes

shaneutt removed the conventions/bypass label May 15, 2026

shaneutt requested review from twghu and removed request for twghu May 19, 2026 13:29

shaneutt added the holding-pattern Waiting for discussions or contributor updates in order to proceed label May 19, 2026

praxis-bot closed this May 19, 2026

github-project-automation Bot moved this from Review to Done in AI Gateway May 19, 2026

usize reopened this May 19, 2026

praxis-bot closed this May 19, 2026

usize added the skip/pr-hygiene PR size and description bypass label May 19, 2026

usize reopened this May 19, 2026

usize force-pushed the feat/ext-proc-skeleton branch from 836a254 to c2c6d3d Compare May 19, 2026 19:55

usize requested a review from a team May 21, 2026 00:46

twghu reviewed May 21, 2026

View reviewed changes

Comment thread filter/ext-proc/src/lib.rs

twghu reviewed May 21, 2026

View reviewed changes

twghu requested changes May 21, 2026

View reviewed changes

twghu reviewed May 21, 2026

View reviewed changes

shaneutt requested changes May 21, 2026

View reviewed changes

usize added 21 commits May 22, 2026 18:05

fix: use Display instead of Debug for BodySendMode in error messages

6b0d7f1

Error messages now show the snake_case YAML value the user typed (e.g. 'buffered') instead of the Rust enum variant ('Buffered'). Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

fix: anchor ext-proc sed pattern in Containerfile

488c436

Use 'filter\/ext-proc' instead of bare 'ext-proc' to avoid matching unrelated workspace members in the future. Implementation assisted by Opus 4.6 Signed-off-by: usize <mofoster@redhat.com>

fix: remove unused praxis-proto dep and update ext-proc version

d053d73

- Remove unused praxis-proto dependency from filter/ext-proc - Update praxis-ext-proc version from 0.2.0 to 0.3.1 in workspace dependencies to match the workspace version Signed-off-by: usize <mofoster@redhat.com>

fix: set publish = false for ext-proc crate

a745776

This is a workspace-internal crate, not intended for crates.io. Signed-off-by: usize <mofoster@redhat.com>

fix: use warn! for skeleton on_request in ext_proc

58fb5e0

If the unimplemented on_request path ever fires in a real deployment it should be visible in logs, not buried at trace level. Signed-off-by: usize <mofoster@redhat.com>

fix: clarify assert message for zero message_timeout_ms test

40e76ee

Describe the rejection condition rather than just naming the field. Signed-off-by: usize <mofoster@redhat.com>

fix: use inline format args in ext_proc error messages

8f9f882

The project convention requires inline format args. Field access expressions are bound to locals first since format! does not support dotted paths directly. Signed-off-by: usize <mofoster@redhat.com>

fix: simplify closure param in ext_proc target URI error

1eec3c6

The input type is inferred; only the return type annotation is needed for the .into() conversion. Signed-off-by: usize <mofoster@redhat.com>

fix: derive Debug on ExtProcFilter

48be75b

Enables diagnostic formatting for the filter struct. All fields including tonic::Channel implement Debug. Signed-off-by: usize <mofoster@redhat.com>

fix: derive Clone and Copy on ProcessingModeConfig

78109c2

All fields are Copy enums (HeaderSendMode, BodySendMode), so the containing struct can be Copy too. Signed-off-by: usize <mofoster@redhat.com>

fix: replace ambiguous skeleton label in ext_proc warn message

fe82dcc

Use explicit "not yet implemented" instead of "(skeleton)" which was unclear to reviewers. Signed-off-by: usize <mofoster@redhat.com>

fix: document praxis-proto dependency on ext_proc channel field

9e91ab3

Make the ExternalProcessorClient relationship explicit so the next contributor knows praxis-proto needs to be added to Cargo.toml when implementing the gRPC callout. Signed-off-by: usize <mofoster@redhat.com>

fix: apply rustfmt after Copy derive and validation changes

28af1b3

The Copy derive on ProcessingModeConfig triggered trivially_copy_pass_by_ref — pass by value instead of reference. Also reformats chained method calls per nightly rustfmt. Signed-off-by: usize <mofoster@redhat.com>

fix: update Cargo.lock for yaml_serde migration

2018b23

The workspace migrated from serde_yaml to yaml_serde in praxis-proxy#381. Cargo.lock for ext-proc needs to reflect the new package name. Signed-off-by: usize <mofoster@redhat.com>

usize force-pushed the feat/ext-proc-skeleton branch from c97e938 to 2018b23 Compare May 23, 2026 01:05

Conversation

usize commented May 10, 2026

Uh oh!

usize commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

usize commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praxis-bot commented May 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

praxis-bot commented May 19, 2026

PR too large

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twghu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

usize commented May 10, 2026 •

edited

Loading

usize commented May 14, 2026 •

edited

Loading