From c8c5a9cc7b6bded25675c7a12810ecc63dfd42ac Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 13 Jan 2026 11:26:55 +0000 Subject: [PATCH 1/4] ArgParser: Add option dependencies (with_required) and multiple example usages - Add with_required() method to specify that an option requires another - Change add_example_usage() to support multiple examples per command - Add unit tests for option dependencies - Update ArgParser documentation --- .../internal-libraries/ArgParser.en.rst | 59 ++++- include/tscore/ArgParser.h | 18 +- src/tscore/ArgParser.cc | 93 ++++++- src/tscore/CMakeLists.txt | 1 + .../test_ArgParser_OptionDependencies.cc | 237 ++++++++++++++++++ 5 files changed, 395 insertions(+), 13 deletions(-) create mode 100644 src/tscore/unit_tests/test_ArgParser_OptionDependencies.cc diff --git a/doc/developer-guide/internal-libraries/ArgParser.en.rst b/doc/developer-guide/internal-libraries/ArgParser.en.rst index 9336f8b8430..c15a552ff5a 100644 --- a/doc/developer-guide/internal-libraries/ArgParser.en.rst +++ b/doc/developer-guide/internal-libraries/ArgParser.en.rst @@ -155,6 +155,40 @@ Example with a required group: // User must specify either --json or --xml, but not both +Option Dependencies +------------------- + +ArgParser supports option dependencies, where one option requires another option to be present. +This is useful when an option only makes sense in combination with another option. + +To specify that an option requires another option, use the ``with_required()`` method immediately after +adding the option: + +.. code-block:: cpp + + command.add_option("--tags", "-t", "Debug tags", "", 1) + command.add_option("--append", "-a", "Append tags to existing tags") + .with_required("--tags"); // --append requires --tags to be present + +When ``--append`` is used without ``--tags``, ArgParser will display an error message and exit: + +.. code-block:: text + + Error: Option '--append' requires '--tags' to be specified + +Multiple dependencies can be specified by chaining ``with_required()`` calls: + +.. code-block:: cpp + + command.add_option("--verbose-append", "-V", "Verbose append mode") + .with_required("--tags") + .with_required("--append"); // requires both --tags and --append + +.. Note:: + + The ``with_required()`` method must be called immediately after ``add_option()`` or + ``add_option_to_group()``. It applies to the most recently added option. + Parsing Arguments ----------------- @@ -267,10 +301,23 @@ Classes is called under certain command, it will be added as a subcommand for the current command. For Example, :code:`command1.add_command("command2", "description")` will make :code:`command2` a subcommand of :code:`command1`. :code:`require_commands()` is also available within :class:`Command`. - .. function:: void add_example_usage(std::string const &usage) + .. function:: Command &add_example_usage(std::string const &usage) + + Add an example usage for the command to output in ``help_message``. This method can be + called multiple times to add multiple examples. Returns the Command instance for chained calls. + + Example:: - Add an example usage for the command to output in `help_message`. - For Example: :code:`command.add_example_usage("traffic_blabla init --path=/path/to/file")`. + command.add_example_usage("traffic_ctl server debug enable -t my_tags") + .add_example_usage("traffic_ctl server debug enable -t new_tag -a # append mode"); + + This will output in help: + + .. code-block:: text + + Example Usage: + traffic_ctl server debug enable -t my_tags + traffic_ctl server debug enable -t new_tag -a # append mode .. function:: Command &set_default() @@ -284,6 +331,12 @@ Classes Add an option to a mutually exclusive group for this command. + .. function:: Command &with_required(std::string const &required_option) + + Specify that the last added option requires another option to be present. + Must be called immediately after ``add_option()`` or ``add_option_to_group()``. + Returns the Command instance for chained calls. + .. class:: Arguments :class:`Arguments` holds the parsed arguments and function to invoke. diff --git a/include/tscore/ArgParser.h b/include/tscore/ArgParser.h index 49e27fb8a24..92a25dabac0 100644 --- a/include/tscore/ArgParser.h +++ b/include/tscore/ArgParser.h @@ -176,6 +176,13 @@ class ArgParser std::string const &description, std::string const &envvar = "", unsigned arg_num = 0, std::string const &default_value = "", std::string const &key = ""); + /** Specify that the last added option requires another option to be present. + Must be called immediately after add_option() or add_option_to_group(). + @param required_option The option that must be present (e.g., "--tags") + @return The Command instance for chained calls. + */ + Command &with_required(std::string const &required_option); + /** Two ways of adding a sub-command to current command: @return The new sub-command instance. */ @@ -217,6 +224,8 @@ class ArgParser void append_option_data(Arguments &ret, AP_StrVec &args, int index); // Helper method to validate mutually exclusive groups void validate_mutex_groups(Arguments &ret) const; + // Helper method to validate option dependencies + void validate_dependencies(Arguments &ret) const; // The command name and help message std::string _name; std::string _description; @@ -225,8 +234,8 @@ class ArgParser unsigned _arg_num = 0; // Stored Env variable std::string _envvar; - // An example usage can be added for the help message - std::string _example_usage; + // Example usages can be added for the help message + std::vector _example_usages; // Function associated with this command Function _f; // look up key @@ -248,6 +257,11 @@ class ArgParser // Key: long option name. Value: group name std::map _option_to_group; + // Option dependencies: dependent_option -> list of required options + std::map> _option_dependencies; + // Track the last added option for requires() chaining + std::string _last_added_option; + // require command / option for this parser bool _command_required = false; diff --git a/src/tscore/ArgParser.cc b/src/tscore/ArgParser.cc index 00a4fd20b1c..5237f937c0a 100644 --- a/src/tscore/ArgParser.cc +++ b/src/tscore/ArgParser.cc @@ -132,9 +132,12 @@ ArgParser::Command::help_message(std::string_view err) const std::cout << "\nOptions ======================= Default ===== Description =============" << std::endl; output_option(); } - // output example usage - if (!_example_usage.empty()) { - std::cout << "\nExample Usage: " << _example_usage << std::endl; + // output example usages + if (!_example_usages.empty()) { + std::cout << "\nExample Usage:" << std::endl; + for (const auto &example : _example_usages) { + std::cout << " " << example << std::endl; + } } // standard return code ArgParser::do_exit(usage_return_code); @@ -303,6 +306,7 @@ ArgParser::Command::add_option(std::string const &long_option, std::string const if (short_option != "-" && !short_option.empty()) { _option_map[short_option] = long_option; } + _last_added_option = long_option; // track for with_requires() chaining return *this; } @@ -340,7 +344,7 @@ ArgParser::Command::add_option_to_group(std::string const &group_name, std::stri ArgParser::do_exit(1); } - // Add the option normally + // Add the option normally (this also sets _last_added_option) add_option(long_option, short_option, description, envvar, arg_num, default_value, key); // Track this option in the mutex group @@ -375,7 +379,7 @@ ArgParser::Command::add_command(std::string const &cmd_name, std::string const & ArgParser::Command & ArgParser::Command::add_example_usage(std::string const &usage) { - _example_usage = usage; + _example_usages.push_back(usage); return *this; } @@ -443,11 +447,28 @@ ArgParser::Command::output_option() const msg = msg + std::string(INDENT_ONE - msg.size(), ' ') + it.second.default_value; } } - if (!it.second.description.empty()) { + // Build description with dependency info if applicable + std::string desc = it.second.description; + auto dep_it = _option_dependencies.find(it.first); + if (dep_it != _option_dependencies.end() && !dep_it->second.empty()) { + if (!desc.empty()) { + desc += " "; + } + desc += "(requires"; + for (size_t i = 0; i < dep_it->second.size(); ++i) { + desc += " " + dep_it->second[i]; + if (i < dep_it->second.size() - 1) { + desc += ","; + } + } + desc += ")"; + } + + if (!desc.empty()) { if (INDENT_TWO - static_cast(msg.size()) < 0) { - std::cout << msg << "\n" << std::string(INDENT_TWO, ' ') << it.second.description << std::endl; + std::cout << msg << "\n" << std::string(INDENT_TWO, ' ') << desc << std::endl; } else { - std::cout << msg << std::string(INDENT_TWO - msg.size(), ' ') << it.second.description << std::endl; + std::cout << msg << std::string(INDENT_TWO - msg.size(), ' ') << desc << std::endl; } } } @@ -574,6 +595,59 @@ ArgParser::Command::validate_mutex_groups(Arguments &ret) const } } +// Specify that the last added option requires another option +ArgParser::Command & +ArgParser::Command::with_required(std::string const &required_option) +{ + if (_last_added_option.empty()) { + std::cerr << "Error: with_required() must be called after add_option()" << std::endl; + ArgParser::do_exit(1); + } + + // Validate that required option exists + if (_option_list.find(required_option) == _option_list.end()) { + std::cerr << "Error: Required option '" << required_option << "' not found" << std::endl; + ArgParser::do_exit(1); + } + + _option_dependencies[_last_added_option].push_back(required_option); + + return *this; +} + +// Validate option dependencies +void +ArgParser::Command::validate_dependencies(Arguments &ret) const +{ + for (const auto &[dependent, required_list] : _option_dependencies) { + // Get the key for the dependent option + auto it = _option_list.find(dependent); + if (it == _option_list.end()) { + continue; + } + + const std::string &dep_key = it->second.key; + + // Check if dependent option was used + if (ret.get(dep_key)) { + // Dependent option was used, check all required options + for (const auto &required : required_list) { + auto req_it = _option_list.find(required); + if (req_it == _option_list.end()) { + continue; + } + + const std::string &req_key = req_it->second.key; + + if (!ret.get(req_key)) { + std::string error_msg = "Option '" + dependent + "' requires '" + required + "' to be specified"; + help_message(error_msg); // exit with status code 64 (EX_USAGE - command line usage error) + } + } + } + } +} + // Append the args of option to parsed data. Return true if there is any option called void ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int index) @@ -694,6 +768,9 @@ ArgParser::Command::parse(Arguments &ret, AP_StrVec &args) // Validate mutually exclusive groups validate_mutex_groups(ret); + + // Validate option dependencies + validate_dependencies(ret); } if (command_called) { diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index 03ade9cfda0..9b2382e1c4a 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -165,6 +165,7 @@ if(BUILD_TESTING) unit_tests/test_scoped_resource.cc unit_tests/test_Version.cc unit_tests/test_ArgParser_MutexGroup.cc + unit_tests/test_ArgParser_OptionDependencies.cc unit_tests/test_Allocator.cc ) target_link_libraries( diff --git a/src/tscore/unit_tests/test_ArgParser_OptionDependencies.cc b/src/tscore/unit_tests/test_ArgParser_OptionDependencies.cc new file mode 100644 index 00000000000..c996a91613c --- /dev/null +++ b/src/tscore/unit_tests/test_ArgParser_OptionDependencies.cc @@ -0,0 +1,237 @@ +/** @file + + Unit test for ArgParser option dependencies (with_required) + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include +#include "tscore/ArgParser.h" + +class TestArgParser : public ts::ArgParser +{ +public: + TestArgParser() { ts::ArgParser::set_test_mode(true); } +}; + +TEST_CASE("Option dependencies - basic dependency", "[option_dependencies]") +{ + ts::ArgParser parser; + parser.add_description("Test basic option dependency"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--tags", "-t", "Debug tags", "", 1); + parser.add_option("--append", "-a", "Append to existing tags").with_required("--tags"); + + // Test with both options - should work + const char *argv1[] = {"test", "--tags", "http", "--append", nullptr}; + ts::Arguments args1 = parser.parse(argv1); + REQUIRE(args1.get("tags") == true); + REQUIRE(args1.get("tags").value() == "http"); + REQUIRE(args1.get("append") == true); + + // Test with only --tags - should work + const char *argv2[] = {"test", "--tags", "dns", nullptr}; + ts::Arguments args2 = parser.parse(argv2); + REQUIRE(args2.get("tags") == true); + REQUIRE(args2.get("tags").value() == "dns"); + REQUIRE(args2.get("append") == false); + + // Test with neither option - should work + const char *argv3[] = {"test", nullptr}; + ts::Arguments args3 = parser.parse(argv3); + REQUIRE(args3.get("tags") == false); + REQUIRE(args3.get("append") == false); +} + +TEST_CASE("Option dependencies - violation detection", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test dependency violation"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--tags", "-t", "Debug tags", "", 1); + parser.add_option("--append", "-a", "Append to existing tags").with_required("--tags"); + + // Test with only --append (without --tags) - should error + const char *argv[] = {"test", "--append", nullptr}; + REQUIRE_THROWS(parser.parse(argv)); +} + +TEST_CASE("Option dependencies - short option violation", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test dependency violation with short option"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--tags", "-t", "Debug tags", "", 1); + parser.add_option("--append", "-a", "Append to existing tags").with_required("--tags"); + + // Test with short option -a (without --tags) - should error + const char *argv[] = {"test", "-a", nullptr}; + REQUIRE_THROWS(parser.parse(argv)); +} + +TEST_CASE("Option dependencies - multiple dependencies", "[option_dependencies]") +{ + ts::ArgParser parser; + parser.add_description("Test multiple option dependencies"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--tags", "-t", "Debug tags", "", 1); + parser.add_option("--append", "-a", "Append mode"); + parser.add_option("--verbose-append", "-V", "Verbose append mode").with_required("--tags").with_required("--append"); + + // Test with all options - should work + const char *argv1[] = {"test", "--tags", "http", "--append", "--verbose-append", nullptr}; + ts::Arguments args1 = parser.parse(argv1); + REQUIRE(args1.get("tags") == true); + REQUIRE(args1.get("append") == true); + REQUIRE(args1.get("verbose-append") == true); +} + +TEST_CASE("Option dependencies - multiple dependencies violation", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test multiple dependency violation"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--tags", "-t", "Debug tags", "", 1); + parser.add_option("--append", "-a", "Append mode"); + parser.add_option("--verbose-append", "-V", "Verbose append mode").with_required("--tags").with_required("--append"); + + // Test with --verbose-append but only --tags (missing --append) - should error + const char *argv[] = {"test", "--tags", "http", "--verbose-append", nullptr}; + REQUIRE_THROWS(parser.parse(argv)); +} + +TEST_CASE("Option dependencies - with subcommands", "[option_dependencies]") +{ + ts::ArgParser parser; + ts::ArgParser::Command &cmd = parser.add_command("debug", "Debug commands"); + + cmd.add_option("--tags", "-t", "Debug tags", "", 1); + cmd.add_option("--append", "-a", "Append to existing tags").with_required("--tags"); + + // Test with subcommand and both options - should work + const char *argv1[] = {"test", "debug", "--tags", "http", "--append", nullptr}; + ts::Arguments args1 = parser.parse(argv1); + REQUIRE(args1.get("debug") == true); + REQUIRE(args1.get("tags") == true); + REQUIRE(args1.get("append") == true); + + // Test with subcommand and only --tags - should work + const char *argv2[] = {"test", "debug", "-t", "dns", nullptr}; + ts::Arguments args2 = parser.parse(argv2); + REQUIRE(args2.get("debug") == true); + REQUIRE(args2.get("tags") == true); + REQUIRE(args2.get("append") == false); +} + +TEST_CASE("Option dependencies - subcommand violation", "[option_dependencies]") +{ + TestArgParser parser; + TestArgParser::Command &cmd = parser.add_command("debug", "Debug commands"); + + cmd.add_option("--tags", "-t", "Debug tags", "", 1); + cmd.add_option("--append", "-a", "Append to existing tags").with_required("--tags"); + + // Test with subcommand and only --append - should error + const char *argv[] = {"test", "debug", "--append", nullptr}; + REQUIRE_THROWS(parser.parse(argv)); +} + +TEST_CASE("Option dependencies - invalid required option", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test invalid required option"); + parser.add_global_usage("test [OPTIONS]"); + + parser.add_option("--append", "-a", "Append mode"); + + // Try to require an option that doesn't exist - should throw + REQUIRE_THROWS(parser.add_option("--verbose", "-v", "Verbose mode").with_required("--nonexistent")); +} + +TEST_CASE("Option dependencies - with_required without add_option", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test with_required without prior add_option"); + parser.add_global_usage("test [OPTIONS]"); + + // Calling with_required() without first calling add_option() should error + // This is a bit tricky to test since with_required returns Command& + // The error would occur at runtime when there's no _last_added_option + // We need to test this via the Command directly + parser.add_option("--first", "-f", "First option"); + + // Add a second option and require the first - this should work + parser.add_option("--second", "-s", "Second option").with_required("--first"); + + const char *argv[] = {"test", "--first", "--second", nullptr}; + ts::Arguments args = parser.parse(argv); + REQUIRE(args.get("first") == true); + REQUIRE(args.get("second") == true); +} + +TEST_CASE("Option dependencies - combined with mutex groups", "[option_dependencies]") +{ + ts::ArgParser parser; + parser.add_description("Test dependencies combined with mutex groups"); + parser.add_global_usage("test [OPTIONS]"); + + // Mutex group for mode + parser.add_mutex_group("mode", false, "Operation mode"); + parser.add_option_to_group("mode", "--enable", "-e", "Enable mode"); + parser.add_option_to_group("mode", "--disable", "-d", "Disable mode"); + + // Option that requires --enable + parser.add_option("--tags", "-t", "Debug tags", "", 1).with_required("--enable"); + + // Test with --enable and --tags - should work + const char *argv1[] = {"test", "--enable", "--tags", "http", nullptr}; + ts::Arguments args1 = parser.parse(argv1); + REQUIRE(args1.get("enable") == true); + REQUIRE(args1.get("tags") == true); + + // Test with --disable only - should work (--tags not used) + const char *argv2[] = {"test", "--disable", nullptr}; + ts::Arguments args2 = parser.parse(argv2); + REQUIRE(args2.get("disable") == true); + REQUIRE(args2.get("tags") == false); +} + +TEST_CASE("Option dependencies - combined with mutex groups violation", "[option_dependencies]") +{ + TestArgParser parser; + parser.add_description("Test dependencies combined with mutex groups violation"); + parser.add_global_usage("test [OPTIONS]"); + + // Mutex group for mode + parser.add_mutex_group("mode", false, "Operation mode"); + parser.add_option_to_group("mode", "--enable", "-e", "Enable mode"); + parser.add_option_to_group("mode", "--disable", "-d", "Disable mode"); + + // Option that requires --enable + parser.add_option("--tags", "-t", "Debug tags", "", 1).with_required("--enable"); + + // Test with --disable and --tags - should error (--tags requires --enable) + const char *argv[] = {"test", "--disable", "--tags", "http", nullptr}; + REQUIRE_THROWS(parser.parse(argv)); +} From a16206e6662f98c922bf0686897f2e0cfb85319c Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 13 Jan 2026 11:27:07 +0000 Subject: [PATCH 2/4] traffic_ctl: Add --append flag to 'server debug enable' for appending tags - Add --append/-a option to append debug tags instead of replacing them - Uses ArgParser's with_required() to enforce --tags dependency - Add autest for server debug enable/disable commands - Update traffic_ctl documentation --- .../command-line/traffic_ctl.en.rst | 16 +++- src/traffic_ctl/CtrlCommands.cc | 21 ++++- src/traffic_ctl/CtrlCommands.h | 1 + src/traffic_ctl/traffic_ctl.cc | 5 +- tests/gold_tests/traffic_ctl/gold/test_2.gold | 1 - tests/gold_tests/traffic_ctl/gold/test_3.gold | 1 - .../traffic_ctl_server_debug.test.py | 80 +++++++++++++++++++ .../traffic_ctl/traffic_ctl_test_utils.py | 54 +++++++++++++ 8 files changed, 174 insertions(+), 5 deletions(-) delete mode 100644 tests/gold_tests/traffic_ctl/gold/test_2.gold delete mode 100644 tests/gold_tests/traffic_ctl/gold/test_3.gold create mode 100644 tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py diff --git a/doc/appendices/command-line/traffic_ctl.en.rst b/doc/appendices/command-line/traffic_ctl.en.rst index 55dbee55707..28718666479 100644 --- a/doc/appendices/command-line/traffic_ctl.en.rst +++ b/doc/appendices/command-line/traffic_ctl.en.rst @@ -535,6 +535,11 @@ traffic_ctl server This string should contain an anchored regular expression that filters the messages based on the debug tag tag. Please refer to :ts:cv:`proxy.config.diags.debug.tags` for more information + .. option:: --append, -a + + Append the specified tags to the existing debug tags instead of replacing them. This option requires + ``--tags`` to be specified. The new tags will be combined with existing tags using the ``|`` separator. + .. option:: --client_ip, -c ip Please see :ts:cv:`proxy.config.diags.debug.client_ip` for information. @@ -547,13 +552,22 @@ traffic_ctl server Disables logging for diagnostic messages. Equivalent to set :ts:cv:`proxy.config.diags.debug.enabled` to ``0``. - Example: + Examples: .. code-block:: bash + # Set debug tags (replaces existing tags) $ traffic_ctl server debug enable --tags "quic|quiche" ■ TS Runtime debug set to »ON(1)« - tags »"quic|quiche"«, client_ip »unchanged« + # Append debug tags to existing tags + $ traffic_ctl server debug enable --tags "http" --append + ■ TS Runtime debug set to »ON(1)« - tags »"quic|quiche|http"«, client_ip »unchanged« + + # Disable debug logging + $ traffic_ctl server debug disable + ■ TS Runtime debug set to »OFF(0)« + .. _traffic-control-command-storage: traffic_ctl storage diff --git a/src/traffic_ctl/CtrlCommands.cc b/src/traffic_ctl/CtrlCommands.cc index c2f888e87ef..83c3483efe9 100644 --- a/src/traffic_ctl/CtrlCommands.cc +++ b/src/traffic_ctl/CtrlCommands.cc @@ -695,12 +695,31 @@ ServerCommand::server_debug() { // Set ATS to enable or disable debug at runtime. const bool enable = get_parsed_arguments()->get(ENABLE_STR); + const bool append = get_parsed_arguments()->get(APPEND_STR); // If the following is not passed as options then the request will ignore them as default values // will be set. - const std::string tags = get_parsed_arguments()->get(TAGS_STR).value(); + std::string tags = get_parsed_arguments()->get(TAGS_STR).value(); const std::string client_ip = get_parsed_arguments()->get(CLIENT_IP_STR).value(); + // If append mode is enabled and tags are provided, fetch current tags and combine + if (append && !tags.empty()) { + shared::rpc::RecordLookupRequest lookup_request; + lookup_request.emplace_rec("proxy.config.diags.debug.tags", shared::rpc::NOT_REGEX, shared::rpc::CONFIG_REC_TYPES); + auto lookup_response = invoke_rpc(lookup_request); + + if (!lookup_response.is_error()) { + auto const &records = lookup_response.result.as(); + if (!records.recordList.empty()) { + std::string current_tags = records.recordList[0].currentValue; + if (!current_tags.empty()) { + // Combine: current|new + tags = current_tags + "|" + tags; + } + } + } + } + const SetDebugServerRequest request{enable, tags, client_ip}; shared::rpc::JSONRPCResponse const &response = invoke_rpc(request); diff --git a/src/traffic_ctl/CtrlCommands.h b/src/traffic_ctl/CtrlCommands.h index c10e697ed5f..8d5fc2b9316 100644 --- a/src/traffic_ctl/CtrlCommands.h +++ b/src/traffic_ctl/CtrlCommands.h @@ -236,6 +236,7 @@ class ServerCommand : public CtrlCommand static inline const std::string ENABLE_STR{"enable"}; static inline const std::string DISABLE_STR{"disable"}; static inline const std::string TAGS_STR{"tags"}; + static inline const std::string APPEND_STR{"append"}; static inline const std::string CLIENT_IP_STR{"client_ip"}; static inline const std::string STATUS_STR{"status"}; diff --git a/src/traffic_ctl/traffic_ctl.cc b/src/traffic_ctl/traffic_ctl.cc index a018304a3f7..6529c0f88b7 100644 --- a/src/traffic_ctl/traffic_ctl.cc +++ b/src/traffic_ctl/traffic_ctl.cc @@ -193,8 +193,11 @@ main([[maybe_unused]] int argc, const char **argv) server_command.add_command("debug", "Enable/Disable ATS for diagnostic messages at runtime").require_commands(); debug_command.add_command("enable", "Enables logging for diagnostic messages at runtime", Command_Execute) .add_option("--tags", "-t", "Debug tags", "TS_DEBUG_TAGS", 1) + .add_option("--append", "-a", "Append tags to existing tags instead of replacing") + .with_required("--tags") .add_option("--client_ip", "-c", "Client's ip", "", 1, "") - .add_example_usage("traffic_ctl server debug enable -t my_tags -c X.X.X.X"); + .add_example_usage("traffic_ctl server debug enable -t my_tags -c X.X.X.X") + .add_example_usage("traffic_ctl server debug enable -t new_tag -a # append mode"); debug_command.add_command("disable", "Disables logging for diagnostic messages at runtime", Command_Execute) .add_example_usage("traffic_ctl server debug disable"); diff --git a/tests/gold_tests/traffic_ctl/gold/test_2.gold b/tests/gold_tests/traffic_ctl/gold/test_2.gold deleted file mode 100644 index 3c75916cf79..00000000000 --- a/tests/gold_tests/traffic_ctl/gold/test_2.gold +++ /dev/null @@ -1 +0,0 @@ -proxy.config.diags.debug.enabled: 1 diff --git a/tests/gold_tests/traffic_ctl/gold/test_3.gold b/tests/gold_tests/traffic_ctl/gold/test_3.gold deleted file mode 100644 index e12f994befe..00000000000 --- a/tests/gold_tests/traffic_ctl/gold/test_3.gold +++ /dev/null @@ -1 +0,0 @@ -proxy.config.diags.debug.tags: rpc # default http|dns diff --git a/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py b/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py new file mode 100644 index 00000000000..f899f980708 --- /dev/null +++ b/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py @@ -0,0 +1,80 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys + +# To include util classes +sys.path.insert(0, f'{Test.TestDirectory}') + +from traffic_ctl_test_utils import Make_traffic_ctl + +Test.Summary = ''' +Test traffic_ctl server debug enable/disable commands. +''' + +Test.ContinueOnFail = True + +records_yaml = ''' +diags: + debug: + enabled: 0 + tags: xyz +''' + +traffic_ctl = Make_traffic_ctl(Test, records_yaml) + +###### +# Test 1: Enable debug with tags +traffic_ctl.server().debug().enable(tags="http").exec() +# Test 2: Verify debug is enabled and tags are set +traffic_ctl.config().get("proxy.config.diags.debug.enabled").validate_with_text("proxy.config.diags.debug.enabled: 1") +# Test 3: Verify tags are set +traffic_ctl.config().get("proxy.config.diags.debug.tags").validate_with_text("proxy.config.diags.debug.tags: http") + +# Test 4: Disable debug +traffic_ctl.server().debug().disable().exec() +# Test 5: Verify debug is disabled +traffic_ctl.config().get("proxy.config.diags.debug.enabled").validate_with_text("proxy.config.diags.debug.enabled: 0") + +# Test 6: Enable debug with new tags (replace mode) +traffic_ctl.server().debug().enable(tags="cache").exec() +# Test 7: Verify tags are replaced +traffic_ctl.config().get("proxy.config.diags.debug.tags").validate_with_text("proxy.config.diags.debug.tags: cache") + +# Test 8: Enable debug with append mode - should combine with existing tags +traffic_ctl.server().debug().enable(tags="http", append=True).exec() +# Test 9: Verify tags are appended +traffic_ctl.config().get("proxy.config.diags.debug.tags").validate_with_text("proxy.config.diags.debug.tags: cache|http") + +# Test 10: Append another tag +traffic_ctl.server().debug().enable(tags="dns", append=True).exec() +# Test 11: Verify all tags are present +traffic_ctl.config().get("proxy.config.diags.debug.tags").validate_with_text("proxy.config.diags.debug.tags: cache|http|dns") + +# Test 12: Disable and verify +traffic_ctl.server().debug().disable().exec() +# Test 13: Verify debug is disabled +traffic_ctl.config().get("proxy.config.diags.debug.enabled").validate_with_text("proxy.config.diags.debug.enabled: 0") + +# Test 14: Verify --append requires --tags (should fail with error) +# This tests the ArgParser requires() functionality +tr = Test.AddTestRun("test --append without --tags") +tr.Processes.Default.Env = traffic_ctl._ts.Env +tr.Processes.Default.Command = "traffic_ctl server debug enable --append" +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + "Option \'--append\' requires \'--tags\' to be specified", "Should show error that --append requires --tags") +tr.StillRunningAfter = traffic_ctl._ts diff --git a/tests/gold_tests/traffic_ctl/traffic_ctl_test_utils.py b/tests/gold_tests/traffic_ctl/traffic_ctl_test_utils.py index b8c4b18f74c..a40d60e86bd 100644 --- a/tests/gold_tests/traffic_ctl/traffic_ctl_test_utils.py +++ b/tests/gold_tests/traffic_ctl/traffic_ctl_test_utils.py @@ -233,6 +233,50 @@ def validate_with_goldfile(self, file: str): self._finish() +class Debug(Common): + """ + Handy class to map traffic_ctl server debug options. + """ + + def __init__(self, dir, tr, tn): + super().__init__(tr) + self._cmd = "traffic_ctl server debug " + self._dir = dir + self._tn = tn + + def enable(self, tags=None, append=False, client_ip=None): + """ + Enable debug logging at runtime. + + Args: + tags: Debug tags to set (e.g., "http|dns") + append: If True, append tags to existing tags instead of replacing + client_ip: Client IP filter for debug output + + Example: + traffic_ctl.server().debug().enable(tags="http").exec() + traffic_ctl.server().debug().enable(tags="dns", append=True).exec() + """ + self._cmd = f'{self._cmd} enable' + if tags: + self._cmd = f'{self._cmd} --tags {tags}' + if append: + self._cmd = f'{self._cmd} --append' + if client_ip: + self._cmd = f'{self._cmd} --client_ip {client_ip}' + return self + + def disable(self): + """ + Disable debug logging at runtime. + + Example: + traffic_ctl.server().debug().disable().exec() + """ + self._cmd = f'{self._cmd} disable' + return self + + class Server(Common): """ Handy class to map traffic_ctl server options. @@ -254,6 +298,16 @@ def drain(self, undo=False): self._cmd = f'{self._cmd} --undo' return self + def debug(self): + """ + Returns a Debug object for debug enable/disable commands. + + Example: + traffic_ctl.server().debug().enable(tags="http").exec() + traffic_ctl.server().debug().disable().exec() + """ + return Debug(self._dir, self._tr, self._tn) + def as_json(self): self._cmd = f'{self._cmd} -f json' return self From b9da9f8d64cff94b017947de8972f8e68f9c25dd Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 13 Jan 2026 12:09:26 +0000 Subject: [PATCH 3/4] format --- src/tscore/ArgParser.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tscore/ArgParser.cc b/src/tscore/ArgParser.cc index 5237f937c0a..5636d107d14 100644 --- a/src/tscore/ArgParser.cc +++ b/src/tscore/ArgParser.cc @@ -448,7 +448,7 @@ ArgParser::Command::output_option() const } } // Build description with dependency info if applicable - std::string desc = it.second.description; + std::string desc = it.second.description; auto dep_it = _option_dependencies.find(it.first); if (dep_it != _option_dependencies.end() && !dep_it->second.empty()) { if (!desc.empty()) { From fec397dcfa5c8e6c5945d45ba2727e2c79df17f2 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Thu, 15 Jan 2026 10:53:04 +0000 Subject: [PATCH 4/4] Fix typos --- include/tscore/ArgParser.h | 2 +- src/tscore/ArgParser.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/tscore/ArgParser.h b/include/tscore/ArgParser.h index 92a25dabac0..a4bd5916feb 100644 --- a/include/tscore/ArgParser.h +++ b/include/tscore/ArgParser.h @@ -259,7 +259,7 @@ class ArgParser // Option dependencies: dependent_option -> list of required options std::map> _option_dependencies; - // Track the last added option for requires() chaining + // Track the last added option for with_required() chaining std::string _last_added_option; // require command / option for this parser diff --git a/src/tscore/ArgParser.cc b/src/tscore/ArgParser.cc index 5636d107d14..65298317a61 100644 --- a/src/tscore/ArgParser.cc +++ b/src/tscore/ArgParser.cc @@ -306,7 +306,7 @@ ArgParser::Command::add_option(std::string const &long_option, std::string const if (short_option != "-" && !short_option.empty()) { _option_map[short_option] = long_option; } - _last_added_option = long_option; // track for with_requires() chaining + _last_added_option = long_option; // track for with_required() chaining return *this; }