From ff11a9ef27d06761dc5d4242d6ceb3aa6b6d0371 Mon Sep 17 00:00:00 2001 From: filipecosta90 Date: Thu, 7 Nov 2024 01:26:51 +0000 Subject: [PATCH] Ensuring we produce a valid JSON in case of empty string and -nan --- JSON_handler.cpp | 36 ++++++++++++++++++++++++++++++++++-- run_stats.cpp | 25 +++++++++++++++++++------ run_stats_types.cpp | 18 ++++++++++-------- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/JSON_handler.cpp b/JSON_handler.cpp index b484a61b..030afaa6 100644 --- a/JSON_handler.cpp +++ b/JSON_handler.cpp @@ -22,7 +22,7 @@ #include #include - +#include // For std::isnan #include "JSON_handler.h" @@ -71,7 +71,39 @@ void json_handler::write_obj(const char * objectname, const char * format, ...) fprintf(m_json_file, "\"%s\": ", objectname); va_list argptr; va_start(argptr, format); - vfprintf(m_json_file, format, argptr); + // Check if format includes "%s", indicating a string is expected + if (strstr(format, "%s") != nullptr) { + // Use a temporary va_list to check the string argument without advancing the original + va_list tmp_argptr; + va_copy(tmp_argptr, argptr); + const char *str_arg = va_arg(tmp_argptr, const char*); + va_end(tmp_argptr); + + if (str_arg == nullptr) { + // Handle NULL strings by writing "null" to the JSON file + fprintf(m_json_file, "null"); + } else { + // Print the valid string argument + vfprintf(m_json_file, format, argptr); + } + } + // Check if the format expects a floating-point number + else if (strstr(format, "f") != nullptr || strstr(format, "e") != nullptr || strstr(format, "g") != nullptr) { + va_list tmp_argptr; + va_copy(tmp_argptr, argptr); + double value = va_arg(tmp_argptr, double); + va_end(tmp_argptr); + + if (std::isnan(value)) { + fprintf(m_json_file, "null"); + } else { + vfprintf(m_json_file, format, argptr); + } + } + else { + // For other format specifiers, proceed as usual + vfprintf(m_json_file, format, argptr); + } va_end(argptr); beutify(); fprintf(m_json_file, ","); diff --git a/run_stats.cpp b/run_stats.cpp index 2d31acd3..08470290 100644 --- a/run_stats.cpp +++ b/run_stats.cpp @@ -879,16 +879,19 @@ void run_stats::summarize(totals& result) const result.m_ask_sec = (double) (totals.m_set_cmd.m_ask + totals.m_get_cmd.m_ask) / test_duration_usec * 1000000; } -void result_print_to_json(json_handler * jsonhandler, const char * type, double ops, +void result_print_to_json(json_handler * jsonhandler, const char * type, double ops_sec, double hits, double miss, double moved, double ask, double kbs, double kbs_rx, double kbs_tx, + double latency, long ops, std::vector quantile_list, struct hdr_histogram* latency_histogram, std::vector timestamps, std::vector timeserie_stats ) { if (jsonhandler != NULL){ // Added for double verification in case someone accidently send NULL. jsonhandler->open_nesting(type); - jsonhandler->write_obj("Count","%lld", hdr_total_count(latency_histogram)); - jsonhandler->write_obj("Ops/sec","%.2f", ops); + const long hdr_count = hdr_total_count(latency_histogram); + assert(hdr_count < ops && "reported ops and tracked ops via hdrhistogram are same"); + jsonhandler->write_obj("Count","%lld", hdr_count); + jsonhandler->write_obj("Ops/sec","%.2f", ops_sec); jsonhandler->write_obj("Hits/sec","%.2f", hits); jsonhandler->write_obj("Misses/sec","%.2f", miss); @@ -899,9 +902,9 @@ void result_print_to_json(json_handler * jsonhandler, const char * type, double jsonhandler->write_obj("ASK/sec","%.2f", ask); const bool has_samples = hdr_total_count(latency_histogram)>0; - const double avg_latency = has_samples ? hdr_mean(latency_histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; - const double min_latency = has_samples ? hdr_min(latency_histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; - const double max_latency = has_samples ? hdr_max(latency_histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; + const double avg_latency = latency; + const double min_latency = has_samples ? (hdr_min(latency_histogram) * 1.0)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; + const double max_latency = has_samples ? (hdr_max(latency_histogram) * 1.0)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; // to be retrocompatible jsonhandler->write_obj("Latency","%.3f", avg_latency); jsonhandler->write_obj("Average Latency","%.3f", avg_latency); @@ -1240,6 +1243,8 @@ void run_stats::print_json(json_handler *jsonhandler, arbitrary_command_list& co m_totals.m_ar_commands[i].m_bytes_sec, m_totals.m_ar_commands[i].m_bytes_sec_rx, m_totals.m_ar_commands[i].m_bytes_sec_tx, + m_totals.m_ar_commands[i].m_latency, + m_totals.m_ar_commands[i].m_ops, quantiles_list, arbitrary_command_latency_histogram, timestamps, @@ -1258,6 +1263,8 @@ void run_stats::print_json(json_handler *jsonhandler, arbitrary_command_list& co m_totals.m_set_cmd.m_bytes_sec, m_totals.m_set_cmd.m_bytes_sec_rx, m_totals.m_set_cmd.m_bytes_sec_tx, + m_totals.m_set_cmd.m_latency, + m_totals.m_set_cmd.m_ops, quantiles_list, m_set_latency_histogram, timestamps, @@ -1271,6 +1278,8 @@ void run_stats::print_json(json_handler *jsonhandler, arbitrary_command_list& co m_totals.m_get_cmd.m_bytes_sec, m_totals.m_get_cmd.m_bytes_sec_rx, m_totals.m_get_cmd.m_bytes_sec_tx, + m_totals.m_get_cmd.m_latency, + m_totals.m_get_cmd.m_ops, quantiles_list, m_get_latency_histogram, timestamps, @@ -1284,6 +1293,8 @@ void run_stats::print_json(json_handler *jsonhandler, arbitrary_command_list& co 0.0, 0.0, 0.0, + 0.0, + m_totals.m_wait_cmd.m_ops, quantiles_list, m_wait_latency_histogram, timestamps, @@ -1299,6 +1310,8 @@ void run_stats::print_json(json_handler *jsonhandler, arbitrary_command_list& co m_totals.m_bytes_sec, m_totals.m_bytes_sec_rx, m_totals.m_bytes_sec_tx, + m_totals.m_latency, + m_totals.m_ops, quantiles_list, m_totals.latency_histogram, timestamps, diff --git a/run_stats_types.cpp b/run_stats_types.cpp index bf634766..5d5be1a9 100644 --- a/run_stats_types.cpp +++ b/run_stats_types.cpp @@ -23,7 +23,7 @@ #include #include "run_stats_types.h" - +#include one_sec_cmd_stats::one_sec_cmd_stats() : @@ -36,8 +36,8 @@ one_sec_cmd_stats::one_sec_cmd_stats() : m_ask(0), m_total_latency(0), m_avg_latency(0.0), - m_min_latency(0.0), - m_max_latency(0.0) { + m_min_latency(std::numeric_limits::max()), + m_max_latency(std::numeric_limits::lowest()) { } @@ -51,8 +51,8 @@ void one_sec_cmd_stats::reset() { m_ask = 0; m_total_latency = 0; m_avg_latency = 0; - m_max_latency = 0; - m_min_latency = 0; + m_max_latency = std::numeric_limits::lowest(); + m_min_latency = std::numeric_limits::max(); summarized_quantile_values.clear(); } @@ -65,7 +65,9 @@ void one_sec_cmd_stats::merge(const one_sec_cmd_stats& other) { m_moved += other.m_moved; m_ask += other.m_ask; m_total_latency += other.m_total_latency; - m_avg_latency = (double) m_total_latency / (double) m_ops / (double) LATENCY_HDR_RESULTS_MULTIPLIER; + if (m_ops > 0) { + m_avg_latency = (double) m_total_latency / (double) m_ops / (double) LATENCY_HDR_RESULTS_MULTIPLIER; + } m_max_latency = other.m_max_latency > m_max_latency ? other.m_max_latency : m_max_latency; m_min_latency = other.m_min_latency < m_min_latency ? other.m_min_latency : m_min_latency; } @@ -78,8 +80,8 @@ void one_sec_cmd_stats::summarize_quantiles(safe_hdr_histogram histogram, std::v summarized_quantile_values.push_back(value); } m_avg_latency = has_samples ? hdr_mean(histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; - m_max_latency = has_samples ? hdr_max(histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; - m_min_latency = has_samples ? hdr_min(histogram)/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : 0.0; + m_max_latency = has_samples ? (1.0 * hdr_max(histogram))/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : std::numeric_limits::lowest(); + m_min_latency = has_samples ? (1.0 * hdr_min(histogram))/ (double) LATENCY_HDR_RESULTS_MULTIPLIER : std::numeric_limits::max();; } void one_sec_cmd_stats::update_op(unsigned int bytes_rx, unsigned int bytes_tx, unsigned int latency) {