This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 694e917437f83d27d1f1b94874b5d74321fb584d Author: Damian Meden <[email protected]> AuthorDate: Wed Jul 24 12:04:00 2024 +0200 Metrics: Remove code related to clear/reset metrics at runtime. (#11561) We have decided to deprecate this feature with the latest metrics machinery code. Metrics are now non persistent so they get reset on every restart. (cherry picked from commit 99337bfeb4e55235e6d70837f284d78e89a2f3eb) --- doc/appendices/command-line/traffic_ctl.en.rst | 9 -- doc/developer-guide/jsonrpc/jsonrpc-api.en.rst | 140 +-------------------- include/records/RecCore.h | 6 - src/mgmt/rpc/handlers/records/Records.cc | 50 -------- src/records/P_RecCore.cc | 63 ---------- src/traffic_ctl/CtrlCommands.cc | 22 ---- src/traffic_ctl/CtrlCommands.h | 4 - src/traffic_ctl/jsonrpc/CtrlRPCRequests.h | 27 ---- src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h | 14 --- src/traffic_ctl/traffic_ctl.cc | 2 - src/traffic_server/RpcAdminPubHandlers.cc | 4 - .../json/admin_clear_metrics_records_req.json | 10 -- .../gold_tests/jsonrpc/jsonrpc_api_schema.test.py | 6 - 13 files changed, 3 insertions(+), 354 deletions(-) diff --git a/doc/appendices/command-line/traffic_ctl.en.rst b/doc/appendices/command-line/traffic_ctl.en.rst index d5551d3c82..b50b188486 100644 --- a/doc/appendices/command-line/traffic_ctl.en.rst +++ b/doc/appendices/command-line/traffic_ctl.en.rst @@ -305,13 +305,6 @@ traffic_ctl metric Display the current values of all statistics whose names match the given regular expression. -.. program:: traffic_ctl metric -.. option:: zero METRIC [METRIC...] - - :ref:`admin_clear_metrics_records` - - Reset the named statistics to zero. - .. program:: traffic_ctl metric .. option:: describe RECORD [RECORD...] @@ -518,8 +511,6 @@ but rather to the rpc endpoint, so you can directly send requests and receive re - admin_host_set_status - admin_server_stop_drain - admin_server_start_drain - - admin_clear_metrics_records - - admin_clear_all_metrics_records - admin_plugin_send_basic_msg - admin_lookup_records - admin_config_set_records diff --git a/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst index c7703f000c..c1b7986345 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst @@ -323,7 +323,7 @@ The following errors could be generated when requesting record from the server. .. enumerator:: RECORD_WRITE_ERROR = 2006 - Generic error while writing the record. ie: RecResetStatRecord() returns REC_ERR_OKAY + Generic error while writing the record. .. enumerator:: REQUESTED_TYPE_MISMATCH = 2007 @@ -446,16 +446,11 @@ JSONRPC API * `admin_lookup_records`_ -* `admin_clear_all_metrics_records`_ * `admin_config_set_records`_ * `admin_config_reload`_ -* `admin_clear_metrics_records`_ - -* `admin_clear_all_metrics_records`_ - * `admin_host_set_status`_ * `admin_server_stop_drain`_ @@ -968,135 +963,8 @@ Validation: You can request for the record `proxy.process.proxy.reconfigure_time` which will be updated with the time of the requested update. -.. _jsonrpc-api-management-metrics: - -Metrics -======= - -.. _admin_clear_metrics_records: - -admin_clear_metrics_records ---------------------------- - -|method| - -Description -~~~~~~~~~~~ - -Clear one or more metric values. This API will take the incoming metric names and reset their associated value. The format for the incoming -request should follow the `RecordRequest`_ . - - - -Parameters -~~~~~~~~~~ - -* ``params``: A list of `RecordRequest`_ objects. - -.. note:: - - Only the ``rec_name`` will be used, if this is not provided, the API will report it back as part of the `RecordErrorObject`_ . - - -Result -~~~~~~ - -This api will only inform for errors during the metric update, all errors will be inside the `RecordErrorObject`_ object. -Successfully metric updates will not report back to the client. So it can be assumed that the records were properly updated. - -.. note:: - - As per our internal API if the metric could not be updated because there is no change in the value, ie: it's already ``0`` this will be reported back to the client as part of the `RecordErrorObject`_ - -Examples -~~~~~~~~ - - -Request: - -.. code-block:: json - :linenos: - - { - "id": "ded7018e-0720-11eb-abe2-001fc69cc946", - "jsonrpc": "2.0", - "method": "admin_clear_metrics_records", - "params": [ - { - "record_name": "proxy.process.http.total_client_connections_ipv6" - }, - { - "record_name": "proxy.config.log.rolling_intervi_should_fail" - } - ] - } - - -Response: - -.. code-block:: json - - { - "jsonrpc": "2.0", - "result": { - "errorList": [{ - "code": "2006", - "record_name": "proxy.config.log.rolling_intervi_should_fail" - }] - }, - "id": "ded7018e-0720-11eb-abe2-001fc69cc946" - } - - -.. _admin_clear_all_metrics_records: - -admin_clear_all_metrics_records -------------------------------- - -|method| - -Description -~~~~~~~~~~~ - -Clear all the metrics. - - -Parameters -~~~~~~~~~~ - -* ``params``: This can be Omitted - - -Result -~~~~~~ - -This api will only inform for errors during the metric update. Errors will be tracked down in the `error` field. - -.. note:: - - As per our internal API if the metric could not be updated because there is no change in the value, ie: it's already ``0`` this - will be reported back to the client as part of the `RecordErrorObject`_ - -Examples -~~~~~~~~ - -Request: - -.. code-block:: json - :linenos: - - { - "id": "dod7018e-0720-11eb-abe2-001fc69cc997", - "jsonrpc": "2.0", - "method": "admin_clear_all_metrics_records" - } - - - -Response: - -The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. - +Host +==== .. _admin_host_set_status: @@ -1622,8 +1490,6 @@ Response: "admin_host_set_status", "admin_server_stop_drain", "admin_server_start_drain", - "admin_clear_metrics_records", - "admin_clear_all_metrics_records", "admin_plugin_send_basic_msg", "admin_lookup_records", "admin_config_set_records", diff --git a/include/records/RecCore.h b/include/records/RecCore.h index 6e54daec5e..d730ee5e03 100644 --- a/include/records/RecCore.h +++ b/include/records/RecCore.h @@ -275,12 +275,6 @@ RecFloat REC_readFloat(char *name, bool *found, bool lock = true); RecCounter REC_readCounter(char *name, bool *found, bool lock = true); RecString REC_readString(const char *name, bool *found, bool lock = true); -//------------------------------------------------------------------------ -// Clear Statistics -//------------------------------------------------------------------------ -RecErrT RecResetStatRecord(const char *name); -RecErrT RecResetStatRecord(RecT type = RECT_NULL, bool all = false); - //------------------------------------------------------------------------ // Set RecRecord attributes //------------------------------------------------------------------------ diff --git a/src/mgmt/rpc/handlers/records/Records.cc b/src/mgmt/rpc/handlers/records/Records.cc index 8c5d248587..4799bf6c81 100644 --- a/src/mgmt/rpc/handlers/records/Records.cc +++ b/src/mgmt/rpc/handlers/records/Records.cc @@ -246,54 +246,4 @@ lookup_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node const &p resp[ERROR_LIST_KEY] = errorList; return resp; } - -swoc::Rv<YAML::Node> -clear_all_metrics_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node const & /* params ATS_UNUSED */) -{ - using namespace rpc::handlers::records::utils; - swoc::Rv<YAML::Node> resp; - if (RecResetStatRecord(RECT_NULL, true) != REC_ERR_OKAY) { - return swoc::Errata{std::error_code{errors::Codes::METRIC}, "Failed to clear stats"}; - } - - return resp; -} - -swoc::Rv<YAML::Node> -clear_metrics_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node const ¶ms) -{ - using namespace rpc::handlers::records::utils; - - YAML::Node resp, errorList; - - for (auto &&element : params) { - RequestRecordElement recordElement; - try { - recordElement = element.as<RequestRecordElement>(); - } catch (YAML::Exception const &) { - errorList.push_back(ErrorInfo{{err::RecordError::INVALID_INCOMING_DATA}}); - continue; - } - - if (!recordElement.recName.empty()) { - if (RecResetStatRecord(recordElement.recName.c_str()) != REC_ERR_OKAY) { - // This could be due the fact that the record is already cleared or the metric does not have any significant - // value. - ErrorInfo ei{err::RecordError::RECORD_WRITE_ERROR}; - ei.recordName = recordElement.recName; - errorList.push_back(ei); - } - } else { - errorList.push_back(ErrorInfo{{err::RecordError::INVALID_INCOMING_DATA}}); - continue; - } - } - - if (!errorList.IsNull()) { - resp[ERROR_LIST_KEY] = errorList; - } - - return resp; -} - } // namespace rpc::handlers::records diff --git a/src/records/P_RecCore.cc b/src/records/P_RecCore.cc index a1f4881569..810ceb14cd 100644 --- a/src/records/P_RecCore.cc +++ b/src/records/P_RecCore.cc @@ -451,69 +451,6 @@ RecExecConfigUpdateCbs(unsigned int update_required_type) return update_type; } -static RecErrT -reset_stat_record(RecRecord *rec) -{ - RecErrT err = REC_ERR_FAIL; - - if (i_am_the_record_owner(rec->rec_type)) { - rec_mutex_acquire(&(rec->lock)); - ++(rec->version); - err = RecDataSet(rec->data_type, &(rec->data), &(rec->data_default)) ? REC_ERR_OKAY : REC_ERR_FAIL; - rec_mutex_release(&(rec->lock)); - } - - return err; -} - -//------------------------------------------------------------------------ -// RecResetStatRecord -//------------------------------------------------------------------------ -RecErrT -RecResetStatRecord(const char *name) -{ - RecErrT err = REC_ERR_FAIL; - - if (auto it = g_records_ht.find(name); it != g_records_ht.end()) { - err = reset_stat_record(it->second); - } - - return err; -} - -//------------------------------------------------------------------------ -// RecResetStatRecord -//------------------------------------------------------------------------ -RecErrT -RecResetStatRecord(RecT type, bool all) -{ - int i, num_records; - RecErrT err = REC_ERR_OKAY; - - RecDebug(DL_Note, "Reset Statistics Records"); - - num_records = g_num_records; - for (i = 0; i < num_records; i++) { - RecRecord *r1 = &(g_records[i]); - - if (!REC_TYPE_IS_STAT(r1->rec_type)) { - continue; - } - - if (r1->data_type == RECD_STRING) { - continue; - } - - if (((type == RECT_NULL) || (r1->rec_type == type)) && (all || (r1->stat_meta.persist_type != RECP_NON_PERSISTENT))) { - if (reset_stat_record(r1) != REC_ERR_OKAY) { - err = REC_ERR_FAIL; - } - } - } - - return err; -} - RecErrT RecSetSyncRequired(char *name, bool lock) { diff --git a/src/traffic_ctl/CtrlCommands.cc b/src/traffic_ctl/CtrlCommands.cc index fe6d46693d..f619373c21 100644 --- a/src/traffic_ctl/CtrlCommands.cc +++ b/src/traffic_ctl/CtrlCommands.cc @@ -240,12 +240,6 @@ MetricCommand::MetricCommand(ts::Arguments *args) : RecordCommand(args) } else if (args->get(DESCRIBE_STR)) { _printer = std::make_unique<RecordDescribePrinter>(printOpts); _invoked_func = [&]() { metric_describe(); }; - } else if (args->get(CLEAR_STR)) { - _printer = std::make_unique<GenericPrinter>(printOpts); - _invoked_func = [&]() { metric_clear(); }; - } else if (args->get(ZERO_STR)) { - _printer = std::make_unique<GenericPrinter>(printOpts); - _invoked_func = [&]() { metric_zero(); }; } else if (args->get(MONITOR_STR)) { _printer = std::make_unique<MetricRecordPrinter>(printOpts); _invoked_func = [&]() { metric_monitor(); }; @@ -270,22 +264,6 @@ MetricCommand::metric_describe() _printer->write_output(record_fetch(get_parsed_arguments()->get(DESCRIBE_STR), shared::rpc::NOT_REGEX, RecordQueryType::METRIC)); } -void -MetricCommand::metric_clear() -{ - [[maybe_unused]] auto const &response = invoke_rpc(ClearAllMetricRequest{}); -} - -void -MetricCommand::metric_zero() -{ - auto records = get_parsed_arguments()->get(ZERO_STR); - ClearMetricRequest request{// names - {{std::begin(records), std::end(records)}}}; - - [[maybe_unused]] auto const &response = invoke_rpc(request); -} - void MetricCommand::metric_monitor() { diff --git a/src/traffic_ctl/CtrlCommands.h b/src/traffic_ctl/CtrlCommands.h index 43074681b6..855a47c5b9 100644 --- a/src/traffic_ctl/CtrlCommands.h +++ b/src/traffic_ctl/CtrlCommands.h @@ -146,15 +146,11 @@ public: // ----------------------------------------------------------------------------------------------------------------------------------- class MetricCommand : public RecordCommand { - static inline const std::string CLEAR_STR{"clear"}; - static inline const std::string ZERO_STR{"zero"}; static inline const std::string MONITOR_STR{"monitor"}; void metric_get(); void metric_match(); void metric_describe(); - void metric_clear(); - void metric_zero(); void metric_monitor(); public: diff --git a/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h b/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h index 05c6aca4f0..f4b17c6fa3 100644 --- a/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h +++ b/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h @@ -60,33 +60,6 @@ struct ConfigShowFileRegistryRequest : shared::rpc::ClientRequest { } }; //------------------------------------------------------------------------------------------------------------------------------------ -/// -/// @brief Models the clear 'all' metrics request. -/// -struct ClearAllMetricRequest : shared::rpc::ClientRequest { - std::string - get_method() const override - { - return "admin_clear_all_metrics_records"; - } -}; -//------------------------------------------------------------------------------------------------------------------------------------ -/// -/// @brief Models the clear metrics request. -/// -struct ClearMetricRequest : shared::rpc::ClientRequest { - using super = shared::rpc::ClientRequest; - struct Params { - std::vector<std::string> names; //!< client expects a list of record names. - }; - ClearMetricRequest(Params p) { super::params = p; } - std::string - get_method() const override - { - return "admin_clear_metrics_records"; - } -}; -//------------------------------------------------------------------------------------------------------------------------------------ struct ConfigSetRecordRequest : shared::rpc::ClientRequest { struct Params { std::string recName; diff --git a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h index 598fbbfa3a..6e659f7b7a 100644 --- a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h +++ b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h @@ -71,20 +71,6 @@ template <> struct convert<HostSetStatusRequest::Params> { } }; //------------------------------------------------------------------------------------------------------------------------------------ -template <> struct convert<ClearMetricRequest::Params> { - static Node - encode(ClearMetricRequest::Params const ¶ms) - { - Node node; - for (auto &name : params.names) { - Node n; - n["record_name"] = name; - node.push_back(n); - } - return node; - } -}; -//------------------------------------------------------------------------------------------------------------------------------------ template <> struct convert<BasicPluginMessageRequest::Params> { static Node encode(BasicPluginMessageRequest::Params const ¶ms) diff --git a/src/traffic_ctl/traffic_ctl.cc b/src/traffic_ctl/traffic_ctl.cc index 5ae9bbe283..a58725e7da 100644 --- a/src/traffic_ctl/traffic_ctl.cc +++ b/src/traffic_ctl/traffic_ctl.cc @@ -143,7 +143,6 @@ main([[maybe_unused]] int argc, const char **argv) // metric commands metric_command.add_command("get", "Get one or more metric values", "", MORE_THAN_ONE_ARG_N, [&]() { command->execute(); }) .add_example_usage("traffic_ctl metric get METRIC [METRIC ...]"); - metric_command.add_command("clear", "Clear all metric values", [&]() { command->execute(); }); metric_command.add_command("describe", "Show detailed information about one or more metric values", "", MORE_THAN_ONE_ARG_N, [&]() { command->execute(); }); // not implemented metric_command.add_command("match", "Get metrics matching a regular expression", "", MORE_THAN_ZERO_ARG_N, @@ -158,7 +157,6 @@ main([[maybe_unused]] int argc, const char **argv) "Terminate execution after requesting <count> metrics. If 0 is passed, program should be terminated by a SIGINT", "", 1, "0") .add_option("--interval", "-i", "Wait interval seconds between sending each metric request. Minimum value is 1s.", "", 1, "5"); - metric_command.add_command("zero", "Clear one or more metric values", "", MORE_THAN_ONE_ARG_N, [&]() { command->execute(); }); // plugin command plugin_command diff --git a/src/traffic_server/RpcAdminPubHandlers.cc b/src/traffic_server/RpcAdminPubHandlers.cc index f596260bfe..99a2b20120 100644 --- a/src/traffic_server/RpcAdminPubHandlers.cc +++ b/src/traffic_server/RpcAdminPubHandlers.cc @@ -43,10 +43,6 @@ register_admin_jsonrpc_handlers() using namespace rpc::handlers::records; rpc::add_method_handler("admin_lookup_records", &lookup_records, &core_ats_rpc_service_provider_handle, {{rpc::NON_RESTRICTED_API}}); - rpc::add_method_handler("admin_clear_all_metrics_records", &clear_all_metrics_records, &core_ats_rpc_service_provider_handle, - {{rpc::RESTRICTED_API}}); - rpc::add_method_handler("admin_clear_metrics_records", &clear_metrics_records, &core_ats_rpc_service_provider_handle, - {{rpc::RESTRICTED_API}}); // plugin using namespace rpc::handlers::plugins; diff --git a/tests/gold_tests/jsonrpc/json/admin_clear_metrics_records_req.json b/tests/gold_tests/jsonrpc/json/admin_clear_metrics_records_req.json deleted file mode 100644 index 24881bfa93..0000000000 --- a/tests/gold_tests/jsonrpc/json/admin_clear_metrics_records_req.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "id":"0ea2672f-b369-4d99-8d46-98c3fadc152d", - "jsonrpc":"2.0", - "method":"admin_clear_metrics_records", - "params":[ - { - "record_name":"$record_name" - } - ] -} \ No newline at end of file diff --git a/tests/gold_tests/jsonrpc/jsonrpc_api_schema.test.py b/tests/gold_tests/jsonrpc/jsonrpc_api_schema.test.py index 30fa8ecbd1..e6532e5947 100644 --- a/tests/gold_tests/jsonrpc/jsonrpc_api_schema.test.py +++ b/tests/gold_tests/jsonrpc/jsonrpc_api_schema.test.py @@ -153,12 +153,6 @@ add_testrun_for_jsonrpc_request( request_file_name='json/admin_config_reload_req.json', result_schema_file_name=success_schema_file_name_name) -# admin_clear_metrics_records -add_testrun_for_jsonrpc_request( - "Clear admin_clear_metrics_records", - request_file_name='json/admin_clear_metrics_records_req.json', - context={'record_name': 'proxy.process.http.404_responses'}) - # admin_host_set_status add_testrun_for_jsonrpc_request( "Test admin_host_set_status",
