Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

display build URL for changepoint #15

Merged
merged 7 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions orion.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
"""
This is the cli file for orion, tool to detect regressions using hunter
"""

# pylint: disable = import-error
import sys
import warnings
from functools import reduce
import logging
import os
import re
import pyshorteners

import click
import pandas as pd

from fmatch.matcher import Matcher
from utils import orion_funcs

warnings.filterwarnings("ignore", message="Unverified HTTPS request.*")

@click.group()
# pylint: disable=unused-argument
Expand All @@ -22,7 +26,8 @@ def cli(max_content_width=120):
cli function to group commands
"""

# pylint: disable=too-many-locals

# pylint: disable=too-many-locals, too-many-statements
@click.command()
@click.option("--uuid", default="", help="UUID to use as base for comparisons")
@click.option("--baseline", default="", help="Baseline UUID(s) to to compare against uuid")
Expand Down Expand Up @@ -50,42 +55,45 @@ def orion(**kwargs):
ES_URL=None

if "ES_SERVER" in data.keys():
ES_URL = data['ES_SERVER']
ES_URL = data["ES_SERVER"]
else:
if 'ES_SERVER' in os.environ:
ES_URL=os.environ.get("ES_SERVER")
if "ES_SERVER" in os.environ:
ES_URL = os.environ.get("ES_SERVER")
else:
logger.error("ES_SERVER environment variable/config variable not set")
sys.exit(1)

shortener = pyshorteners.Shortener()
for test in data["tests"]:
uuid = kwargs["uuid"]
baseline = kwargs["baseline"]
match = Matcher(index="perf_scale_ci", level=level, ES_URL=ES_URL)
match = Matcher(index="ospst-perf-scale-ci-*",
level=level, ES_URL=ES_URL, verify_certs=False)
if uuid == "":
metadata = orion_funcs.get_metadata(test, logger)
else:
metadata = orion_funcs.filter_metadata(uuid,match,logger)

logger.info("The test %s has started", test["name"])
if baseline == "":
uuids = match.get_uuid_by_metadata(metadata)
runs = match.get_uuid_by_metadata(metadata)
uuids = [run["uuid"] for run in runs]
buildUrls = {run["uuid"]: run["buildUrl"] for run in runs}
if len(uuids) == 0:
logging.info("No UUID present for given metadata")
sys.exit()
else:
uuids = [uuid for uuid in re.split(' |,',baseline) if uuid]
uuids.append(uuid)
if metadata["benchmark.keyword"] == "k8s-netperf" :
if metadata["benchmark.keyword"] == "ospst-k8s-netperf" :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are going to need both ospst-k8s-netperf and k8s-netperf to work depending on which ES_SERVER we are looking at. Can we change the if statement to if "k8s-netperf" in metadata["benchmark.keyword"]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shashank-boyapally is work for allowing multiple ES_SERVERs part of a different PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The discussion in the slack channel for allowing multiple ES_SERVERs in the same run is not yet developed, this just take cares for the internal instance changes we had.. will be developing that once I understand the further requirements more clearly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think the if "k8s-netperf" in metadata["benchmark.keyword"] would be better because this update breaks the usage of the QE elastic search index completely. This change would allow both to still work

Copy link
Contributor Author

@shashank-boyapally shashank-boyapally Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll keep it as the previous, but what index would we like to search is to be decided, as I believe in QE instance the index is k8s-netperf and in the internal instance it is ospst-k8s-netperf, should we include this index in the config file itself for better flexibility?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I understand the index is different between QE instance and the internal instance. The code block I put in previous comments will help cover both instances. I think setting in the config will make it less flexible as we will have to have multiple files to pull the same type of data from each of the instances. That might need to be something addressed with the pr for setting multiple ES_SERVERS

index = "k8s-netperf"
ids = uuids
elif metadata["benchmark.keyword"] == "ingress-perf" :
elif metadata["benchmark.keyword"] == "ospst-ingress-perf":
index = "ingress-performance"
ids = uuids
else:
index = "ripsaw-kube-burner"
index = "ospst-ripsaw-kube-burner*"
Copy link
Collaborator

@paigerube14 paigerube14 Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add the same starting * as k8s-netperf and ingress-perf here

*ripsaw-kube-burner*

if baseline == "":
runs = match.match_kube_burner(uuids)
runs = match.match_kube_burner(uuids, index)
ids = match.filter_runs(runs, runs)
else:
ids = uuids
Expand All @@ -98,13 +106,16 @@ def orion(**kwargs):
dataframe_list,
)

shortener = pyshorteners.Shortener()
merged_df["buildUrl"] = merged_df["uuid"].apply(
lambda uuid: shortener.tinyurl.short(buildUrls[uuid])) #pylint: disable = cell-var-from-loop
csv_name = kwargs["output"].split(".")[0]+"-"+test['name']+".csv"
match.save_results(
merged_df, csv_file_path=csv_name
)

if kwargs["hunter_analyze"]:
orion_funcs.run_hunter_analyze(merged_df,test)
_ = orion_funcs.run_hunter_analyze(merged_df,test)


if __name__ == "__main__":
Expand Down
3 changes: 2 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ certifi==2023.11.17
click==8.1.7
elastic-transport==8.11.0
elasticsearch==7.13.0
fmatch==0.0.5
fmatch==0.0.7
python-dateutil==2.8.2
pytz==2023.3.post1
PyYAML==6.0.1
six==1.16.0
tzdata==2023.4
urllib3==1.26.18
pyshorteners==1.0.1
8 changes: 5 additions & 3 deletions utils/orion_funcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ def run_hunter_analyze(merged_df,test):
merged_df["timestamp"] = merged_df["timestamp"].astype(int) // 10**9
metrics = {column: Metric(1, 1.0)
for column in merged_df.columns
if column not in ["uuid","timestamp"]}
if column not in ["uuid","timestamp","buildUrl"]}
data = {column: merged_df[column]
for column in merged_df.columns
if column not in ["uuid","timestamp"]}
attributes={column: merged_df[column] for column in merged_df.columns if column in ["uuid"]}
if column not in ["uuid","timestamp","buildUrl"]}
attributes={column: merged_df[column]
for column in merged_df.columns if column in ["uuid","buildUrl"]}
series=Series(
test_name=test["name"],
branch=None,
Expand All @@ -42,6 +43,7 @@ def run_hunter_analyze(merged_df,test):
report=Report(series,change_points)
output = report.produce_report(test_name="test",report_type=ReportType.LOG)
print(output)
return change_points

# pylint: disable=too-many-locals
def get_metric_data(ids, index, metrics, match, logger):
Expand Down
Loading