accesslog: support node in cel (envoyproxy#31319)

Commit Message: with this patch, it's able send `xds.node.id` or `xds.node.metadata[xxx]` with command `%(CEL(xds.node.id))%`, this's useful when send log with OpenTelemetry sink. Additional Description: tests and release notes will be added if this's right forward. Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features:
gupta-deeptig · Dec 19, 2023 · 7e834d7 · 7e834d7
1 parent 56f88a1
commit 7e834d7
Show file tree

Hide file tree

Showing 16 changed files with 76 additions and 27 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -302,5 +302,8 @@ new_features:
 - area: access_log
   change: |
     Added new access log command operator ``%EMIT_TIME%`` to get the time when the log entry is emitted.
+- area: access_log
+  change: |
+    Added support for node data in ``%CEL%`` formatter.
 
 deprecated:
diff --git a/source/extensions/access_loggers/filters/cel/cel.cc b/source/extensions/access_loggers/filters/cel/cel.cc
@@ -9,17 +9,18 @@ namespace CEL {
 namespace Expr = Envoy::Extensions::Filters::Common::Expr;
 
 CELAccessLogExtensionFilter::CELAccessLogExtensionFilter(
-    Expr::BuilderInstanceSharedPtr builder, const google::api::expr::v1alpha1::Expr& input_expr)
-    : builder_(builder), parsed_expr_(input_expr) {
+    const ::Envoy::LocalInfo::LocalInfo& local_info, Expr::BuilderInstanceSharedPtr builder,
+    const google::api::expr::v1alpha1::Expr& input_expr)
+    : local_info_(local_info), builder_(builder), parsed_expr_(input_expr) {
   compiled_expr_ = Expr::createExpression(builder_->builder(), parsed_expr_);
 }
 
 bool CELAccessLogExtensionFilter::evaluate(const Formatter::HttpFormatterContext& log_context,
                                            const StreamInfo::StreamInfo& stream_info) const {
   Protobuf::Arena arena;
-  auto eval_status =
-      Expr::evaluate(*compiled_expr_, arena, stream_info, &log_context.requestHeaders(),
-                     &log_context.responseHeaders(), &log_context.responseTrailers());
+  auto eval_status = Expr::evaluate(*compiled_expr_, arena, &local_info_, stream_info,
+                                    &log_context.requestHeaders(), &log_context.responseHeaders(),
+                                    &log_context.responseTrailers());
   if (!eval_status.has_value() || eval_status.value().IsError()) {
     return false;
   }

diff --git a/source/extensions/access_loggers/filters/cel/cel.h b/source/extensions/access_loggers/filters/cel/cel.h
@@ -19,13 +19,15 @@ namespace CEL {
 
 class CELAccessLogExtensionFilter : public AccessLog::Filter {
 public:
-  CELAccessLogExtensionFilter(Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr,
+  CELAccessLogExtensionFilter(const ::Envoy::LocalInfo::LocalInfo& local_info,
+                              Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr,
                               const google::api::expr::v1alpha1::Expr&);
 
   bool evaluate(const Formatter::HttpFormatterContext& log_context,
                 const StreamInfo::StreamInfo& stream_info) const override;
 
 private:
+  const ::Envoy::LocalInfo::LocalInfo& local_info_;
   Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder_;
   const google::api::expr::v1alpha1::Expr parsed_expr_;
   Extensions::Filters::Common::Expr::ExpressionPtr compiled_expr_;

diff --git a/source/extensions/access_loggers/filters/cel/config.cc b/source/extensions/access_loggers/filters/cel/config.cc
@@ -33,6 +33,7 @@ Envoy::AccessLog::FilterPtr CELAccessLogExtensionFilterFactory::createFilter(
   }
 
   return std::make_unique<CELAccessLogExtensionFilter>(
+      context.serverFactoryContext().localInfo(),
       Extensions::Filters::Common::Expr::getBuilder(context.serverFactoryContext()),
       parse_status.value().expr());
 #else

diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc
@@ -385,6 +385,10 @@ absl::optional<CelValue> XDSWrapper::operator[](CelValue key) const {
     const absl::string_view filter_chain_name =
         filter_chain_info.has_value() ? filter_chain_info->name() : absl::string_view{};
     return CelValue::CreateStringView(filter_chain_name);
+  } else if (value == Node) {
+    if (local_info_) {
+      return CelProtoWrapper::CreateMessage(&local_info_->node(), &arena_);
+    }
   }
   return {};
 }

diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h
@@ -89,6 +89,7 @@ constexpr absl::string_view RouteName = "route_name";
 constexpr absl::string_view RouteMetadata = "route_metadata";
 constexpr absl::string_view UpstreamHostMetadata = "upstream_host_metadata";
 constexpr absl::string_view FilterChainName = "filter_chain_name";
+constexpr absl::string_view Node = "node";
 
 class WrapperFieldValues {
 public:
@@ -232,12 +233,14 @@ class FilterStateWrapper : public BaseWrapper {
 
 class XDSWrapper : public BaseWrapper {
 public:
-  XDSWrapper(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info)
-      : BaseWrapper(arena), info_(info) {}
+  XDSWrapper(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info,
+             const LocalInfo::LocalInfo* local_info)
+      : BaseWrapper(arena), info_(info), local_info_(local_info) {}
   absl::optional<CelValue> operator[](CelValue key) const override;
 
 private:
   const StreamInfo::StreamInfo& info_;
+  const LocalInfo::LocalInfo* local_info_;
 };
 
 } // namespace Expr

diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc
@@ -64,23 +64,26 @@ absl::optional<CelValue> StreamActivation::FindValue(absl::string_view name,
     return CelValue::CreateMap(
         Protobuf::Arena::Create<FilterStateWrapper>(arena, *arena, info.filterState()));
   case ActivationToken::XDS:
-    return CelValue::CreateMap(Protobuf::Arena::Create<XDSWrapper>(arena, *arena, info));
-  };
+    return CelValue::CreateMap(
+        Protobuf::Arena::Create<XDSWrapper>(arena, *arena, info, local_info_));
+  }
   return {};
 }
 
 void StreamActivation::resetActivation() const {
+  local_info_ = nullptr;
   activation_info_ = nullptr;
   activation_request_headers_ = nullptr;
   activation_response_headers_ = nullptr;
   activation_response_trailers_ = nullptr;
 }
 
-ActivationPtr createActivation(const StreamInfo::StreamInfo& info,
+ActivationPtr createActivation(const LocalInfo::LocalInfo* local_info,
+                               const StreamInfo::StreamInfo& info,
                                const Http::RequestHeaderMap* request_headers,
                                const Http::ResponseHeaderMap* response_headers,
                                const Http::ResponseTrailerMap* response_trailers) {
-  return std::make_unique<StreamActivation>(info, request_headers, response_headers,
+  return std::make_unique<StreamActivation>(local_info, info, request_headers, response_headers,
                                             response_trailers);
 }
 
@@ -131,11 +134,13 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph
 }
 
 absl::optional<CelValue> evaluate(const Expression& expr, Protobuf::Arena& arena,
+                                  const LocalInfo::LocalInfo* local_info,
                                   const StreamInfo::StreamInfo& info,
                                   const Http::RequestHeaderMap* request_headers,
                                   const Http::ResponseHeaderMap* response_headers,
                                   const Http::ResponseTrailerMap* response_trailers) {
-  auto activation = createActivation(info, request_headers, response_headers, response_trailers);
+  auto activation =
+      createActivation(local_info, info, request_headers, response_headers, response_trailers);
   auto eval_status = expr.Evaluate(*activation, &arena);
   if (!eval_status.ok()) {
     return {};
@@ -147,7 +152,7 @@ absl::optional<CelValue> evaluate(const Expression& expr, Protobuf::Arena& arena
 bool matches(const Expression& expr, const StreamInfo::StreamInfo& info,
              const Http::RequestHeaderMap& headers) {
   Protobuf::Arena arena;
-  auto eval_status = Expr::evaluate(expr, arena, info, &headers, nullptr, nullptr);
+  auto eval_status = Expr::evaluate(expr, arena, nullptr, info, &headers, nullptr, nullptr);
   if (!eval_status.has_value()) {
     return false;
   }

diff --git a/source/extensions/filters/common/expr/evaluator.h b/source/extensions/filters/common/expr/evaluator.h
@@ -26,11 +26,13 @@ using ExpressionPtr = std::unique_ptr<Expression>;
 // Base class for the context used by the CEL evaluator to look up attributes.
 class StreamActivation : public google::api::expr::runtime::BaseActivation {
 public:
-  StreamActivation(const StreamInfo::StreamInfo& info,
+  StreamActivation(const ::Envoy::LocalInfo::LocalInfo* local_info,
+                   const StreamInfo::StreamInfo& info,
                    const ::Envoy::Http::RequestHeaderMap* request_headers,
                    const ::Envoy::Http::ResponseHeaderMap* response_headers,
                    const ::Envoy::Http::ResponseTrailerMap* response_trailers)
-      : activation_info_(&info), activation_request_headers_(request_headers),
+      : local_info_(local_info), activation_info_(&info),
+        activation_request_headers_(request_headers),
         activation_response_headers_(response_headers),
         activation_response_trailers_(response_trailers) {}
 
@@ -44,6 +46,7 @@ class StreamActivation : public google::api::expr::runtime::BaseActivation {
 
 protected:
   void resetActivation() const;
+  mutable const ::Envoy::LocalInfo::LocalInfo* local_info_{nullptr};
   mutable const StreamInfo::StreamInfo* activation_info_{nullptr};
   mutable const ::Envoy::Http::RequestHeaderMap* activation_request_headers_{nullptr};
   mutable const ::Envoy::Http::ResponseHeaderMap* activation_response_headers_{nullptr};
@@ -52,7 +55,8 @@ class StreamActivation : public google::api::expr::runtime::BaseActivation {
 
 // Creates an activation providing the common context attributes.
 // The activation lazily creates wrappers during an evaluation using the evaluation arena.
-ActivationPtr createActivation(const StreamInfo::StreamInfo& info,
+ActivationPtr createActivation(const ::Envoy::LocalInfo::LocalInfo* local_info,
+                               const StreamInfo::StreamInfo& info,
                                const ::Envoy::Http::RequestHeaderMap* request_headers,
                                const ::Envoy::Http::ResponseHeaderMap* response_headers,
                                const ::Envoy::Http::ResponseTrailerMap* response_trailers);
@@ -84,6 +88,7 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph
 // Evaluates an expression for a request. The arena is used to hold intermediate computational
 // results and potentially the final value.
 absl::optional<CelValue> evaluate(const Expression& expr, Protobuf::Arena& arena,
+                                  const ::Envoy::LocalInfo::LocalInfo* local_info,
                                   const StreamInfo::StreamInfo& info,
                                   const ::Envoy::Http::RequestHeaderMap* request_headers,
                                   const ::Envoy::Http::ResponseHeaderMap* response_headers,

diff --git a/source/extensions/formatter/cel/cel.cc b/source/extensions/formatter/cel/cel.cc
@@ -15,19 +15,22 @@ namespace Formatter {
 
 namespace Expr = Filters::Common::Expr;
 
-CELFormatter::CELFormatter(Expr::BuilderInstanceSharedPtr expr_builder,
+CELFormatter::CELFormatter(const ::Envoy::LocalInfo::LocalInfo& local_info,
+                           Expr::BuilderInstanceSharedPtr expr_builder,
                            const google::api::expr::v1alpha1::Expr& input_expr,
                            absl::optional<size_t>& max_length)
-    : expr_builder_(expr_builder), parsed_expr_(input_expr), max_length_(max_length) {
+    : local_info_(local_info), expr_builder_(expr_builder), parsed_expr_(input_expr),
+      max_length_(max_length) {
   compiled_expr_ = Expr::createExpression(expr_builder_->builder(), parsed_expr_);
 }
 
 absl::optional<std::string>
 CELFormatter::formatWithContext(const Envoy::Formatter::HttpFormatterContext& context,
                                 const StreamInfo::StreamInfo& stream_info) const {
   Protobuf::Arena arena;
-  auto eval_status = Expr::evaluate(*compiled_expr_, arena, stream_info, &context.requestHeaders(),
-                                    &context.responseHeaders(), &context.responseTrailers());
+  auto eval_status =
+      Expr::evaluate(*compiled_expr_, arena, &local_info_, stream_info, &context.requestHeaders(),
+                     &context.responseHeaders(), &context.responseTrailers());
   if (!eval_status.has_value() || eval_status.value().IsError()) {
     return absl::nullopt;
   }
@@ -60,7 +63,8 @@ CELFormatterCommandParser::parse(const std::string& command, const std::string&
                            parse_status.status().ToString());
     }
 
-    return std::make_unique<CELFormatter>(expr_builder_, parse_status.value().expr(), max_length);
+    return std::make_unique<CELFormatter>(local_info_, expr_builder_, parse_status.value().expr(),
+                                          max_length);
   }
 
   return nullptr;

diff --git a/source/extensions/formatter/cel/cel.h b/source/extensions/formatter/cel/cel.h
@@ -14,7 +14,8 @@ namespace Formatter {
 
 class CELFormatter : public ::Envoy::Formatter::FormatterProvider {
 public:
-  CELFormatter(Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr,
+  CELFormatter(const ::Envoy::LocalInfo::LocalInfo& local_info,
+               Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr,
                const google::api::expr::v1alpha1::Expr&, absl::optional<size_t>&);
 
   absl::optional<std::string>
@@ -24,6 +25,7 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider {
                                             const StreamInfo::StreamInfo&) const override;
 
 private:
+  const ::Envoy::LocalInfo::LocalInfo& local_info_;
   Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr expr_builder_;
   const google::api::expr::v1alpha1::Expr parsed_expr_;
   const absl::optional<size_t> max_length_;
@@ -33,12 +35,14 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider {
 class CELFormatterCommandParser : public ::Envoy::Formatter::CommandParser {
 public:
   CELFormatterCommandParser(Server::Configuration::CommonFactoryContext& context)
-      : expr_builder_(Extensions::Filters::Common::Expr::getBuilder(context)){};
+      : local_info_(context.localInfo()),
+        expr_builder_(Extensions::Filters::Common::Expr::getBuilder(context)){};
   ::Envoy::Formatter::FormatterProviderPtr parse(const std::string& command,
                                                  const std::string& subcommand,
                                                  absl::optional<size_t>& max_length) const override;
 
 private:
+  const ::Envoy::LocalInfo::LocalInfo& local_info_;
   Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr expr_builder_;
 };
 

diff --git a/source/extensions/matching/http/cel_input/cel_input.h b/source/extensions/matching/http/cel_input/cel_input.h
@@ -43,6 +43,7 @@ class HttpCelDataInput : public Matcher::DataInput<Envoy::Http::HttpMatchingData
     // and attributes from stream info.
     std::unique_ptr<google::api::expr::runtime::BaseActivation> activation =
         Extensions::Filters::Common::Expr::createActivation(
+            nullptr, // TODO: pass local_info to CEL activation.
             data.streamInfo(), maybe_request_headers.ptr(), maybe_response_headers.ptr(),
             maybe_response_trailers.ptr());
 

diff --git a/source/extensions/rate_limit_descriptors/expr/config.cc b/source/extensions/rate_limit_descriptors/expr/config.cc
@@ -36,7 +36,7 @@ class ExpressionDescriptor : public RateLimit::DescriptorProducer {
                           const Http::RequestHeaderMap& headers,
                           const StreamInfo::StreamInfo& info) const override {
     ProtobufWkt::Arena arena;
-    const auto result = Filters::Common::Expr::evaluate(*compiled_expr_.get(), arena, info,
+    const auto result = Filters::Common::Expr::evaluate(*compiled_expr_.get(), arena, nullptr, info,
                                                         &headers, nullptr, nullptr);
     if (!result.has_value() || result.value().IsError()) {
       // If result is an error and if skip_if_error is true skip this descriptor,

diff --git a/test/extensions/filters/common/expr/BUILD b/test/extensions/filters/common/expr/BUILD
@@ -24,6 +24,7 @@ envoy_extension_cc_test(
         "//source/extensions/clusters/original_dst:original_dst_cluster_lib",
         "//source/extensions/filters/common/expr:cel_state_lib",
         "//source/extensions/filters/common/expr:context_lib",
+        "//test/mocks/local_info:local_info_mocks",
         "//test/mocks/network:network_mocks",
         "//test/mocks/router:router_mocks",
         "//test/mocks/ssl:ssl_mocks",

diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc
@@ -7,6 +7,7 @@
 #include "source/extensions/filters/common/expr/cel_state.h"
 #include "source/extensions/filters/common/expr/context.h"
 
+#include "test/mocks/local_info/mocks.h"
 #include "test/mocks/network/mocks.h"
 #include "test/mocks/router/mocks.h"
 #include "test/mocks/ssl/mocks.h"
@@ -829,6 +830,7 @@ TEST(Context, FilterStateAttributes) {
 }
 
 TEST(Context, XDSAttributes) {
+  NiceMock<LocalInfo::MockLocalInfo> local_info;
   NiceMock<StreamInfo::MockStreamInfo> info;
   std::shared_ptr<NiceMock<Upstream::MockClusterInfo>> cluster_info(
       new NiceMock<Upstream::MockClusterInfo>());
@@ -847,7 +849,7 @@ TEST(Context, XDSAttributes) {
   info.downstream_connection_info_provider_->setFilterChainInfo(filter_chain_info);
 
   Protobuf::Arena arena;
-  XDSWrapper wrapper(arena, info);
+  XDSWrapper wrapper(arena, info, &local_info);
 
   {
     const auto value = wrapper[CelValue::CreateStringView(ClusterName)];
@@ -893,6 +895,11 @@ TEST(Context, XDSAttributes) {
     const auto value = wrapper[CelValue::CreateInt64(5)];
     EXPECT_FALSE(value.has_value());
   }
+  {
+    const auto value = wrapper[CelValue::CreateStringView(Node)];
+    EXPECT_TRUE(value.has_value());
+    ASSERT_TRUE(value.value().IsMessage());
+  }
 }
 
 } // namespace

diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc
@@ -44,7 +44,7 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest
 
     // Evaluate the CEL expression.
     Protobuf::Arena arena;
-    Expr::evaluate(*expr, arena, *stream_info, &request_headers, &response_headers,
+    Expr::evaluate(*expr, arena, nullptr, *stream_info, &request_headers, &response_headers,
                    &response_trailers);
   } catch (const CelException& e) {
     ENVOY_LOG_MISC(debug, "CelException: {}", e.what());

diff --git a/test/extensions/formatter/cel/cel_test.cc b/test/extensions/formatter/cel/cel_test.cc
@@ -34,6 +34,14 @@ class CELFormatterTest : public ::testing::Test {
 };
 
 #ifdef USE_CEL_PARSER
+TEST_F(CELFormatterTest, TestNodeId) {
+  auto cel_parser = std::make_unique<CELFormatterCommandParser>(context_.server_factory_context_);
+  absl::optional<size_t> max_length = absl::nullopt;
+  auto formatter = cel_parser->parse("CEL", "xds.node.id", max_length);
+  EXPECT_THAT(formatter->formatValueWithContext(formatter_context_, stream_info_),
+              ProtoEq(ValueUtil::stringValue("node_name")));
+}
+
 TEST_F(CELFormatterTest, TestFormatValue) {
   auto cel_parser = std::make_unique<CELFormatterCommandParser>(context_.server_factory_context_);
   absl::optional<size_t> max_length = absl::nullopt;