Change Point Analysis

sdks/python/apache_beam/testing/analyzers/github_issues_utils.py

@@ -48,14 +48,12 @@
   Performance Regression or Improvement: {}:{}
 """
 
-# TODO: Provide a better debugging tool for the user to visualize metrics.
-# For example, a link to dashboards or query to get recent data to analyze,etc.
 _ISSUE_DESCRIPTION_TEMPLATE = """
   Performance change found in the
   test: `{}` for the metric: `{}`.
 
   For more information on how to triage the alerts, please look at
-   `Triage performance alert issues` section of the README.
+  `Triage performance alert issues` section of the [README](https://github.com/apache/beam/blob/3a453cd9fe2883eebed4b326d800b4cf8b2069e4/sdks/python/apache_beam/testing/analyzers/README.md).
 """
 _METRIC_INFO_TEMPLATE = "timestamp: {}, metric_value: `{}`"
 _AWAITING_TRIAGE_LABEL = 'awaiting triage'
@@ -170,7 +168,7 @@ def get_issue_description(
 
   runs_to_display = [
       _METRIC_INFO_TEMPLATE.format(timestamps[i].ctime(), metric_values[i])
-      for i in range(max_timestamp_index, min_timestamp_index - 1, -1)
+      for i in reversed(range(min_timestamp_index, max_timestamp_index + 1))
   ]
 
   runs_to_display[change_point_index - min_timestamp_index] += " <---- Anomaly"

sdks/python/apache_beam/testing/analyzers/perf_analysis.py

@@ -74,17 +74,20 @@ def run_change_point_analysis(params, test_id, big_query_metrics_fetcher):
       metric_values=metric_values)
   if not change_point_index:
     return
-
-  if not is_change_point_in_valid_window(
-      num_runs_in_change_point_window,
-      len(timestamps) - 1 - change_point_index):
+  # since timestamps are ordered in ascending order and
+  # num_runs_in_change_point_window refers to the latest runs,
+  # latest_change_point_run can help determine if the change point
+  # index is recent wrt num_runs_in_change_point_window
+  latest_change_point_run = len(timestamps) - 1 - change_point_index
+  if not is_change_point_in_valid_window(num_runs_in_change_point_window,
+                                         latest_change_point_run):
     logging.info(
         'Performance regression/improvement found for the test: %s. '
         'Since the change point run %s '
         'lies outside the num_runs_in_change_point_window distance: %s, '
         'alert is not raised.' % (
             params['test_name'],
-            len(timestamps) - 1 - change_point_index,
+            latest_change_point_run,
             num_runs_in_change_point_window))
     return
 
@@ -105,8 +108,8 @@ def run_change_point_analysis(params, test_id, big_query_metrics_fetcher):
         timestamps=timestamps,
         min_runs_between_change_points=min_runs_between_change_points)
 
-  # TODO: remove before merging.
-  logging.info("Performance alert is %s for test %s" % (is_alert, test_name))
+  logging.debug(
+      "Performance alert is %s for test %s" % (is_alert, params['test_name']))
   if is_alert:
     issue_number, issue_url = create_performance_alert(
     metric_name, params['test_name'], timestamps,

sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py

@@ -21,6 +21,7 @@
 import time
 import unittest
 
+# pylint: disable=ungrouped-imports
 try:
   import apache_beam.testing.analyzers.perf_analysis as analysis
   from apache_beam.testing.analyzers import constants

sdks/python/apache_beam/testing/analyzers/perf_analysis_utils.py

@@ -54,8 +54,8 @@ class GitHubIssueMetaData:
 
 
 def is_change_point_in_valid_window(
-    num_runs_in_change_point_window: int, change_point_index: int) -> bool:
-  return num_runs_in_change_point_window >= change_point_index
+    num_runs_in_change_point_window: int, latest_change_point_run: int) -> bool:
+  return num_runs_in_change_point_window > latest_change_point_run
 
 
 def get_existing_issues_data(
@@ -161,8 +161,7 @@ def find_latest_change_point_index(metric_values: List[Union[float, int]]):
     return None
   # Consider the latest change point.
   change_points_idx.sort()
-  change_point_index = change_points_idx[-1]
-  return change_point_index
+  return change_points_idx[-1]
 
 
 def publish_issue_metadata_to_big_query(issue_metadata, test_name):

sdks/python/apache_beam/testing/analyzers/tests_config.yaml

@@ -23,7 +23,7 @@ test_1:
   metric_name: mean_load_model_latency_milli_secs
   labels:
     - run-inference
-  # TODO: Remove before merging.
+  # Optional parameters.
   min_runs_between_change_points: 5
   num_runs_in_change_point_window: 30