diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto index 6b230536cd86e..f171068aaeedc 100644 --- a/api/envoy/config/bootstrap/v3/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3/bootstrap.proto @@ -41,7 +41,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // ` for more detail. // Bootstrap :ref:`configuration overview `. -// [#next-free-field: 38] +// [#next-free-field: 39] message Bootstrap { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v2.Bootstrap"; @@ -101,6 +101,24 @@ message Bootstrap { core.v3.ApiConfigSource ads_config = 3; } + message ApplicationLogConfig { + message LogFormat { + oneof log_format { + option (validate.required) = true; + + // Flush application logs in JSON format. The configured JSON struct can + // support all the format flags specified in the :option:`--log-format` + // command line options section, except for the ``%v`` and ``%_`` flags. + google.protobuf.Struct json_format = 1; + } + } + + // Optional field to set the application logs format. If this field is set, it will override + // the default log format. Setting both this field and :option:`--log-format` command line + // option is not allowed, and will cause a bootstrap error. + LogFormat log_format = 1; + } + reserved 10, 11; reserved "runtime"; @@ -360,6 +378,9 @@ message Bootstrap { // Envoy only supports ListenerManager for this field and Envoy Mobile // supports ApiListenerManager. core.v3.TypedExtensionConfig listener_manager = 37; + + // Optional application log configuration. + ApplicationLogConfig application_log_config = 38; } // Administration interface :ref:`operations documentation diff --git a/changelogs/current.yaml b/changelogs/current.yaml index aff04bd00c7de..efdbc4904c2d3 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -284,6 +284,11 @@ new_features: change: | added new field ``envoy.extensions.filters.http.fault.v3.HTTPFault.filter_metadata`` to aid in logging. Metadata will be stored in StreamInfo dynamic metadata under a namespace corresponding to the name of the fault filter. +- area: application_logs + change: | + Added bootstrap option + :ref:`application_log_format ` + to enable setting application log format as JSON structure. - area: ext_proc change: | added new field ``filter_metadata `: * The ``--log-level`` flag can be set to control the log severity logged to Stackdriver. `Reference documentation `_ for Stackdriver on GKE. + +Printing logs in JSON format +---------------------------- + +It is possible to use the bootstrap config :ref:`json_format ` +to print the logs in custom JSON format. The json format struct can support all the format flags that are specified in :ref:`command line options `, +except for the ``%v`` and ``%_`` flags, as they may break the JSON structure log. Instead, use the ``%j`` flag. Example: + +.. code-block:: yaml + + application_log_config: + log_format: + json_format: + Timestamp: "%Y-%m-%dT%T.%F" + ThreadId: "%t" + SourceLine: "%s:%#" + Level: "%l" + Message: "%j" + FixedValue: "SomeFixedValue" + +.. note:: + Setting both ``application_log_config.log_format`` and CLI option ``--log-format`` is not allowed, and will cause a bootstrap error. diff --git a/envoy/server/options.h b/envoy/server/options.h index ab90efdd1f0eb..327da389107a8 100644 --- a/envoy/server/options.h +++ b/envoy/server/options.h @@ -169,6 +169,11 @@ class Options { */ virtual const std::string& logFormat() const PURE; + /** + * @return whether or not a log format was set by CLI option. + */ + virtual bool logFormatSet() const PURE; + /** * @return const bool indicating whether to escape c-style escape sequences in logs. */ diff --git a/mobile/test/common/integration/client_integration_test.cc b/mobile/test/common/integration/client_integration_test.cc index ac06e467d150b..13b2e3997e89d 100644 --- a/mobile/test/common/integration/client_integration_test.cc +++ b/mobile/test/common/integration/client_integration_test.cc @@ -97,7 +97,7 @@ void ClientIntegrationTest::trickleTest() { stream_prototype_->setOnData([this](envoy_data c_data, bool) { if (explicit_flow_control_) { - // Allow reading up to 100 bytes + // Allow reading up to 100 bytes. stream_->readData(100); } cc_.on_data_calls++; diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 2f45fc1c671cb..7c00df4467577 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -211,6 +211,7 @@ envoy_cc_library( ":lock_guard_lib", ":macros", ":non_copyable", + "//source/common/protobuf:protobuf", ] + select({ "//bazel:android_logger": ["logger_impl_lib_android"], "//conditions:default": ["logger_impl_lib_standard"], diff --git a/source/common/common/logger.cc b/source/common/common/logger.cc index 9b02e67496605..476f2e06a9f9a 100644 --- a/source/common/common/logger.cc +++ b/source/common/common/logger.cc @@ -254,6 +254,31 @@ void Registry::setLogFormat(const std::string& log_format) { } } +absl::Status Registry::setJsonLogFormat(const Protobuf::Message& log_format_struct) { + Protobuf::util::JsonPrintOptions json_options; + json_options.preserve_proto_field_names = true; + json_options.always_print_primitive_fields = true; + + std::string format_as_json; + const auto status = + Protobuf::util::MessageToJsonString(log_format_struct, &format_as_json, json_options); + + if (!status.ok()) { + return absl::InvalidArgumentError("Provided struct cannot be serialized as JSON string"); + } + + if (format_as_json.find("%v") != std::string::npos) { + return absl::InvalidArgumentError("Usage of %v is unavailable for JSON log formats"); + } + + if (format_as_json.find("%_") != std::string::npos) { + return absl::InvalidArgumentError("Usage of %_ is unavailable for JSON log formats"); + } + + setLogFormat(format_as_json); + return absl::OkStatus(); +} + Logger* Registry::logger(const std::string& log_name) { Logger* logger_to_return = nullptr; for (Logger& logger : loggers()) { diff --git a/source/common/common/logger.h b/source/common/common/logger.h index 7d7b86f29f2cf..28aecaf86640a 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -15,6 +15,7 @@ #include "source/common/common/logger_impl.h" #include "source/common/common/macros.h" #include "source/common/common/non_copyable.h" +#include "source/common/protobuf/protobuf.h" #include "absl/container/flat_hash_map.h" #include "absl/strings/string_view.h" @@ -353,6 +354,11 @@ class Registry { */ static void setLogFormat(const std::string& log_format); + /** + * Sets the log format from a struct as a JSON string. + */ + static absl::Status setJsonLogFormat(const Protobuf::Message& log_format_struct); + /** * @return std::vector& the installed loggers. */ diff --git a/source/server/BUILD b/source/server/BUILD index a5709a05c6e1d..fc81eafc00f38 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -513,7 +513,9 @@ envoy_cc_library( hdrs = ["utils.h"], deps = [ "//envoy/init:manager_interface", + "//envoy/server:options_interface", "//source/common/common:assert_lib", "@envoy_api//envoy/admin/v3:pkg_cc_proto", + "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", ], ) diff --git a/source/server/config_validation/BUILD b/source/server/config_validation/BUILD index 5d79fefce13a4..f5e8bd30b1d90 100644 --- a/source/server/config_validation/BUILD +++ b/source/server/config_validation/BUILD @@ -101,6 +101,7 @@ envoy_cc_library( "//source/common/version:version_lib", "//source/server:configuration_lib", "//source/server:server_lib", + "//source/server:utils_lib", "//source/server/admin:admin_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index c836e4cd1d1ae..dc80c3e7145be 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -15,6 +15,7 @@ #include "source/server/listener_manager_factory.h" #include "source/server/regex_engine.h" #include "source/server/ssl_context_manager.h" +#include "source/server/utils.h" namespace Envoy { namespace Server { @@ -85,6 +86,11 @@ void ValidationInstance::initialize(const Options& options, InstanceUtil::loadBootstrapConfig(bootstrap_, options, messageValidationContext().staticValidationVisitor(), *api_); + if (bootstrap_.has_application_log_config()) { + Utility::assertExclusiveLogFormatMethod(options_, bootstrap_.application_log_config()); + Utility::maybeSetApplicationLogFormat(bootstrap_.application_log_config()); + } + // Inject regex engine to singleton. Regex::EnginePtr regex_engine = createRegexEngine( bootstrap_, messageValidationContext().staticValidationVisitor(), serverFactoryContext()); diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index eae78c65d513c..6345c789e8973 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -194,6 +194,7 @@ OptionsImpl::OptionsImpl(std::vector args, } log_format_ = log_format.getValue(); + log_format_set_ = log_format.isSet(); log_format_escaped_ = log_format_escaped.getValue(); enable_fine_grain_logging_ = enable_fine_grain_logging.getValue(); diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 6528e0aeb8a10..d02f37dce5bc1 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -78,7 +78,10 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable> component_log_levels_; std::string component_log_level_str_; std::string log_format_{Logger::Logger::DEFAULT_LOG_FORMAT}; + bool log_format_set_{false}; bool log_format_escaped_{false}; std::string log_path_; uint64_t restart_epoch_{0}; diff --git a/source/server/server.cc b/source/server/server.cc index f5c2b062ea8b5..8ed78eb8b139c 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -422,6 +422,11 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add messageValidationContext().staticValidationVisitor(), *api_); bootstrap_config_update_time_ = time_source_.systemTime(); + if (bootstrap_.has_application_log_config()) { + Utility::assertExclusiveLogFormatMethod(options_, bootstrap_.application_log_config()); + Utility::maybeSetApplicationLogFormat(bootstrap_.application_log_config()); + } + #ifdef ENVOY_PERFETTO perfetto::TracingInitArgs args; // Include in-process events only. diff --git a/source/server/utils.cc b/source/server/utils.cc index fcb5e043ec999..7bb8957a59c67 100644 --- a/source/server/utils.cc +++ b/source/server/utils.cc @@ -1,5 +1,7 @@ #include "source/server/utils.h" +#include "envoy/common/exception.h" + #include "source/common/common/assert.h" namespace Envoy { @@ -21,6 +23,28 @@ envoy::admin::v3::ServerInfo::State serverState(Init::Manager::State state, return envoy::admin::v3::ServerInfo::PRE_INITIALIZING; } +void assertExclusiveLogFormatMethod( + const Options& options, + const envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig& application_log_config) { + if (options.logFormatSet() && application_log_config.has_log_format()) { + throw EnvoyException( + "Only one of ApplicationLogConfig.log_format or CLI option --log-format can be specified."); + } +} + +void maybeSetApplicationLogFormat( + const envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig& application_log_config) { + if (application_log_config.has_log_format() && + application_log_config.log_format().has_json_format()) { + const auto status = + Logger::Registry::setJsonLogFormat(application_log_config.log_format().json_format()); + + if (!status.ok()) { + throw EnvoyException(fmt::format("setJsonLogFormat error: {}", status.ToString())); + } + } +} + } // namespace Utility } // namespace Server } // namespace Envoy diff --git a/source/server/utils.h b/source/server/utils.h index 1ab4b51de275c..2d3b981c2c87d 100644 --- a/source/server/utils.h +++ b/source/server/utils.h @@ -1,7 +1,9 @@ #pragma once #include "envoy/admin/v3/server_info.pb.h" +#include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/init/manager.h" +#include "envoy/server/options.h" namespace Envoy { namespace Server { @@ -14,6 +16,13 @@ namespace Utility { envoy::admin::v3::ServerInfo::State serverState(Init::Manager::State state, bool health_check_failed); +void assertExclusiveLogFormatMethod( + const Options& options, + const envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig& application_log_config); + +void maybeSetApplicationLogFormat( + const envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig& application_log_config); + } // namespace Utility } // namespace Server } // namespace Envoy diff --git a/test/common/common/logger_test.cc b/test/common/common/logger_test.cc index b026eddc6419b..e9998db311e3b 100644 --- a/test/common/common/logger_test.cc +++ b/test/common/common/logger_test.cc @@ -4,6 +4,7 @@ #include "source/common/common/json_escape_string.h" #include "source/common/common/logger.h" +#include "test/mocks/common.h" #include "test/test_common/environment.h" #include "gmock/gmock.h" @@ -176,17 +177,6 @@ TEST_P(LoggerCustomFlagsTest, LogMessageAsJsonStringEscaped) { "StreamAggregatedResources gRPC config stream closed: 14, connection error: desc = " "\\\"transport: Error while dialing dial tcp [::1]:15012: connect: connection refused\\\""); } - -struct MockLogSink : SinkDelegate { - MockLogSink(DelegatingLogSinkSharedPtr log_sink) : SinkDelegate(log_sink) { setDelegate(); } - ~MockLogSink() override { restoreDelegate(); } - - MOCK_METHOD(void, log, (absl::string_view, const spdlog::details::log_msg&)); - MOCK_METHOD(void, logWithStableName, - (absl::string_view, absl::string_view, absl::string_view, absl::string_view)); - void flush() override {} -}; - class NamedLogTest : public Loggable, public testing::Test {}; TEST_F(NamedLogTest, NamedLogsAreSentToSink) { @@ -270,6 +260,130 @@ TEST(LoggerTest, LogWithLogDetails) { ENVOY_LOG_MISC(info, "hello"); } +TEST(LoggerTest, TestJsonFormatError) { + ProtobufWkt::Any log_struct; + log_struct.set_type_url("type.googleapis.com/bad.type.url"); + log_struct.set_value("asdf"); + + // This scenario shouldn't happen in production, the test is added mainly for coverage. + auto status = Envoy::Logger::Registry::setJsonLogFormat(log_struct); + EXPECT_FALSE(status.ok()); + EXPECT_EQ("INVALID_ARGUMENT: Provided struct cannot be serialized as JSON string", + status.ToString()); +} + +TEST(LoggerTest, TestJsonFormatNonEscapedThrows) { + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + + { + ProtobufWkt::Struct log_struct; + (*log_struct.mutable_fields())["Message"].set_string_value("%v"); + (*log_struct.mutable_fields())["NullField"].set_null_value(ProtobufWkt::NULL_VALUE); + + auto status = Envoy::Logger::Registry::setJsonLogFormat(log_struct); + EXPECT_FALSE(status.ok()); + EXPECT_EQ("INVALID_ARGUMENT: Usage of %v is unavailable for JSON log formats", + status.ToString()); + } + + { + ProtobufWkt::Struct log_struct; + (*log_struct.mutable_fields())["Message"].set_string_value("%_"); + (*log_struct.mutable_fields())["NullField"].set_null_value(ProtobufWkt::NULL_VALUE); + + auto status = Envoy::Logger::Registry::setJsonLogFormat(log_struct); + EXPECT_FALSE(status.ok()); + EXPECT_EQ("INVALID_ARGUMENT: Usage of %_ is unavailable for JSON log formats", + status.ToString()); + } +} + +TEST(LoggerTest, TestJsonFormatEmptyStruct) { + ProtobufWkt::Struct log_struct; + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + EXPECT_TRUE(Envoy::Logger::Registry::setJsonLogFormat(log_struct).ok()); + + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)).WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_THAT(msg, HasSubstr("{}")); + EXPECT_EQ(log.logger_name, "misc"); + })); + + ENVOY_LOG_MISC(info, "hello"); +} + +TEST(LoggerTest, TestJsonFormatNullAndFixedField) { + ProtobufWkt::Struct log_struct; + (*log_struct.mutable_fields())["Message"].set_string_value("%j"); + (*log_struct.mutable_fields())["FixedValue"].set_string_value("Fixed"); + (*log_struct.mutable_fields())["NullField"].set_null_value(ProtobufWkt::NULL_VALUE); + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + EXPECT_TRUE(Envoy::Logger::Registry::setJsonLogFormat(log_struct).ok()); + + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)).WillOnce(Invoke([](auto msg, auto&) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("\"Message\":\"hello\"")); + EXPECT_THAT(msg, HasSubstr("\"FixedValue\":\"Fixed\"")); + EXPECT_THAT(msg, HasSubstr("\"NullField\":null")); + })); + + ENVOY_LOG_MISC(info, "hello"); +} + +TEST(LoggerTest, TestJsonFormat) { + ProtobufWkt::Struct log_struct; + (*log_struct.mutable_fields())["Level"].set_string_value("%l"); + (*log_struct.mutable_fields())["Message"].set_string_value("%j"); + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + EXPECT_TRUE(Envoy::Logger::Registry::setJsonLogFormat(log_struct).ok()); + + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)) + .WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("\"Level\":\"info\"")); + EXPECT_THAT(msg, HasSubstr("\"Message\":\"hello\"")); + EXPECT_EQ(log.logger_name, "misc"); + })) + .WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("\"Level\":\"info\"")); + EXPECT_THAT(msg, HasSubstr("\"Message\":\"hel\\nlo\"")); + EXPECT_EQ(log.logger_name, "misc"); + })) + .WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("\"Level\":\"info\"")); + EXPECT_THAT(msg, HasSubstr("\"Message\":\"hel\\\"lo\"")); + EXPECT_EQ(log.logger_name, "misc"); + })); + + ENVOY_LOG_MISC(info, "hello"); + ENVOY_LOG_MISC(info, "hel\nlo"); + ENVOY_LOG_MISC(info, "hel\"lo"); +} + +TEST(LoggerTest, TestJsonFormatWithNestedJsonMessage) { + ProtobufWkt::Struct log_struct; + (*log_struct.mutable_fields())["Level"].set_string_value("%l"); + (*log_struct.mutable_fields())["Message"].set_string_value("%j"); + (*log_struct.mutable_fields())["FixedValue"].set_string_value("Fixed"); + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + EXPECT_TRUE(Envoy::Logger::Registry::setJsonLogFormat(log_struct).ok()); + + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)).WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("\"Level\":\"info\"")); + EXPECT_THAT(msg, HasSubstr("\"Message\":\"{\\\"nested_message\\\":\\\"hello\\\"}\"")); + EXPECT_THAT(msg, HasSubstr("\"FixedValue\":\"Fixed\"")); + EXPECT_EQ(log.logger_name, "misc"); + })); + + ENVOY_LOG_MISC(info, "{\"nested_message\":\"hello\"}"); +} + } // namespace } // namespace Logger } // namespace Envoy diff --git a/test/mocks/common.h b/test/mocks/common.h index 058a7736af69f..1e496030e4f9b 100644 --- a/test/mocks/common.h +++ b/test/mocks/common.h @@ -143,4 +143,16 @@ class MockKeyValueStoreFactory : public KeyValueStoreFactory { std::string name() const override { return "mock_key_value_store_factory"; } }; +struct MockLogSink : Logger::SinkDelegate { + MockLogSink(Logger::DelegatingLogSinkSharedPtr log_sink) : Logger::SinkDelegate(log_sink) { + setDelegate(); + } + ~MockLogSink() override { restoreDelegate(); } + + MOCK_METHOD(void, log, (absl::string_view, const spdlog::details::log_msg&)); + MOCK_METHOD(void, logWithStableName, + (absl::string_view, absl::string_view, absl::string_view, absl::string_view)); + void flush() override {} +}; + } // namespace Envoy diff --git a/test/mocks/server/options.h b/test/mocks/server/options.h index fb64f4f1b6642..0a0f7433124c3 100644 --- a/test/mocks/server/options.h +++ b/test/mocks/server/options.h @@ -34,6 +34,7 @@ class MockOptions : public Options { MOCK_METHOD((const std::vector>&), componentLogLevels, (), (const)); MOCK_METHOD(const std::string&, logFormat, (), (const)); + MOCK_METHOD(bool, logFormatSet, (), (const)); MOCK_METHOD(bool, logFormatEscaped, (), (const)); MOCK_METHOD(bool, enableFineGrainLogging, (), (const)); MOCK_METHOD(const std::string&, logPath, (), (const)); diff --git a/test/server/BUILD b/test/server/BUILD index 74ae0259db08f..e2c210dc293e0 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -397,5 +397,6 @@ envoy_cc_test( srcs = envoy_select_admin_functionality(["utils_test.cc"]), deps = [ "//source/server:utils_lib", + "//test/mocks/server:options_mocks", ], ) diff --git a/test/server/config_validation/server_test.cc b/test/server/config_validation/server_test.cc index d52e83202b07d..cda36802a3dd8 100644 --- a/test/server/config_validation/server_test.cc +++ b/test/server/config_validation/server_test.cc @@ -6,6 +6,7 @@ #include "source/server/config_validation/server.h" #include "test/integration/server.h" +#include "test/mocks/common.h" #include "test/mocks/network/mocks.h" #include "test/mocks/server/options.h" #include "test/mocks/stats/mocks.h" @@ -14,6 +15,9 @@ #include "test/test_common/registry.h" #include "test/test_common/test_time.h" +using testing::HasSubstr; +using testing::Return; + namespace Envoy { namespace Server { namespace { @@ -73,14 +77,54 @@ class RuntimeFeatureValidationServerTest : public ValidationServerTest { static const std::vector getAllConfigFiles() { setupTestDirectory(); + return {"runtime_config.yaml"}; + } +}; - auto files = TestUtility::listFiles(ValidationServerTest::directory_, false); +class JsonApplicationLogsValidationServerTest : public ValidationServerTest { +public: + static void SetUpTestSuite() { // NOLINT(readability-identifier-naming) + setupTestDirectory(); + } - // Strip directory part. options_ adds it for each test. - for (auto& file : files) { - file = file.substr(directory_.length() + 1); - } - return files; + static void setupTestDirectory() { + directory_ = + TestEnvironment::runfilesDirectory("envoy/test/server/config_validation/test_data/"); + } + + static const std::vector getAllConfigFiles() { + setupTestDirectory(); + return {"json_application_logs.yaml"}; + } +}; + +class JsonApplicationLogsValidationServerForbiddenFlagvTest : public ValidationServerTest { +public: + static void SetUpTestSuite() { // NOLINT(readability-identifier-naming) + setupTestDirectory(); + } + static void setupTestDirectory() { + directory_ = + TestEnvironment::runfilesDirectory("envoy/test/server/config_validation/test_data/"); + } + static const std::vector getAllConfigFiles() { + setupTestDirectory(); + return {"json_application_logs_forbidden_flagv.yaml"}; + } +}; + +class JsonApplicationLogsValidationServerForbiddenFlag_Test : public ValidationServerTest { +public: + static void SetUpTestSuite() { // NOLINT(readability-identifier-naming) + setupTestDirectory(); + } + static void setupTestDirectory() { + directory_ = + TestEnvironment::runfilesDirectory("envoy/test/server/config_validation/test_data/"); + } + static const std::vector getAllConfigFiles() { + setupTestDirectory(); + return {"json_application_logs_forbidden_flag_.yaml"}; } }; @@ -206,6 +250,80 @@ INSTANTIATE_TEST_SUITE_P( AllConfigs, RuntimeFeatureValidationServerTest, ::testing::ValuesIn(RuntimeFeatureValidationServerTest::getAllConfigFiles())); +TEST_P(JsonApplicationLogsValidationServerTest, BootstrapApplicationLogsAndCLIThrows) { + Thread::MutexBasicLockable access_log_lock; + Stats::IsolatedStoreImpl stats_store; + DangerousDeprecatedTestTime time_system; + EXPECT_CALL(options_, logFormatSet()).WillRepeatedly(Return(true)); + EXPECT_THROW_WITH_MESSAGE( + ValidationInstance server(options_, time_system.timeSystem(), + Network::Address::InstanceConstSharedPtr(), stats_store, + access_log_lock, component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()), + EnvoyException, + "Only one of ApplicationLogConfig.log_format or CLI option --log-format can be specified."); +} + +TEST_P(JsonApplicationLogsValidationServerTest, JsonApplicationLogs) { + Thread::MutexBasicLockable access_log_lock; + Stats::IsolatedStoreImpl stats_store; + DangerousDeprecatedTestTime time_system; + ValidationInstance server(options_, time_system.timeSystem(), + Network::Address::InstanceConstSharedPtr(), stats_store, + access_log_lock, component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()); + + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)).WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_THAT(msg, HasSubstr("{\"MessageFromProto\":\"hello\"}")); + EXPECT_EQ(log.logger_name, "misc"); + })); + + ENVOY_LOG_MISC(info, "hello"); + server.shutdown(); +} + +INSTANTIATE_TEST_SUITE_P( + AllConfigs, JsonApplicationLogsValidationServerTest, + ::testing::ValuesIn(JsonApplicationLogsValidationServerTest::getAllConfigFiles())); + +TEST_P(JsonApplicationLogsValidationServerForbiddenFlagvTest, TestForbiddenFlag) { + Thread::MutexBasicLockable access_log_lock; + Stats::IsolatedStoreImpl stats_store; + DangerousDeprecatedTestTime time_system; + EXPECT_THROW_WITH_MESSAGE( + ValidationInstance server(options_, time_system.timeSystem(), + Network::Address::InstanceConstSharedPtr(), stats_store, + access_log_lock, component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()), + EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %v is unavailable for JSON log formats"); +} + +INSTANTIATE_TEST_SUITE_P( + AllConfigs, JsonApplicationLogsValidationServerForbiddenFlagvTest, + ::testing::ValuesIn( + JsonApplicationLogsValidationServerForbiddenFlagvTest::getAllConfigFiles())); + +TEST_P(JsonApplicationLogsValidationServerForbiddenFlag_Test, TestForbiddenFlag) { + Thread::MutexBasicLockable access_log_lock; + Stats::IsolatedStoreImpl stats_store; + DangerousDeprecatedTestTime time_system; + EXPECT_THROW_WITH_MESSAGE( + ValidationInstance server(options_, time_system.timeSystem(), + Network::Address::InstanceConstSharedPtr(), stats_store, + access_log_lock, component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()), + EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %_ is unavailable for JSON log formats"); +} + +INSTANTIATE_TEST_SUITE_P( + AllConfigs, JsonApplicationLogsValidationServerForbiddenFlag_Test, + ::testing::ValuesIn( + JsonApplicationLogsValidationServerForbiddenFlag_Test::getAllConfigFiles())); + } // namespace } // namespace Server } // namespace Envoy diff --git a/test/server/config_validation/test_data/json_application_logs.yaml b/test/server/config_validation/test_data/json_application_logs.yaml new file mode 100644 index 0000000000000..2510444d98408 --- /dev/null +++ b/test/server/config_validation/test_data/json_application_logs.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%j" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/config_validation/test_data/json_application_logs_forbidden_flag_.yaml b/test/server/config_validation/test_data/json_application_logs_forbidden_flag_.yaml new file mode 100644 index 0000000000000..3b3b8164adaea --- /dev/null +++ b/test/server/config_validation/test_data/json_application_logs_forbidden_flag_.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%_" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/config_validation/test_data/json_application_logs_forbidden_flagv.yaml b/test/server/config_validation/test_data/json_application_logs_forbidden_flagv.yaml new file mode 100644 index 0000000000000..16474cea24cd4 --- /dev/null +++ b/test/server/config_validation/test_data/json_application_logs_forbidden_flagv.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%v" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 79136c60e55e7..b09db966bcc08 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -113,6 +113,7 @@ TEST_F(OptionsImplTest, All) { EXPECT_EQ(spdlog::level::info, options->logLevel()); EXPECT_EQ(2, options->componentLogLevels().size()); EXPECT_EQ("[%v]", options->logFormat()); + EXPECT_TRUE(options->logFormatSet()); EXPECT_EQ("/foo/bar", options->logPath()); EXPECT_EQ(true, options->enableFineGrainLogging()); EXPECT_EQ("cluster", options->serviceClusterName()); @@ -209,6 +210,7 @@ TEST_F(OptionsImplTest, SetAll) { EXPECT_EQ(Server::DrainStrategy::Immediate, options->drainStrategy()); EXPECT_EQ(spdlog::level::trace, options->logLevel()); EXPECT_EQ("%L %n %v", options->logFormat()); + EXPECT_TRUE(options->logFormatSet()); EXPECT_EQ("/foo/bar", options->logPath()); EXPECT_EQ(std::chrono::seconds(43), options->parentShutdownTime()); EXPECT_EQ(44, options->restartEpoch()); @@ -528,18 +530,21 @@ TEST_F(OptionsImplTest, SetCpusetOnly) { TEST_F(OptionsImplTest, LogFormatDefault) { std::unique_ptr options = createOptionsImpl({"envoy", "-c", "hello"}); EXPECT_EQ(options->logFormat(), "[%Y-%m-%d %T.%e][%t][%l][%n] [%g:%#] %v"); + EXPECT_FALSE(options->logFormatSet()); } TEST_F(OptionsImplTest, LogFormatOverride) { std::unique_ptr options = createOptionsImpl({"envoy", "-c", "hello", "--log-format", "%%v %v %t %v"}); EXPECT_EQ(options->logFormat(), "%%v %v %t %v"); + EXPECT_TRUE(options->logFormatSet()); } TEST_F(OptionsImplTest, LogFormatOverrideNoPrefix) { std::unique_ptr options = createOptionsImpl({"envoy", "-c", "hello", "--log-format", "%%v %v %t %v"}); EXPECT_EQ(options->logFormat(), "%%v %v %t %v"); + EXPECT_TRUE(options->logFormatSet()); } // Test that --base-id and --restart-epoch with non-default values are accepted. diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 59cbd75f3644f..c47df5d57c57f 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -20,6 +20,7 @@ #include "test/common/stats/stat_test_utility.h" #include "test/config/v2_link_hacks.h" #include "test/integration/server.h" +#include "test/mocks/common.h" #include "test/mocks/server/bootstrap_extension_factory.h" #include "test/mocks/server/fatal_action_factory.h" #include "test/mocks/server/hot_restart.h" @@ -1624,6 +1625,41 @@ TEST_P(ServerInstanceImplTest, AdminAccessLogFilter) { EXPECT_NO_THROW(initialize("test/server/test_data/server/access_log_filter_bootstrap.yaml")); } +TEST_P(ServerInstanceImplTest, BootstrapApplicationLogsAndCLIThrows) { + EXPECT_CALL(options_, logFormatSet()).WillRepeatedly(Return(true)); + EXPECT_THROW_WITH_MESSAGE( + initialize("test/server/test_data/server/json_application_log.yaml"), EnvoyException, + "Only one of ApplicationLogConfig.log_format or CLI option --log-format can be specified."); +} + +TEST_P(ServerInstanceImplTest, JsonApplicationLog) { + EXPECT_NO_THROW(initialize("test/server/test_data/server/json_application_log.yaml")); + + Envoy::Logger::Registry::setLogLevel(spdlog::level::info); + MockLogSink sink(Envoy::Logger::Registry::getSink()); + EXPECT_CALL(sink, log(_, _)).WillOnce(Invoke([](auto msg, auto& log) { + EXPECT_NO_THROW(Json::Factory::loadFromString(std::string(msg))); + EXPECT_THAT(msg, HasSubstr("{\"MessageFromProto\":\"hello\"}")); + EXPECT_EQ(log.logger_name, "misc"); + })); + + ENVOY_LOG_MISC(info, "hello"); +} + +TEST_P(ServerInstanceImplTest, JsonApplicationLogFailWithForbiddenFlagv) { + EXPECT_THROW_WITH_MESSAGE( + initialize("test/server/test_data/server/json_application_log_forbidden_flagv.yaml"), + EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %v is unavailable for JSON log formats"); +} + +TEST_P(ServerInstanceImplTest, JsonApplicationLogFailWithForbiddenFlag_) { + EXPECT_THROW_WITH_MESSAGE( + initialize("test/server/test_data/server/json_application_log_forbidden_flag_.yaml"), + EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %_ is unavailable for JSON log formats"); +} + } // namespace } // namespace Server } // namespace Envoy diff --git a/test/server/test_data/server/json_application_log.yaml b/test/server/test_data/server/json_application_log.yaml new file mode 100644 index 0000000000000..2510444d98408 --- /dev/null +++ b/test/server/test_data/server/json_application_log.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%j" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/test_data/server/json_application_log_forbidden_flag_.yaml b/test/server/test_data/server/json_application_log_forbidden_flag_.yaml new file mode 100644 index 0000000000000..3b3b8164adaea --- /dev/null +++ b/test/server/test_data/server/json_application_log_forbidden_flag_.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%_" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/test_data/server/json_application_log_forbidden_flagv.yaml b/test/server/test_data/server/json_application_log_forbidden_flagv.yaml new file mode 100644 index 0000000000000..16474cea24cd4 --- /dev/null +++ b/test/server/test_data/server/json_application_log_forbidden_flagv.yaml @@ -0,0 +1,11 @@ +--- +application_log_config: + log_format: + json_format: + MessageFromProto: "%v" + +admin: + address: + socket_address: + address: 0.0.0.0 + port_value: 9000 diff --git a/test/server/utils_test.cc b/test/server/utils_test.cc index 1e2ae258f5a02..7576e41522c1b 100644 --- a/test/server/utils_test.cc +++ b/test/server/utils_test.cc @@ -1,9 +1,12 @@ #include "source/server/utils.h" +#include "test/mocks/server/options.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" +using testing::Return; + namespace Envoy { namespace Server { namespace Utility { @@ -15,6 +18,76 @@ TEST(UtilsTest, BadServerState) { EXPECT_ENVOY_BUG(Utility::serverState(static_cast(123), true), "unexpected server state"); } + +TEST(UtilsTest, AssertExclusiveLogFormatMethod) { + { + testing::NiceMock options; + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + EXPECT_NO_THROW(Utility::assertExclusiveLogFormatMethod(options, log_config)); + } + + { + testing::NiceMock options; + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + EXPECT_CALL(options, logFormatSet()).WillRepeatedly(Return(true)); + EXPECT_NO_THROW(Utility::assertExclusiveLogFormatMethod(options, log_config)); + } + + { + testing::NiceMock options; + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + log_config.mutable_log_format(); + EXPECT_NO_THROW(Utility::assertExclusiveLogFormatMethod(options, log_config)); + } + + { + testing::NiceMock options; + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + EXPECT_CALL(options, logFormatSet()).WillRepeatedly(Return(true)); + log_config.mutable_log_format(); + EXPECT_THROW_WITH_MESSAGE( + Utility::assertExclusiveLogFormatMethod(options, log_config), EnvoyException, + "Only one of ApplicationLogConfig.log_format or CLI option --log-format can be specified."); + } +} + +TEST(UtilsTest, MaybeSetApplicationLogFormat) { + { + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + EXPECT_NO_THROW(Utility::maybeSetApplicationLogFormat(log_config)); + } + + { + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + log_config.mutable_log_format(); + EXPECT_NO_THROW(Utility::maybeSetApplicationLogFormat(log_config)); + } + + { + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + log_config.mutable_log_format()->mutable_json_format(); + EXPECT_NO_THROW(Utility::maybeSetApplicationLogFormat(log_config)); + } + + { + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + auto* format = log_config.mutable_log_format()->mutable_json_format(); + format->mutable_fields()->operator[]("Message").set_string_value("%v"); + EXPECT_THROW_WITH_MESSAGE(Utility::maybeSetApplicationLogFormat(log_config), EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %v is " + "unavailable for JSON log formats"); + } + + { + envoy::config::bootstrap::v3::Bootstrap::ApplicationLogConfig log_config; + auto* format = log_config.mutable_log_format()->mutable_json_format(); + format->mutable_fields()->operator[]("Message").set_string_value("%_"); + EXPECT_THROW_WITH_MESSAGE(Utility::maybeSetApplicationLogFormat(log_config), EnvoyException, + "setJsonLogFormat error: INVALID_ARGUMENT: Usage of %_ is " + "unavailable for JSON log formats"); + } +} + } // namespace Utility } // namespace Server } // namespace Envoy diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index fed59e9247439..f9c7f231773c8 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -208,6 +208,7 @@ paths: - source/server/config_validation/server.cc - source/server/admin/html/active_stats.js - source/server/server.cc + - source/server/utils.cc - source/server/configuration_impl.h - source/server/hot_restarting_base.cc - source/server/hot_restart_impl.cc