Improved overall stability of tool on local and remote executions. (#62)

* [add] Improved overall stability of tool on local and remote executions. Properly parsing git vars. Properly handling missing branch info.

Author: filipecosta90

.github/workflows/publish-pypi.yml

@@ -1,7 +1,7 @@
 name: Publish Pypi
 on:
   release:
-    types: [published]
+    types: [ published ]
 
 jobs:
   pytest:

README.md

@@ -2,10 +2,12 @@
 ![Actions](https://github.com/RedisLabsModules/redisbench-admin/workflows/Run%20Tests/badge.svg?branch=master)
 ![Actions](https://badge.fury.io/py/redisbench-admin.svg)
 
-# [redisbench-admin](https://github.com/RedisLabsModules/redisbench-admin) 
+# [redisbench-admin](https://github.com/RedisLabsModules/redisbench-admin)
 
 Redis benchmark run helper can help you with the following tasks:
-- Setup abd teardown of benchmarking infrastructure specified on [RedisLabsModules/testing-infrastructure](https://github.com/RedisLabsModules/testing-infrastructure)
+
+- Setup abd teardown of benchmarking infrastructure specified
+  on [RedisLabsModules/testing-infrastructure](https://github.com/RedisLabsModules/testing-infrastructure)
 - Setup and teardown of an Redis and Redis Modules DBs for benchmarking
 - Management of benchmark data and specifications across different setups
 - Running benchmarks and recording results
@@ -14,6 +16,7 @@ Redis benchmark run helper can help you with the following tasks:
 - [SOON] Finding performance problems by attaching telemetry probes
 
 Current supported benchmark tools:
+
 - [redisgraph-benchmark-go](https://github.com/RedisGraph/redisgraph-benchmark-go)
 - [ftsb_redisearch](https://github.com/RediSearch/ftsb)
 - [redis-benchmark](https://github.com/redis/redis)

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "redisbench-admin"
-version = "0.1.53"
+version = "0.1.54"
 description = "Redis benchmark run helper. A wrapper around Redis and Redis Modules benchmark tools ( ftsb_redisearch, memtier_benchmark, redis-benchmark, aibench, etc... )."
 authors = ["filipecosta90 <[email protected]>"]
 readme = "README.md"

redisbench_admin/run/common.py

@@ -1,9 +1,14 @@
 import logging
 import re
 
+import yaml
+
 from redisbench_admin.run.redis_benchmark.redis_benchmark import prepareRedisBenchmarkCommand
 from redisbench_admin.run.redisgraph_benchmark_go.redisgraph_benchmark_go import prepareRedisGraphBenchmarkGoCommand
-from redisbench_admin.utils.remote import executeRemoteCommands, getFileFromRemoteSetup
+from redisbench_admin.utils.benchmark_config import parseExporterMetricsDefinition, parseExporterTimeMetricDefinition, \
+    parseExporterTimeMetric
+from redisbench_admin.utils.remote import executeRemoteCommands, getFileFromRemoteSetup, extractRedisGraphVersion, \
+    extractPerVersionTimeSeriesFromResults, pushDataToRedisTimeSeries, extractPerBranchTimeSeriesFromResults
 
 
 def extract_benchmark_tool_settings(benchmark_config):
@@ -35,9 +40,9 @@ def prepare_benchmark_parameters(benchmark_config, benchmark_tool, server_plaint
                                  remote_results_file, isremote=False):
     for entry in benchmark_config["clientconfig"]:
         if 'parameters' in entry:
-            if benchmark_tool == 'redis-benchmark':
+            if 'redis-benchmark' in benchmark_tool:
                 command_arr, command_str = prepareRedisBenchmarkCommand(
-                    "redis-benchmark",
+                    benchmark_tool,
                     server_private_ip,
                     server_plaintext_port,
                     entry
@@ -46,13 +51,16 @@ def prepare_benchmark_parameters(benchmark_config, benchmark_tool, server_plaint
                 command_arr.append(redirect_file)
                 command_str = command_str + " " + redirect_file
 
-            if benchmark_tool == 'redisgraph-benchmark-go':
+            if 'redisgraph-benchmark-go' in benchmark_tool:
+                if isremote is True:
+                    benchmark_tool = "/tmp/redisgraph-benchmark-go"
                 command_arr = prepareRedisGraphBenchmarkGoCommand(
-                    "/tmp/redisgraph-benchmark-go",
+                    benchmark_tool,
                     server_private_ip,
                     server_plaintext_port,
                     entry,
                     remote_results_file,
+                    isremote
                 )
                 command_str = " ".join(command_arr)
     return command_arr, command_str
@@ -87,3 +95,125 @@ def runRemoteBenchmark(
             remote_results_file,
         )
     return remote_run_result
+
+
+def merge_default_and_specific_properties_dictType(benchmark_config, default_properties, propertygroup_keyname,
+                                                   usecase_filename):
+    if propertygroup_keyname not in benchmark_config:
+        benchmark_config[propertygroup_keyname] = default_properties
+        logging.info(
+            "Using exclusively default '{}' properties (total={}) given the file {} had no '{}' property group".format(
+                propertygroup_keyname, len(benchmark_config[propertygroup_keyname].keys()), usecase_filename,
+                propertygroup_keyname)
+        )
+    else:
+        use_case_specific_properties = benchmark_config[propertygroup_keyname]
+        for default_property in default_properties:
+            default_rule, default_details = list(default_property.items())[0]
+            default_condition = list(default_details.values())[0]
+            comparison_key = "{}{}".format(default_rule, default_condition)
+            found = False
+            found_details = None
+            for usecase_kpi in use_case_specific_properties:
+                usecase_rule, usecase_details = list(usecase_kpi.items())[0]
+                usecase_condition = list(usecase_details.values())[0]
+                usecase_comparison_key = "{}{}".format(usecase_rule, usecase_condition)
+                if comparison_key == usecase_comparison_key:
+                    found = True
+                    found_details = usecase_details
+            if found:
+                logging.info(
+                    "Skipping to add default '{}' property ({}) given the file {} had the same specific property ({})".format(
+                        propertygroup_keyname,
+                        default_property, usecase_filename, usecase_kpi)
+                )
+            else:
+                use_case_specific_properties.append(default_property)
+                logging.info(
+                    "Adding a default '{}' property ({}) given the file {} did not had the specific property".format(
+                        propertygroup_keyname, default_property, usecase_filename)
+                )
+
+
+def process_default_yaml_properties_file(default_kpis, default_metrics, defaults_filename, exporter_timemetric_path,
+                                         stream):
+    default_config = yaml.safe_load(stream)
+    if "exporter" in default_config:
+        default_metrics = parseExporterMetricsDefinition(default_config["exporter"])
+        if len(default_metrics) > 0:
+            logging.info(
+                "Found RedisTimeSeries default metrics specification. Will include the following metrics on all benchmarks {}".format(
+                    " ".join(default_metrics)
+                )
+            )
+        exporter_timemetric_path = parseExporterTimeMetricDefinition(
+            default_config["exporter"]
+        )
+        if exporter_timemetric_path is not None:
+            logging.info(
+                "Found RedisTimeSeries default time metric specification. Will use the following JSON path to retrieve the test time {}".format(
+                    exporter_timemetric_path
+                )
+            )
+    if "kpis" in default_config:
+        logging.info(
+            "Loading default KPIs specifications from file: {}".format(defaults_filename)
+        )
+        default_kpis = default_config["kpis"]
+    return default_kpis, default_metrics, exporter_timemetric_path
+
+
+def common_exporter_logic(deployment_type, exporter_timemetric_path, metrics, results_dict, rts, test_name,
+                          tf_github_branch, tf_github_org, tf_github_repo, tf_triggering_env):
+    if exporter_timemetric_path is not None and len(metrics) > 0:
+        # extract timestamp
+        datapoints_timestamp = parseExporterTimeMetric(
+            exporter_timemetric_path, results_dict
+        )
+
+        rg_version = extractRedisGraphVersion(results_dict)
+        if rg_version is None:
+            rg_version = "N/A"
+
+        # extract per branch datapoints
+        (
+            ok,
+            per_version_time_series_dict,
+        ) = extractPerVersionTimeSeriesFromResults(
+            datapoints_timestamp,
+            metrics,
+            results_dict,
+            rg_version,
+            tf_github_org,
+            tf_github_repo,
+            deployment_type,
+            test_name,
+            tf_triggering_env,
+        )
+
+        # push per-branch data
+        pushDataToRedisTimeSeries(rts, per_version_time_series_dict)
+        if tf_github_branch != None and tf_github_branch != '':
+            # extract per branch datapoints
+            ok, branch_time_series_dict = extractPerBranchTimeSeriesFromResults(
+                datapoints_timestamp,
+                metrics,
+                results_dict,
+                str(tf_github_branch),
+                tf_github_org,
+                tf_github_repo,
+                deployment_type,
+                test_name,
+                tf_triggering_env,
+            )
+            # push per-branch data
+            pushDataToRedisTimeSeries(rts, branch_time_series_dict)
+        else:
+            logging.warning(
+                "Requested to push data to RedisTimeSeries but no git branch definition was found. git branch value {}".format(
+                    tf_github_branch)
+            )
+    else:
+        logging.error(
+            "Requested to push data to RedisTimeSeries but no exporter definition was found. Missing \"exporter\" config."
+        )

redisbench_admin/run/redisgraph_benchmark_go/redisgraph_benchmark_go.py

@@ -7,6 +7,7 @@ def prepareRedisGraphBenchmarkGoCommand(
         server_plaintext_port: object,
         benchmark_config: object,
         results_file: object,
+        is_remote: bool = False,
 ):
     """
     Prepares redisgraph-benchmark-go command parameters
@@ -19,7 +20,11 @@ def prepareRedisGraphBenchmarkGoCommand(
     queries_str = [executable_path]
     for k in benchmark_config["parameters"]:
         if "graph" in k:
-            queries_str.extend(["-graph-key", "'{}'".format(k["graph"])])
+            if is_remote:
+                graph_key = "'{}'".format(k["graph"])
+            else:
+                graph_key = k["graph"]
+            queries_str.extend(["-graph-key", graph_key])
         if "clients" in k:
             queries_str.extend(["-c", "{}".format(k["clients"])])
         if "requests" in k:
@@ -28,8 +33,11 @@ def prepareRedisGraphBenchmarkGoCommand(
             queries_str.extend(["-rps", "{}".format(k["rps"])])
         if "queries" in k:
             for kk in k["queries"]:
-                query = kk["q"]
-                queries_str.extend(["-query", "'{}'".format(query)])
+                if is_remote:
+                    query = "'{}'".format(kk["q"])
+                else:
+                    query = kk["q"]
+                queries_str.extend(["-query", query])
                 if "ratio" in kk:
                     queries_str.extend(["-query-ratio", "{}".format(kk["ratio"])])
     queries_str.extend(["-h", "{}".format(server_private_ip)])

redisbench_admin/run_local/run_local.py

@@ -3,12 +3,14 @@
 import logging
 import os
 import pathlib
+import shutil
 import subprocess
 import sys
 
 import yaml
 
-from redisbench_admin.run.common import extract_benchmark_tool_settings, prepare_benchmark_parameters
+from redisbench_admin.run.common import extract_benchmark_tool_settings, prepare_benchmark_parameters, \
+    merge_default_and_specific_properties_dictType, process_default_yaml_properties_file
 from redisbench_admin.run.redis_benchmark.redis_benchmark import redis_benchmark_from_stdout_csv_to_json, \
     redis_benchmark_ensure_min_version_local
 from redisbench_admin.utils.local import (
@@ -41,23 +43,44 @@ def run_local_command_logic(args):
 
     return_code = 0
     files = []
+    default_metrics = []
+    exporter_timemetric_path = None
+    defaults_filename = "defaults.yml"
+    default_kpis = None
+    if os.path.exists(defaults_filename):
+        with open(defaults_filename, "r") as stream:
+            logging.info(
+                "Loading default specifications from file: {}".format(defaults_filename)
+            )
+            default_kpis, default_metrics, exporter_timemetric_path = process_default_yaml_properties_file(default_kpis,
+                                                                                                           default_metrics,
+                                                                                                           defaults_filename,
+                                                                                                           exporter_timemetric_path,
+                                                                                                           stream)
+
     if args.test == "":
         files = pathlib.Path().glob("*.yml")
         files = [str(x) for x in files]
-        if "defaults.yml" in files:
-            files.remove("defaults.yml")
+        if defaults_filename in files:
+            files.remove(defaults_filename)
+
         logging.info(
             "Running all specified benchmarks: {}".format(" ".join([str(x) for x in files]))
         )
     else:
         logging.info("Running specific benchmark in file: {}".format(args.test))
         files = [args.test]
 
-    for f in files:
-        with open(f, "r") as stream:
-            dirname = os.path.dirname(os.path.abspath(f))
+    for usecase_filename in files:
+        with open(usecase_filename, "r") as stream:
+            dirname = os.path.dirname(os.path.abspath(usecase_filename))
             redis_process = None
             benchmark_config = yaml.safe_load(stream)
+            kpis_keyname = "kpis"
+            if default_kpis != None:
+                merge_default_and_specific_properties_dictType(benchmark_config, default_kpis, kpis_keyname,
+                                                               usecase_filename)
+
             test_name = benchmark_config["name"]
             # after we've spinned Redis, even on error we should always teardown
             # in case of some unexpected error we fail the test
@@ -91,6 +114,14 @@ def run_local_command_logic(args):
                 else:
                     raise Exception("Unable to detect benchmark tool within 'clientconfig' section. Aborting...")
 
+                if benchmark_tool is not None:
+                    logging.info("Checking benchmark tool {} is accessible".format(benchmark_tool))
+                    which_benchmark_tool = shutil.which(benchmark_tool)
+                    if which_benchmark_tool is None:
+                        raise Exception("Benchmark tool was not accesible. Aborting...")
+                    else:
+                        logging.info("Tool {} was detected at {}".format(benchmark_tool, which_benchmark_tool))
+
                 if benchmark_tool not in args.allowed_tools.split(","):
                     raise Exception(
                         "Benchmark tool {} not in the allowed tools list [{}]. Aborting...".format(benchmark_tool,
@@ -101,8 +132,10 @@ def run_local_command_logic(args):
                                                              benchmark_min_tool_version_major,
                                                              benchmark_min_tool_version_minor,
                                                              benchmark_min_tool_version_patch)
+
+                # prepare the benchmark command
                 command, command_str = prepare_benchmark_parameters(benchmark_config, benchmark_tool, args.port,
-                                                                    "localhost", local_benchmark_output_filename)
+                                                                    "localhost", local_benchmark_output_filename, False)
 
                 # run the benchmark
                 if benchmark_tool == 'redis-benchmark':

redisbench_admin/run_remote/args.py

@@ -22,7 +22,7 @@ def create_run_remote_arguments(parser):
     parser.add_argument("--github_repo", type=str, default=None)
     parser.add_argument("--github_org", type=str, default=None)
     parser.add_argument("--github_sha", type=str, default=None)
-    parser.add_argument("--github_branch", type=str, default=None)
+    parser.add_argument("--github_branch", type=str, default=None, nargs='?', const='')
     parser.add_argument("--triggering_env", type=str, default=socket.gethostname())
     parser.add_argument("--terraform_bin_path", type=str, default=TERRAFORM_BIN_PATH)
     parser.add_argument("--setup_name_sufix", type=str, default="")