Skip to content

Add ExtProc Logging Bits for Immediate Response, Continue and Replace#41602

Merged
adisuissa merged 28 commits intoenvoyproxy:mainfrom
melginaldi:extproc-logging-immresp-contreplace
Dec 11, 2025
Merged

Add ExtProc Logging Bits for Immediate Response, Continue and Replace#41602
adisuissa merged 28 commits intoenvoyproxy:mainfrom
melginaldi:extproc-logging-immresp-contreplace

Conversation

@melginaldi
Copy link
Copy Markdown
Contributor

@melginaldi melginaldi commented Oct 17, 2025

Commit Message: Add stats counter in FilterState for when an immediate response is sent
Additional Description: These bits will be useful for internal debugging. I also did some refactoring of the ext_proc_integration_unit test so testing access logs is less code duplication
Risk Level: Low

Docs Changes: N/A
Release Notes: Add new stats counter for Immediate Response sent
Platform Specific Features: N/A

/assign @yanjunxiang-google

…in FilterState

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #41602 was opened by melginaldi.

see: more, trace.

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@melginaldi melginaldi marked this pull request as ready for review October 17, 2025 19:41
@melginaldi
Copy link
Copy Markdown
Contributor Author

/assign @yanjunxiang-google

…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@RyanTheOptimist
Copy link
Copy Markdown
Contributor

/assign @tyxia

…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Copy link
Copy Markdown
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general. Thanks for contribution!

I will let @yanjunxiang-google review it as well

Comment thread source/extensions/filters/http/ext_proc/processor_state.h Outdated
Comment thread test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc Outdated
@tyxia
Copy link
Copy Markdown
Member

tyxia commented Nov 2, 2025

/wait

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@adisuissa
Copy link
Copy Markdown
Contributor

@yanjunxiang-google can you PTAL?

@yanjunxiang-google
Copy link
Copy Markdown
Contributor

yanjunxiang-google commented Nov 13, 2025

/wait

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Nov 13, 2025

Could you elaborate on the motivation and use case ?

continue_and_replace and immediate_response appears to be a niche/specical case, compared to universally applicable fields like call_status, callback_state, and traffic_direction in ext_proc.

Also, if it is for debugging, can we use Envoy trace/log

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Nov 13, 2025

/wait

@yanjunxiang-google
Copy link
Copy Markdown
Contributor

yanjunxiang-google commented Nov 14, 2025

@melginaldi Actually, can we add an Envoy global immediate_response stats and continue_and_replace stats counters here:

#define ALL_EXT_PROC_FILTER_STATS(COUNTER) \
. It gives us good idea on these events as well. What's the benefit to put them into a per call stats?

yanjunxiang-google

This comment was marked as outdated.

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@melginaldi
Copy link
Copy Markdown
Contributor Author

@melginaldi Actually, can we add an Envoy global immediate_response stats and continue_and_replace stats counters here:

#define ALL_EXT_PROC_FILTER_STATS(COUNTER) \

. It gives us good idea on these events as well. What's the benefit to put them into a per call stats?

Yes, I have changed this PR to have immediate_response as a stats counter instead of a bool. Additionally, I have removed continue and replace as I don't feel it is necessary with #41295

…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@phlax
Copy link
Copy Markdown
Member

phlax commented Dec 8, 2025

@yanjunxiang-google i think is waiting on further review

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@yanjunxiang-google
Copy link
Copy Markdown
Contributor

LGTM

@phlax
Copy link
Copy Markdown
Member

phlax commented Dec 8, 2025

@tyxia @adisuissa i think this now needs maintainer signoff

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!
Please also add a release note (in the spirit of

- area: http
change: |
added upstream_rq_completed counter for :ref:`total requests completed
<config_cluster_manager_cluster_stats_dynamic_http>` to dynamic HTTP counters.
)

Comment thread source/extensions/filters/http/ext_proc/ext_proc.h
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…resp-contreplace

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
@melginaldi
Copy link
Copy Markdown
Contributor Author

Thanks, LGTM! Please also add a release note (in the spirit of

- area: http
change: |
added upstream_rq_completed counter for :ref:`total requests completed
<config_cluster_manager_cluster_stats_dynamic_http>` to dynamic HTTP counters.

)

Done! Thanks for the review

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@adisuissa adisuissa enabled auto-merge (squash) December 11, 2025 20:52
@adisuissa adisuissa merged commit b9e162b into envoyproxy:main Dec 11, 2025
25 checks passed
MayorFaj pushed a commit to MayorFaj/envoy that referenced this pull request Dec 15, 2025
…nd Replace (envoyproxy#41602)

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: MayorFaj <mayorfaj@gmail.com>
grnmeira pushed a commit to grnmeira/envoy that referenced this pull request Mar 20, 2026
…nd Replace (envoyproxy#41602)

Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Gustavo <grnmeira@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants