diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 4f616f951122..b81af0b70599 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -5,6 +5,10 @@ behavior_changes: minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* +- area: command line options + change: | + :option:`--enable-fine-grain-logging` and :option:`--component-log-level` were incompatible in that one + would make the other ineffective. Setting both options at once is now an error, to reduce potential confusion. - area: tcp change: | Added support for :ref:`connection_pool_per_downstream_connection diff --git a/docs/root/operations/cli.rst b/docs/root/operations/cli.rst index cd33505380dc..aeb70858a233 100644 --- a/docs/root/operations/cli.rst +++ b/docs/root/operations/cli.rst @@ -109,6 +109,7 @@ following are the command line options that Envoy supports. never set this option. For example, if you want ``upstream`` component to run at ``debug`` level and ``connection`` component to run at ``trace`` level, you should pass ``upstream:debug,connection:trace`` to this flag. See ``ALL_LOGGER_IDS`` in :repo:`/source/common/common/logger.h` for a list of components. + This option is incompatible with :option:`--enable-fine-grain-logging`. .. option:: --cpuset-threads @@ -193,7 +194,7 @@ following are the command line options that Envoy supports. interface. If enabled, main log macros including ``ENVOY_LOG``, ``ENVOY_CONN_LOG``, ``ENVOY_STREAM_LOG`` and ``ENVOY_FLUSH_LOG`` will use a per-file logger, and the usage doesn't need ``Envoy::Logger::Loggable`` any more. The administration interface usage is similar. Please see :ref:`Administration interface - ` for more detail. + ` for more detail. This option is incompatible with :option:`--component-log-level`. .. option:: --socket-path diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index eaa6005bb260..b72e6237249f 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -214,7 +214,12 @@ 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(); + if (enable_fine_grain_logging_ && !component_log_level.getValue().empty()) { + throw MalformedArgvException( + "error: --component-log-level will not work with --enable-fine-grain-logging"); + } parseComponentLogLevels(component_log_level.getValue()); diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 732ab9f03ed9..b03fdb15fee3 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -61,6 +61,13 @@ TEST_F(OptionsImplTest, InvalidMode) { EXPECT_THROW_WITH_REGEX(createOptionsImpl("envoy --mode bogus"), MalformedArgvException, "bogus"); } +TEST_F(OptionsImplTest, InvalidComponentLogLevelWithFineGrainLogging) { + EXPECT_THROW_WITH_REGEX( + createOptionsImpl("envoy --enable-fine-grain-logging --component-log-level http:info"), + MalformedArgvException, + "--component-log-level will not work with --enable-fine-grain-logging"); +} + TEST_F(OptionsImplTest, InvalidCommandLine) { EXPECT_THROW_WITH_REGEX(createOptionsImpl("envoy --blah"), MalformedArgvException, "Couldn't find match for argument"); @@ -98,7 +105,7 @@ TEST_F(OptionsImplTest, All) { "--file-flush-interval-msec 9000 " "--skip-hot-restart-on-no-parent " "--skip-hot-restart-parent-stats " - "--drain-time-s 60 --log-format [%v] --enable-fine-grain-logging --parent-shutdown-time-s 90 " + "--drain-time-s 60 --log-format [%v] --parent-shutdown-time-s 90 " "--log-path " "/foo/bar " "--disable-hot-restart --cpuset-threads --allow-unknown-static-fields " @@ -119,7 +126,7 @@ TEST_F(OptionsImplTest, All) { EXPECT_TRUE(options->skipHotRestartParentStats()); EXPECT_TRUE(options->skipHotRestartOnNoParent()); EXPECT_EQ("/foo/bar", options->logPath()); - EXPECT_EQ(true, options->enableFineGrainLogging()); + EXPECT_EQ(false, options->enableFineGrainLogging()); EXPECT_EQ("cluster", options->serviceClusterName()); EXPECT_EQ("node", options->serviceNodeName()); EXPECT_EQ("zone", options->serviceZone()); @@ -139,6 +146,10 @@ TEST_F(OptionsImplTest, All) { options = createOptionsImpl("envoy --mode init_only"); EXPECT_EQ(Server::Mode::InitOnly, options->mode()); + + // Tested separately because it's mutually exclusive with --component-log-level. + options = createOptionsImpl("envoy --enable-fine-grain-logging"); + EXPECT_EQ(true, options->enableFineGrainLogging()); } // Either variants of allow-unknown-[static-]-fields works.