Skip to content

Cherry pick some dropped Gerrrr commits #127

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.

Sign up for GitHub

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

Closed
StephanDollberg wants to merge 2 commits into from
Closed

Cherry pick some dropped Gerrrr commits #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbf60b7
Add --ignore-direction to regressions subcommand
mfleming Mar 18, 2024
98ed1f5
Make regressions subcommand support --output param
mfleming Mar 18, 2024
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
79 changes: 48 additions & 31 deletions otava/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from otava.graphite import GraphiteError
from otava.importer import DataImportError, Importers
from otava.postgres import Postgres, PostgresError
from otava.report import Report, ReportType
from otava.report import ChangePointReport, RegressionsReport, ReportType
from otava.series import AnalysisOptions, AnalyzedSeries, compare
from otava.slack import NotificationError, SlackNotifier
from otava.test_config import (
Expand Down Expand Up @@ -123,7 +123,7 @@ def analyze(
series = importer.fetch_data(test, selector)
analyzed_series = series.analyze(options)
change_points = analyzed_series.change_points_by_time
report = Report(series, change_points)
report = ChangePointReport(series, change_points)
produced_report = report.produce_report(test.name, report_type)
print(produced_report)
return analyzed_series
Expand Down Expand Up @@ -257,7 +257,12 @@ def update_bigquery(self, test: BigQueryTestConfig, series: AnalyzedSeries):
bigquery.insert_change_point(test, metric_name, attributes, cp)

def regressions(
self, test: TestConfig, selector: DataSelector, options: AnalysisOptions
self,
test: TestConfig,
selector: DataSelector,
options: AnalysisOptions,
report_type: ReportType,
ignore_direction: bool = False,
) -> bool:
importer = self.__importers.get(test)

Expand Down Expand Up @@ -305,20 +310,17 @@ def regressions(
direction = baseline_series.metric(metric_name).direction
m1 = stats.mean_1
m2 = stats.mean_2
change_percent = stats.forward_rel_change() * 100.0
if m2 * direction < m1 * direction and stats.pvalue < options.max_pvalue:
regressions.append(
" {:16}: {:#8.3g} --> {:#8.3g} ({:+6.1f}%)".format(
metric_name, m1, m2, change_percent
)
)
if ignore_direction:
mean_diff = m2 != m1
else:
mean_diff = m2 * direction < m1 * direction

if regressions:
print(f"{test.name}:")
for r in regressions:
print(r)
else:
print(f"{test.name}: OK")
if mean_diff and stats.pvalue < options.max_pvalue:
regressions.append((metric_name, stats))

report = RegressionsReport(regressions)
produced_report = report.produce_report(test.name, report_type)
print(produced_report)
return len(regressions) > 0

def __maybe_create_slack_notifier(self):
Expand Down Expand Up @@ -499,6 +501,14 @@ def setup_analysis_options_parser(parser: argparse.ArgumentParser):
help="use the original edivisive algorithm with no windowing "
"and weak change points analysis improvements",
)
parser.add_argument(
"--output",
help="Output format for the generated report.",
choices=list(ReportType),
dest="report_type",
default=ReportType.LOG,
type=ReportType,
)


def analysis_options_from_args(args: argparse.Namespace) -> AnalysisOptions:
Expand Down Expand Up @@ -576,14 +586,6 @@ def create_otava_cli_parser() -> argparse.ArgumentParser:
metavar="DATE",
dest="cph_report_since",
)
analyze_parser.add_argument(
"--output",
help="Output format for the generated report.",
choices=list(ReportType),
dest="report_type",
default=ReportType.LOG,
type=ReportType,
)
setup_data_selector_parser(analyze_parser)
setup_analysis_options_parser(analyze_parser)

Expand All @@ -594,6 +596,12 @@ def create_otava_cli_parser() -> argparse.ArgumentParser:
regressions_parser.add_argument(
"tests", help="name of the test or group of the tests", nargs="+"
)
regressions_parser.add_argument(
"--ignore-direction",
help="ignore the direction of the change in performance",
dest="ignore_direction",
action="store_true",
)
config.add_service_option_groups(regressions_parser)
setup_data_selector_parser(regressions_parser)
setup_analysis_options_parser(regressions_parser)
Expand Down Expand Up @@ -695,7 +703,13 @@ def script_main(conf: Config = None, args: List[str] = None):
errors = 0
for test in tests:
try:
regressions = otava.regressions(test, selector=data_selector, options=options)
regressions = otava.regressions(
test,
selector=data_selector,
options=options,
ignore_direction=args.ignore_direction,
report_type=args.report_type,
)
if regressions:
regressing_test_count += 1
except OtavaError as err:
Expand All @@ -704,12 +718,15 @@ def script_main(conf: Config = None, args: List[str] = None):
except DataImportError as err:
logging.error(err.message)
errors += 1
if regressing_test_count == 0:
print("No regressions found!")
elif regressing_test_count == 1:
print("Regressions in 1 test found")
else:
print(f"Regressions in {regressing_test_count} tests found")

if args.report_type == ReportType.LOG:
if regressing_test_count == 0:
print("No regressions found!")
elif regressing_test_count == 1:
print("Regressions in 1 test found")
else:
print(f"Regressions in {regressing_test_count} tests found")

if errors > 0:
print("Some tests were skipped due to import / analyze errors. Consult error log.")

Expand Down
50 changes: 47 additions & 3 deletions otava/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
# specific language governing permissions and limitations
# under the License.

from collections import OrderedDict
from collections import OrderedDict, defaultdict
from enum import Enum, unique
from typing import List
from typing import List, Tuple

from tabulate import tabulate

from otava.analysis import TTestStats
from otava.series import ChangePointGroup, Series
from otava.util import format_timestamp, insert_multiple, remove_common_prefix

Expand All @@ -35,7 +36,7 @@ def __str__(self):
return self.value


class Report:
class ChangePointReport:
__series: Series
__change_points: List[ChangePointGroup]

Expand Down Expand Up @@ -133,3 +134,46 @@ def __format_regressions_only(self, test_name: str) -> str:
return f"Regressions in {test_name}:" + "\n" + "\n".join(output)
else:
return f"No regressions found in {test_name}."


class RegressionsReport:
def __init__(self, regressions: List[Tuple[str, TTestStats]]):
self.__regressions = regressions

def produce_report(self, test_name: str, report_type: ReportType):
if report_type == ReportType.LOG:
return self.__format_log(test_name)
elif report_type == ReportType.JSON:
return self.__format_json(test_name)
else:
from hunter.main import HunterError

raise HunterError(f"Unknown report type: {report_type}")

def __format_log(self, test_name: str) -> str:
if not self.__regressions:
return f"{test_name}: OK"

output = f"{test_name}:"
for metric_name, stats in self.__regressions:
m1 = stats.mean_1
m2 = stats.mean_2
change_percent = stats.forward_change_percent()
output += "\n {:16}: {:#8.3g} --> {:#8.3g} ({:+6.1f}%)".format(
metric_name, m1, m2, change_percent
)
return output

def __format_json(self, test_name: str) -> str:
import json

if not self.__regressions:
return json.dumps({test_name: []})

output = defaultdict(list)
for metric_name, stats in self.__regressions:
obj = {"metric": metric_name}
obj.update(stats.to_json())
output[test_name].append(obj)

return json.dumps(output)
53 changes: 29 additions & 24 deletions tests/cli_help_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,11 @@ def test_otava_analyze_help_output():
[--bigquery-credentials BIGQUERY_CREDENTIALS] [--update-grafana]
[--update-postgres] [--update-bigquery]
[--notify-slack NOTIFY_SLACK [NOTIFY_SLACK ...]] [--cph-report-since DATE]
[--output {log,json,regressions_only}] [--branch [STRING]] [--metrics LIST]
[--attrs LIST] [--since-commit STRING | --since-version STRING |
--since DATE] [--until-commit STRING | --until-version STRING | --until DATE]
[--last COUNT] [-P, --p-value PVALUE] [-M MAGNITUDE] [--window WINDOW]
[--orig-edivisive ORIG_EDIVISIVE]
[--branch [STRING]] [--metrics LIST] [--attrs LIST] [--since-commit STRING |
--since-version STRING | --since DATE] [--until-commit STRING |
--until-version STRING | --until DATE] [--last COUNT] [-P, --p-value PVALUE]
[-M MAGNITUDE] [--window WINDOW] [--orig-edivisive ORIG_EDIVISIVE]
[--output {log,json,regressions_only}]
tests [tests ...]

positional arguments:
Expand All @@ -184,8 +184,6 @@ def test_otava_analyze_help_output():
--cph-report-since DATE
Sets a limit on the date range of the Change Point History reported to
Slack. Same syntax as --since.
--output {log,json,regressions_only}
Output format for the generated report.
--branch [STRING] name of the branch
--metrics LIST a comma-separated list of metrics to analyze
--attrs LIST a comma-separated list of attribute names associated with the runs (e.g.
Expand Down Expand Up @@ -220,12 +218,11 @@ def test_otava_analyze_help_output():
[--bigquery-credentials BIGQUERY_CREDENTIALS] [--update-grafana]
[--update-postgres] [--update-bigquery]
[--notify-slack NOTIFY_SLACK [NOTIFY_SLACK ...]] [--cph-report-since DATE]
[--output {log,json,regressions_only}] [--branch [STRING]] [--metrics LIST]
[--attrs LIST]
[--branch [STRING]] [--metrics LIST] [--attrs LIST]
[--since-commit STRING | --since-version STRING | --since DATE]
[--until-commit STRING | --until-version STRING | --until DATE]
[--last COUNT] [-P, --p-value PVALUE] [-M MAGNITUDE] [--window WINDOW]
[--orig-edivisive ORIG_EDIVISIVE]
[--orig-edivisive ORIG_EDIVISIVE] [--output {log,json,regressions_only}]
tests [tests ...]

positional arguments:
Expand All @@ -242,8 +239,6 @@ def test_otava_analyze_help_output():
--cph-report-since DATE
Sets a limit on the date range of the Change Point History reported to
Slack. Same syntax as --since.
--output {log,json,regressions_only}
Output format for the generated report.
--branch [STRING] name of the branch
--metrics LIST a comma-separated list of metrics to analyze
--attrs LIST a comma-separated list of attribute names associated with the runs (e.g.
Expand All @@ -268,7 +263,8 @@ def test_otava_analyze_help_output():

assert (
result.stdout
== usage_and_options + """
== usage_and_options
+ """
minimum accepted magnitude of a change-point computed as abs(new_mean /
old_mean - 1.0); use it to filter out stupidly small changes like < 0.01
--window WINDOW the number of data points analyzed at once; the window size affects the
Expand All @@ -279,6 +275,8 @@ def test_otava_analyze_help_output():
--orig-edivisive ORIG_EDIVISIVE
use the original edivisive algorithm with no windowing and weak change
points analysis improvements
--output {log,json,regressions_only}
Output format for the generated report.

Graphite Options:
Options for Graphite configuration
Expand Down Expand Up @@ -341,10 +339,11 @@ def test_otava_regressions_help_output():
# Python 3.13+ formats usage lines and option aliases differently
if IS_PYTHON_313_PLUS:
usage_and_options = """\
usage: otava regressions [-h] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL]
[--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD]
[--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME]
[--postgres-port POSTGRES_PORT] [--postgres-username POSTGRES_USERNAME]
usage: otava regressions [-h] [--ignore-direction] [--graphite-url GRAPHITE_URL]
[--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER]
[--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN]
[--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT]
[--postgres-username POSTGRES_USERNAME]
[--postgres-password POSTGRES_PASSWORD]
[--postgres-database POSTGRES_DATABASE]
[--bigquery-project-id BIGQUERY_PROJECT_ID]
Expand All @@ -354,14 +353,15 @@ def test_otava_regressions_help_output():
--since-version STRING | --since DATE] [--until-commit STRING |
--until-version STRING | --until DATE] [--last COUNT]
[-P, --p-value PVALUE] [-M MAGNITUDE] [--window WINDOW]
[--orig-edivisive ORIG_EDIVISIVE]
[--orig-edivisive ORIG_EDIVISIVE] [--output {log,json,regressions_only}]
tests [tests ...]

positional arguments:
tests name of the test or group of the tests

options:
-h, --help show this help message and exit
--ignore-direction ignore the direction of the change in performance
--branch [STRING] name of the branch
--metrics LIST a comma-separated list of metrics to analyze
--attrs LIST a comma-separated list of attribute names associated with the runs (e.g.
Expand All @@ -385,10 +385,11 @@ def test_otava_regressions_help_output():
-M, --magnitude MAGNITUDE"""
else:
usage_and_options = """\
usage: otava regressions [-h] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL]
[--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD]
[--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME]
[--postgres-port POSTGRES_PORT] [--postgres-username POSTGRES_USERNAME]
usage: otava regressions [-h] [--ignore-direction] [--graphite-url GRAPHITE_URL]
[--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER]
[--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN]
[--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT]
[--postgres-username POSTGRES_USERNAME]
[--postgres-password POSTGRES_PASSWORD]
[--postgres-database POSTGRES_DATABASE]
[--bigquery-project-id BIGQUERY_PROJECT_ID]
Expand All @@ -398,14 +399,15 @@ def test_otava_regressions_help_output():
[--since-commit STRING | --since-version STRING | --since DATE]
[--until-commit STRING | --until-version STRING | --until DATE]
[--last COUNT] [-P, --p-value PVALUE] [-M MAGNITUDE] [--window WINDOW]
[--orig-edivisive ORIG_EDIVISIVE]
[--orig-edivisive ORIG_EDIVISIVE] [--output {log,json,regressions_only}]
tests [tests ...]

positional arguments:
tests name of the test or group of the tests

options:
-h, --help show this help message and exit
--ignore-direction ignore the direction of the change in performance
--branch [STRING] name of the branch
--metrics LIST a comma-separated list of metrics to analyze
--attrs LIST a comma-separated list of attribute names associated with the runs (e.g.
Expand All @@ -430,7 +432,8 @@ def test_otava_regressions_help_output():

assert (
result.stdout
== usage_and_options + """
== usage_and_options
+ """
minimum accepted magnitude of a change-point computed as abs(new_mean /
old_mean - 1.0); use it to filter out stupidly small changes like < 0.01
--window WINDOW the number of data points analyzed at once; the window size affects the
Expand All @@ -441,6 +444,8 @@ def test_otava_regressions_help_output():
--orig-edivisive ORIG_EDIVISIVE
use the original edivisive algorithm with no windowing and weak change
points analysis improvements
--output {log,json,regressions_only}
Output format for the generated report.

Graphite Options:
Options for Graphite configuration
Expand Down
Loading