logging: add LambdaDelegate (#1354)

Description: adds a new SinkDelegate called LambdaDelegate which uses user-provided callbacks to drive log functionality. Risk Level: low Testing: new unit tests and integration test in main_interface_test. Signed-off-by: Jose Nino <[email protected]> Signed-off-by: JP Simard <[email protected]>
envoyproxy · Nov 28, 2022 · cb32693 · cb32693
1 parent 2bcf617
commit cb32693
Show file tree

Hide file tree

Showing 9 changed files with 162 additions and 18 deletions.
diff --git a/mobile/library/common/BUILD b/mobile/library/common/BUILD
@@ -26,7 +26,7 @@ envoy_cc_library(
     repository = "@envoy",
     deps = [
         ":envoy_mobile_main_common_lib",
-        # "//library/common/common:lambda_logger_delegate_lib",
+        "//library/common/common:lambda_logger_delegate_lib",
         "//library/common/data:utility_lib",
         "//library/common/http:dispatcher_lib",
         "//library/common/http:header_utility_lib",

diff --git a/mobile/library/common/common/BUILD b/mobile/library/common/common/BUILD
@@ -0,0 +1,18 @@
+load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_library", "envoy_package")
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+envoy_cc_library(
+    name = "lambda_logger_delegate_lib",
+    srcs = ["lambda_logger_delegate.cc"],
+    hdrs = ["lambda_logger_delegate.h"],
+    repository = "@envoy",
+    deps = [
+        "//library/common/data:utility_lib",
+        "//library/common/types:c_types_lib",
+        "@envoy//source/common/common:macros",
+        "@envoy//source/common/common:minimal_logger_lib",
+    ],
+)
diff --git a/mobile/library/common/common/lambda_logger_delegate.cc b/mobile/library/common/common/lambda_logger_delegate.cc
@@ -0,0 +1,22 @@
+#include "library/common/common/lambda_logger_delegate.h"
+
+#include <iostream>
+
+#include "library/common/data/utility.h"
+
+namespace Envoy {
+namespace Logger {
+
+LambdaDelegate::LambdaDelegate(envoy_logger logger, DelegatingLogSinkSharedPtr log_sink)
+    : SinkDelegate(log_sink), logger_(logger) {
+  setDelegate();
+}
+
+LambdaDelegate::~LambdaDelegate() { restoreDelegate(); }
+
+void LambdaDelegate::log(absl::string_view msg) {
+  logger_.log(Data::Utility::copyToBridgeData(msg), logger_.context);
+}
+
+} // namespace Logger
+} // namespace Envoy
diff --git a/mobile/library/common/common/lambda_logger_delegate.h b/mobile/library/common/common/lambda_logger_delegate.h
@@ -0,0 +1,30 @@
+#pragma once
+
+#include <string>
+
+#include "common/common/logger.h"
+
+#include "absl/strings/string_view.h"
+#include "library/common/types/c_types.h"
+
+namespace Envoy {
+namespace Logger {
+
+class LambdaDelegate : public SinkDelegate {
+public:
+  LambdaDelegate(envoy_logger logger, DelegatingLogSinkSharedPtr log_sink);
+  ~LambdaDelegate() override;
+
+  // SinkDelegate
+  void log(absl::string_view msg) override;
+  // Currently unexposed. May be desired in the future.
+  void flush() override{};
+
+private:
+  envoy_logger logger_;
+};
+
+using LambdaDelegatePtr = std::unique_ptr<LambdaDelegate>;
+
+} // namespace Logger
+} // namespace Envoy
diff --git a/mobile/library/common/engine.cc b/mobile/library/common/engine.cc
@@ -9,9 +9,9 @@
 
 namespace Envoy {
 
-Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger, const char* config,
+Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger logger, const char* config,
                const char* log_level, std::atomic<envoy_network_t>& preferred_network)
-    : callbacks_(callbacks) {
+    : callbacks_(callbacks), logger_(logger) {
   // Ensure static factory registration occurs on time.
   // TODO: ensure this is only called one time once multiple Engine objects can be allocated.
   // https://github.com/lyft/envoy-mobile/issues/332
@@ -47,17 +47,10 @@ envoy_status_t Engine::run(const std::string config, const std::string log_level
 
       event_dispatcher_ = &main_common_->server()->dispatcher();
 
-      // TODO(junr03): wire up after https://github.com/envoyproxy/envoy-mobile/pull/1354 merges.
-      // Logger::LambdaDelegate::LogCb log_cb = [](absl::string_view) -> void {};
-      // if (logger_.log) {
-      //   log_cb = [this](absl::string_view msg) -> void {
-      //     logger_.log(Data::Utility::copyToBridgeData(msg),
-      //                               logger_.context);
-      //   };
-      // }
-
-      // lambda_logger_ =
-      //     std::make_unique<Logger::LambdaDelegate>(log_cb, Logger::Registry::getSink());
+      if (logger_.log) {
+        lambda_logger_ =
+            std::make_unique<Logger::LambdaDelegate>(logger_, Logger::Registry::getSink());
+      }
 
       cv_.notifyAll();
     } catch (const Envoy::NoServingException& e) {

diff --git a/mobile/library/common/engine.h b/mobile/library/common/engine.h
@@ -6,8 +6,7 @@
 
 #include "absl/base/call_once.h"
 #include "extension_registry.h"
-
-// #include "library/common/common/lambda_logger_delegate.h"
+#include "library/common/common/lambda_logger_delegate.h"
 #include "library/common/envoy_mobile_main_common.h"
 #include "library/common/http/dispatcher.h"
 #include "library/common/types/c_types.h"
@@ -89,12 +88,12 @@ class Engine {
   Stats::ScopePtr client_scope_;
   Stats::StatNameSetPtr stat_name_set_;
   envoy_engine_callbacks callbacks_;
-  // envoy_logger logger_;
+  envoy_logger logger_;
   Thread::MutexBasicLockable mutex_;
   Thread::CondVar cv_;
   std::unique_ptr<Http::Dispatcher> http_dispatcher_;
   std::unique_ptr<MobileMainCommon> main_common_ GUARDED_BY(mutex_);
-  // Logger::LambdaDelegatePtr lambda_logger_{};
+  Logger::LambdaDelegatePtr lambda_logger_{};
   Server::Instance* server_{};
   Server::ServerLifecycleNotifier::HandlePtr postinit_callback_handler_;
   Event::Dispatcher* event_dispatcher_;

diff --git a/mobile/test/common/common/BUILD b/mobile/test/common/common/BUILD
@@ -0,0 +1,16 @@
+load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_package")
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+envoy_cc_test(
+    name = "lambda_logger_delegate_test",
+    srcs = ["lambda_logger_delegate_test.cc"],
+    repository = "@envoy",
+    deps = [
+        "//library/common/common:lambda_logger_delegate_lib",
+        "//library/common/data:utility_lib",
+        "//library/common/types:c_types_lib",
+    ],
+)
diff --git a/mobile/test/common/common/lambda_logger_delegate_test.cc b/mobile/test/common/common/lambda_logger_delegate_test.cc
@@ -0,0 +1,29 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "library/common/common/lambda_logger_delegate.h"
+#include "library/common/data/utility.h"
+
+using testing::_;
+using testing::HasSubstr;
+
+namespace Envoy {
+namespace Logger {
+
+TEST(LambdaDelegate, LogCb) {
+  std::string expected_msg = "Hello LambdaDelegate";
+  std::string actual_msg;
+
+  LambdaDelegate delegate = LambdaDelegate({[](envoy_data data, void* context) -> void {
+                                              auto* actual_msg = static_cast<std::string*>(context);
+                                              *actual_msg = Data::Utility::copyToString(data);
+                                              data.release(data.context);
+                                            },
+                                            &actual_msg},
+                                           Registry::getSink());
+
+  ENVOY_LOG_MISC(error, expected_msg);
+  EXPECT_THAT(actual_msg, HasSubstr(expected_msg));
+}
+
+} // namespace Logger
+} // namespace Envoy
diff --git a/mobile/test/common/main_interface_test.cc b/mobile/test/common/main_interface_test.cc
@@ -1,11 +1,15 @@
 #include "test/common/http/common.h"
 
 #include "absl/synchronization/notification.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "library/common/data/utility.h"
 #include "library/common/http/header_utility.h"
 #include "library/common/main_interface.h"
 
+using testing::_;
+using testing::HasSubstr;
+
 namespace Envoy {
 
 typedef struct {
@@ -385,4 +389,37 @@ TEST(EngineTest, RecordHistogramValue) {
   ASSERT_TRUE(test_context.on_exit.WaitForNotificationWithTimeout(absl::Seconds(3)));
 }
 
+TEST(EngineTest, Logger) {
+  engine_test_context test_context{};
+  envoy_engine_callbacks engine_cbs{[](void* context) -> void {
+                                      auto* engine_running =
+                                          static_cast<engine_test_context*>(context);
+                                      engine_running->on_engine_running.Notify();
+                                    } /*on_engine_running*/,
+                                    [](void* context) -> void {
+                                      auto* exit = static_cast<engine_test_context*>(context);
+                                      exit->on_exit.Notify();
+                                    } /*on_exit*/,
+                                    &test_context /*context*/};
+
+  std::string actual_log;
+  envoy_logger logger{[](envoy_data data, void* context) -> void {
+                        auto* actual_log = static_cast<std::string*>(context);
+                        *actual_log = Data::Utility::copyToString(data);
+                        data.release(data.context);
+                      },
+                      &actual_log};
+
+  run_engine(0, engine_cbs, logger, MINIMAL_NOOP_CONFIG.c_str(), LEVEL_DEBUG.c_str());
+  ASSERT_TRUE(test_context.on_engine_running.WaitForNotificationWithTimeout(absl::Seconds(3)));
+
+  std::string expected_log = "logging!";
+  ENVOY_LOG_MISC(error, expected_log);
+
+  EXPECT_THAT(actual_log, HasSubstr(expected_log));
+
+  terminate_engine(0);
+  ASSERT_TRUE(test_context.on_exit.WaitForNotificationWithTimeout(absl::Seconds(3)));
+}
+
 } // namespace Envoy