ext_proc: add serializeAsProto() and serializeAsString() to support access logs #40000

agrawroh · 2025-06-23T23:57:18Z

Description

This PR adds an override for serializeAsProto() and serializeAsString() to support ExtProcLoggingInfo in the access logs. After this change we should be able to do this to log the filter state emitted from ExtProc filter to measure latencies.

access_log:
  - name: envoy.access_loggers.stdout
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
      log_format:
        json_format:
          ext_proc_plain: "%FILTER_STATE(envoy.filters.http.ext_proc:PLAIN)%"
          ext_proc_typed: "%FILTER_STATE(envoy.filters.http.ext_proc:TYPED)%"
          ext_proc_field: "%FILTER_STATE(envoy.filters.http.ext_proc:FIELD:inbound_header_latency_us)%"

Fix #39999

Commit Message: ext_proc: add serializeAsProto() and serializeAsString() to support access logs
Additional Description: Added support for logging ExtProcLoggingInfo emitted by ExtProc from access logs
Risk Level: Low
Testing: Added Unit/Integration Tests
Docs Changes: N/A
Release Notes: N/A

repokitteh-read-only · 2025-06-23T23:57:25Z

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: #40000 was opened by agrawroh.

see: more, trace.

…ccess logs Signed-off-by: Rohit Agrawal <[email protected]>

agrawroh · 2025-06-25T18:05:08Z

@yanjunxiang-google May I please get a review from you on this as well?

yanjunxiang-google · 2025-06-25T22:20:51Z

source/extensions/filters/http/ext_proc/ext_proc.h

+    if (field_name == "inbound_header_latency_us" && inbound.header_stats_) {
+      return static_cast<int64_t>(inbound.header_stats_->latency_.count());
+    }
+    if (field_name == "inbound_header_status" && inbound.header_stats_) {


Should this field named as inboud_header_call_status to be more clear?

Sure, I can change it. If you have any more feedback I can club that and do a change including everything.

yanavlasov

/wait-any

yanavlasov · 2025-06-26T19:57:42Z

source/extensions/filters/http/ext_proc/ext_proc.h

@@ -123,6 +123,164 @@ class ExtProcLoggingInfo : public Envoy::StreamInfo::FilterState::Object {
  const Envoy::ProtobufWkt::Struct& filterMetadata() const { return filter_metadata_; }
  const std::string& httpResponseCodeDetails() const { return http_response_code_details_; }

+  ProtobufTypes::MessagePtr serializeAsProto() const override {


I would move the function body to the .cc file to make class definition more readable.

yanavlasov · 2025-06-26T19:58:55Z

source/extensions/filters/http/ext_proc/ext_proc.h

+    const auto& inbound = grpcCalls(envoy::config::core::v3::TrafficDirection::INBOUND);
+    const auto& outbound = grpcCalls(envoy::config::core::v3::TrafficDirection::OUTBOUND);
+
+    if (field_name == "inbound_header_latency_us" && inbound.header_stats_) {


Are the field names to be used in the access log format? It does not look like they are documented anywhere.

yanjunxiang-google · 2025-06-26T21:48:30Z

source/extensions/filters/http/ext_proc/ext_proc.h

@@ -123,6 +123,164 @@ class ExtProcLoggingInfo : public Envoy::StreamInfo::FilterState::Object {
  const Envoy::ProtobufWkt::Struct& filterMetadata() const { return filter_metadata_; }
  const std::string& httpResponseCodeDetails() const { return http_response_code_details_; }

+  ProtobufTypes::MessagePtr serializeAsProto() const override {
+    auto struct_msg = std::make_unique<ProtobufWkt::Struct>();
+    const auto& inbound = grpcCalls(envoy::config::core::v3::TrafficDirection::INBOUND);


Maybe you can directly use decoding_processor_grpc_calls_ and encoding_processor_grpc_calls_ here instead of inboud, outboud?

yanjunxiang-google · 2025-06-26T21:53:24Z

source/extensions/filters/http/ext_proc/ext_proc.h

+    const auto& outbound = grpcCalls(envoy::config::core::v3::TrafficDirection::OUTBOUND);
+
+    if (inbound.header_stats_) {
+      (*struct_msg->mutable_fields())["inbound_header_latency_us"].set_number_value(


For the naming, maybe call it: request_header_latency_us, and response_header_latency_us to better aligned with naming in other places of this filter? Same as other places below.

yanjunxiang-google · 2025-06-26T21:56:07Z

source/extensions/filters/http/ext_proc/ext_proc.h

+          static_cast<double>(static_cast<int>(inbound.header_stats_->call_status_)));
+    }
+    if (inbound.body_stats_) {
+      (*struct_msg->mutable_fields())["inbound_body_calls"].set_number_value(


naming: request_body_call_count, i.e, to align with the original naming to make it clearer.

yanjunxiang-google · 2025-06-26T21:56:53Z

source/extensions/filters/http/ext_proc/ext_proc.h

+          static_cast<double>(inbound.body_stats_->total_latency_.count()));
+      (*struct_msg->mutable_fields())["inbound_body_max_latency_us"].set_number_value(
+          static_cast<double>(inbound.body_stats_->max_latency_.count()));
+      (*struct_msg->mutable_fields())["inbound_body_last_status"].set_number_value(


request_body_last_call_status

yanjunxiang-google · 2025-06-26T21:57:20Z

source/extensions/filters/http/ext_proc/ext_proc.h

+    if (inbound.trailer_stats_) {
+      (*struct_msg->mutable_fields())["inbound_trailer_latency_us"].set_number_value(
+          static_cast<double>(inbound.trailer_stats_->latency_.count()));
+      (*struct_msg->mutable_fields())["inbound_trailer_status"].set_number_value(


request_trailer_call_status

yanjunxiang-google · 2025-06-26T21:57:29Z

source/extensions/filters/http/ext_proc/ext_proc.h

+      (*struct_msg->mutable_fields())["inbound_trailer_status"].set_number_value(
+          static_cast<double>(static_cast<int>(inbound.trailer_stats_->call_status_)));
+    }
+    if (outbound.header_stats_) {


same as above

agrawroh force-pushed the ext-proc-hal branch from abb5dca to 2a42326 Compare June 24, 2025 01:11

agrawroh marked this pull request as ready for review June 24, 2025 01:22

agrawroh requested review from tyxia, mattklein123 and yanavlasov as code owners June 24, 2025 01:22

ext_proc: add serializeAsProto() and serializeAsString() to support a…

84d59bc

…ccess logs Signed-off-by: Rohit Agrawal <[email protected]>

agrawroh force-pushed the ext-proc-hal branch from 2a42326 to 84d59bc Compare June 24, 2025 05:49

wbpcode assigned yanavlasov Jun 24, 2025

agrawroh assigned yanjunxiang-google Jun 25, 2025

yanjunxiang-google reviewed Jun 25, 2025

View reviewed changes

yanavlasov requested changes Jun 26, 2025

View reviewed changes

repokitteh-read-only bot added the waiting:any label Jun 26, 2025

yanjunxiang-google reviewed Jun 26, 2025

View reviewed changes

repokitteh-read-only bot removed the waiting:any label Jun 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ext_proc: add serializeAsProto() and serializeAsString() to support access logs #40000

ext_proc: add serializeAsProto() and serializeAsString() to support access logs #40000

agrawroh commented Jun 23, 2025 •

edited

Loading

Uh oh!

repokitteh-read-only bot commented Jun 23, 2025

Uh oh!

agrawroh commented Jun 25, 2025

Uh oh!

yanjunxiang-google Jun 25, 2025

Uh oh!

agrawroh Jun 25, 2025

Uh oh!

yanavlasov left a comment

Uh oh!

yanavlasov Jun 26, 2025

Uh oh!

yanavlasov Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

yanjunxiang-google Jun 26, 2025

Uh oh!

Uh oh!

ext_proc: add serializeAsProto() and serializeAsString() to support access logs #40000

Are you sure you want to change the base?

ext_proc: add serializeAsProto() and serializeAsString() to support access logs #40000

Conversation

agrawroh commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

repokitteh-read-only bot commented Jun 23, 2025

Uh oh!

agrawroh commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanavlasov 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!

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!

Uh oh!

agrawroh commented Jun 23, 2025 •

edited

Loading