Merge pull request #753 from openedx/bmtcril/fail_on_performance_metr…

…ics_error

fix: Fail Tutor command on performance_metrics failure

.github/workflows/integration-test.yml

@@ -139,7 +139,7 @@ jobs:
           make extract_translations
           tutor dev do import-assets
       - name: Performance metrics
-        run: tutor dev do performance-metrics
+        run: tutor dev do performance-metrics --fail_on_error
       - name: Tutor stop
         run: tutor dev stop
 

tutoraspects/commands_v1.py

@@ -137,16 +137,21 @@ def import_assets() -> list[tuple[str, str]]:
 @click.option(
     "--print_sql", is_flag=True, default=False, help="Print the SQL that was run."
 )
-def performance_metrics(course_key, print_sql) -> list[tuple[str, str]]:
+@click.option(
+    "--fail_on_error", is_flag=True, default=False, help="Allow errors to fail the run."
+)
+def performance_metrics(course_key, print_sql, fail_on_error) -> list[tuple[str, str]]:
     """
     Job to measure performance metrics of charts and its queries in Superset and ClickHouse.
     """
     options = f"--course_key {course_key}" if course_key else ""
     options += " --print_sql" if print_sql else ""
+    options += " --fail_on_error" if fail_on_error else ""
 
     return [
         (
             "superset",
+            "set -e && "
             "echo 'Performance...' && "
             f"python /app/pythonpath/performance_metrics.py {options} &&"
             "echo 'Done!';",

tutoraspects/templates/aspects/apps/superset/pythonpath/performance_metrics.py

@@ -46,22 +46,29 @@
 )
 
 @click.command()
-@click.option("--course_key", default="", help="A course_key to apply as a filter.")
+@click.option(
+    "--course_key",
+    default="",
+    help="A course_key to apply as a filter, you must include the 'course-v1:'.")
 @click.option(
     "--print_sql",
     is_flag=True,
     default=False,
     help="Whether to print the SQL run."
 )
-def performance_metrics(course_key, print_sql):
+@click.option(
+    "--fail_on_error", is_flag=True, default=False, help="Allow errors to fail the run."
+)
+def performance_metrics(course_key, print_sql, fail_on_error):
     """
     Measure the performance of the dashboard.
     """
     # Mock the client name to identify the queries in the clickhouse system.query_log
     # table by by the http_user_agent field.
     extra_filters = []
     if course_key:
-        extra_filters+=[{"col":"course_key","op":"==","val":course_key}]
+        extra_filters += [{"col": "course_key", "op": "==", "val": course_key}]
+
     with patch("clickhouse_connect.common.build_client_name") as mock_build_client_name:
         mock_build_client_name.return_value = RUN_ID
         embedable_dashboards = {{SUPERSET_EMBEDDABLE_DASHBOARDS}}
@@ -75,8 +82,12 @@ def performance_metrics(course_key, print_sql):
         for dashboard in dashboards:
             logger.info(f"Dashboard: {dashboard.slug}")
             for slice in dashboard.slices:
-                query_context = get_slice_query_context(slice, query_contexts)
-                result = measure_chart(slice, query_context)
+                query_context = get_slice_query_context(
+                    slice,
+                    query_contexts,
+                    extra_filters
+                )
+                result = measure_chart(slice, query_context, fail_on_error)
                 if not result:
                     continue
                 for query in result["queries"]:
@@ -87,7 +98,7 @@ def performance_metrics(course_key, print_sql):
 
         logger.info("Waiting for clickhouse log...")
         time.sleep(20)
-        get_query_log_from_clickhouse(report, query_contexts, print_sql)
+        get_query_log_from_clickhouse(report, query_contexts, print_sql, fail_on_error)
         return report
 
 
@@ -108,7 +119,11 @@ def get_query_contexts_from_assets():
     logger.info(f"Found {len(query_contexts)} query contexts")
     return query_contexts
 
-def get_slice_query_context(slice, query_contexts, extra_filters=[]):
+
+def get_slice_query_context(slice, query_contexts, extra_filters=None):
+    if not extra_filters:
+        extra_filters = []
+
     query_context = query_contexts.get(str(slice.uuid), {})
     if not query_context:
         logger.info(f"SLICE {slice} has no query context! {slice.uuid}")
@@ -128,12 +143,12 @@ def get_slice_query_context(slice, query_contexts, extra_filters=[]):
 
     if extra_filters:
         for query in query_context["queries"]:
-            query["filters"]+=extra_filters
+            query["filters"] += extra_filters
 
     return query_context
 
 
-def measure_chart(slice, query_context):
+def measure_chart(slice, query_context, fail_on_error):
     """
     Measure the performance of a chart and return the results.
     """
@@ -146,17 +161,25 @@ def measure_chart(slice, query_context):
     start_time = datetime.now()
     try:
         result = command.run()
+
+        for query in result["queries"]:
+            if "error" in query and query["error"]:
+                raise query["error"]
     except Exception as e:
         logger.error(f"Error fetching slice data: {slice}. Error: {e}")
+        if fail_on_error:
+            raise e
         return
+
     end_time = datetime.now()
 
     result["time_elapsed"] = (end_time - start_time).total_seconds()
     result["slice"] = slice
+
     return result
 
 
-def get_query_log_from_clickhouse(report, query_contexts, print_sql):
+def get_query_log_from_clickhouse(report, query_contexts, print_sql, fail_on_error):
     """
     Get the query log from clickhouse and print the results.
     """
@@ -170,7 +193,7 @@ def get_query_log_from_clickhouse(report, query_contexts, print_sql):
         {"col": "http_user_agent", "op": "==", "val": RUN_ID}
     )
 
-    ch_chart_result = measure_chart(slice, query_context)
+    ch_chart_result = measure_chart(slice, query_context, fail_on_error)
 
     clickhouse_queries = {}
     for query in ch_chart_result["queries"]:

tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/charts/Distribution_of_Current_Course_Grade.yaml

@@ -6,24 +6,62 @@ dataset_uuid: 633a1d4e-cd40-482f-a5dc-d5901c2181c2
 description: null
 params:
   adhoc_filters: []
-  all_columns_x:
-  - Course Grade
   annotation_layers: []
   color_scheme: supersetColors
-  cumulative: false
+  comparison_type: values
   extra_form_data: {}
+  forecastInterval: 0.8
+  forecastPeriods: 10
   groupby: []
-  link_length: '10'
-  normalized: true
+  legendOrientation: top
+  legendType: scroll
+  metrics:
+  - aggregate: null
+    column: null
+    datasourceWarning: false
+    expressionType: SQL
+    hasCustomLabel: true
+    label: Number of Learners
+    optionName: metric_kwmmqye349q_e16flj0itp
+    sqlExpression: count()
+  only_total: true
+  order_desc: true
+  orientation: vertical
+  rich_tooltip: true
   row_limit: 10000
+  show_empty_columns: true
   show_legend: true
-  viz_type: histogram
-  x_axis_label: Grade
-  y_axis_label: Number of Learners
-query_context: '{"datasource":{"id":70,"type":"table"},"force":false,"queries":[{"filters":[],"extras":{"having":"","where":""},"applied_time_extras":{},"columns":[],"metrics":[],"annotation_layers":[],"row_limit":10000,"series_limit":0,"order_desc":true,"url_params":{},"custom_params":{},"custom_form_data":{}}],"form_data":{"datasource":"70__table","viz_type":"histogram","slice_id":197,"all_columns_x":["Course
-  Grade"],"adhoc_filters":[],"row_limit":10000,"groupby":[],"color_scheme":"supersetColors","link_length":"10","x_axis_label":"Grade","y_axis_label":"Number
-  of Learners","show_legend":true,"normalized":true,"cumulative":false,"extra_form_data":{},"dashboards":[16],"force":false,"result_format":"json","result_type":"full"},"result_format":"json","result_type":"full"}'
+  sort_series_type: sum
+  tooltipTimeFormat: smart_date
+  truncateXAxis: true
+  truncate_metric: true
+  viz_type: echarts_timeseries_bar
+  x_axis:
+    expressionType: SQL
+    label: Course Grade Percent
+    sqlExpression: grade_bucket
+  x_axis_sort_asc: true
+  x_axis_sort_series: name
+  x_axis_sort_series_ascending: true
+  x_axis_time_format: smart_date
+  x_axis_title: Course Grade Percent
+  x_axis_title_margin: 30
+  y_axis_bounds:
+  - null
+  - null
+  y_axis_format: SMART_NUMBER
+  y_axis_title: Number of Learners
+  y_axis_title_margin: 50
+  y_axis_title_position: Left
+query_context: '{"datasource":{"id":64,"type":"table"},"force":false,"queries":[{"filters":[],"extras":{"having":"","where":""},"applied_time_extras":{},"columns":[{"columnType":"BASE_AXIS","label":"Course
+  Grade Percent","sqlExpression":"grade_bucket","expressionType":"SQL"}],"metrics":[{"expressionType":"SQL","sqlExpression":"count()","column":null,"aggregate":null,"datasourceWarning":false,"hasCustomLabel":true,"label":"Number
+  of Learners","optionName":"metric_kwmmqye349q_e16flj0itp"}],"orderby":[[{"expressionType":"SQL","sqlExpression":"count()","column":null,"aggregate":null,"datasourceWarning":false,"hasCustomLabel":true,"label":"Number
+  of Learners","optionName":"metric_kwmmqye349q_e16flj0itp"},false]],"annotation_layers":[],"row_limit":10000,"series_columns":[],"series_limit":0,"order_desc":true,"url_params":{},"custom_params":{},"custom_form_data":{},"time_offsets":[],"post_processing":[{"operation":"pivot","options":{"index":["Course
+  Grade Percent"],"columns":[],"aggregates":{"Number of Learners":{"operator":"mean"}},"drop_missing_columns":false}},{"operation":"flatten"}]}],"form_data":{"datasource":"64__table","viz_type":"echarts_timeseries_bar","slice_id":2221,"x_axis":{"label":"Course
+  Grade Percent","sqlExpression":"grade_bucket","expressionType":"SQL"},"x_axis_sort_asc":true,"x_axis_sort_series":"name","x_axis_sort_series_ascending":true,"metrics":[{"expressionType":"SQL","sqlExpression":"count()","column":null,"aggregate":null,"datasourceWarning":false,"hasCustomLabel":true,"label":"Number
+  of Learners","optionName":"metric_kwmmqye349q_e16flj0itp"}],"groupby":[],"adhoc_filters":[],"order_desc":true,"row_limit":10000,"truncate_metric":true,"show_empty_columns":true,"comparison_type":"values","annotation_layers":[],"forecastPeriods":10,"forecastInterval":0.8,"orientation":"vertical","x_axis_title":"Course
+  Grade Percent","x_axis_title_margin":30,"y_axis_title":"Number of Learners","y_axis_title_margin":50,"y_axis_title_position":"Left","sort_series_type":"sum","color_scheme":"supersetColors","only_total":true,"show_legend":true,"legendType":"scroll","legendOrientation":"top","x_axis_time_format":"smart_date","y_axis_format":"SMART_NUMBER","truncateXAxis":true,"y_axis_bounds":[null,null],"rich_tooltip":true,"tooltipTimeFormat":"smart_date","extra_form_data":{},"dashboards":[2522],"force":false,"result_format":"json","result_type":"full"},"result_format":"json","result_type":"full"}'
 slice_name: Distribution of Current Course Grade
 uuid: ea565658-6796-40e8-9d1e-01ffd0778bc9
 version: 1.0.0
-viz_type: histogram
+viz_type: echarts_timeseries_bar

tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/charts/Learners_with_Passing_Grade.yaml

@@ -5,8 +5,18 @@ certified_by: null
 dataset_uuid: 633a1d4e-cd40-482f-a5dc-d5901c2181c2
 description: null
 params:
-  adhoc_filters: []
-  annotation_layers: []
+  adhoc_filters:
+  - clause: WHERE
+    comparator: passed
+    datasourceWarning: false
+    expressionType: SIMPLE
+    filterOptionName: filter_uld7zeqwh9a_qn2vi29nkud
+    isExtra: false
+    isNew: false
+    operator: ==
+    operatorId: EQUALS
+    sqlExpression: null
+    subject: approving_state
   conditional_formatting:
   - colorScheme: '#ACE1C4'
     column: Number of Learners
@@ -43,31 +53,36 @@ params:
   time_format: smart_date
   viz_type: big_number_total
   y_axis_format: SMART_NUMBER
-query_context: "{\"datasource\":{\"id\":70,\"type\":\"table\"},\"force\":false,\"\
-  queries\":[{\"filters\":[],\"extras\":{\"having\":\"\",\"where\":\"\"},\"applied_time_extras\"\
-  :{},\"columns\":[],\"metrics\":[{\"aggregate\":\"COUNT_DISTINCT\",\"column\":{\"\
-  advanced_data_type\":null,\"certification_details\":null,\"certified_by\":null,\"\
-  column_name\":\"actor_id\",\"description\":null,\"expression\":null,\"filterable\"\
-  :true,\"groupby\":true,\"id\":457,\"is_certified\":false,\"is_dttm\":false,\"python_date_format\"\
-  :null,\"type\":\"STRING\",\"type_generic\":1,\"verbose_name\":null,\"warning_markdown\"\
+query_context: "{\"datasource\":{\"id\":64,\"type\":\"table\"},\"force\":false,\"\
+  queries\":[{\"filters\":[{\"col\":\"approving_state\",\"op\":\"==\",\"val\":\"passed\"\
+  }],\"extras\":{\"having\":\"\",\"where\":\"\"},\"applied_time_extras\":{},\"columns\"\
+  :[],\"metrics\":[{\"aggregate\":\"COUNT_DISTINCT\",\"column\":{\"advanced_data_type\"\
+  :null,\"certification_details\":null,\"certified_by\":null,\"column_name\":\"actor_id\"\
+  ,\"description\":null,\"expression\":null,\"filterable\":true,\"groupby\":true,\"\
+  id\":457,\"is_certified\":false,\"is_dttm\":false,\"python_date_format\":null,\"\
+  type\":\"STRING\",\"type_generic\":1,\"verbose_name\":null,\"warning_markdown\"\
   :null},\"datasourceWarning\":false,\"expressionType\":\"SIMPLE\",\"hasCustomLabel\"\
   :true,\"label\":\"Number of Learners\",\"optionName\":\"metric_edlf1m0plr_j1zup9r1ixl\"\
   ,\"sqlExpression\":null}],\"annotation_layers\":[],\"series_limit\":0,\"order_desc\"\
   :true,\"url_params\":{},\"custom_params\":{},\"custom_form_data\":{}}],\"form_data\"\
-  :{\"datasource\":\"70__table\",\"viz_type\":\"big_number_total\",\"slice_id\":196,\"\
+  :{\"datasource\":\"64__table\",\"viz_type\":\"big_number_total\",\"slice_id\":2581,\"\
   metric\":{\"aggregate\":\"COUNT_DISTINCT\",\"column\":{\"advanced_data_type\":null,\"\
   certification_details\":null,\"certified_by\":null,\"column_name\":\"actor_id\"\
   ,\"description\":null,\"expression\":null,\"filterable\":true,\"groupby\":true,\"\
   id\":457,\"is_certified\":false,\"is_dttm\":false,\"python_date_format\":null,\"\
   type\":\"STRING\",\"type_generic\":1,\"verbose_name\":null,\"warning_markdown\"\
   :null},\"datasourceWarning\":false,\"expressionType\":\"SIMPLE\",\"hasCustomLabel\"\
   :true,\"label\":\"Number of Learners\",\"optionName\":\"metric_edlf1m0plr_j1zup9r1ixl\"\
-  ,\"sqlExpression\":null},\"adhoc_filters\":[],\"header_font_size\":0.6,\"subheader_font_size\"\
-  :0.4,\"y_axis_format\":\"SMART_NUMBER\",\"time_format\":\"smart_date\",\"conditional_formatting\"\
-  :[{\"colorScheme\":\"#ACE1C4\",\"column\":\"Number of Learners\",\"operator\":\"\
-  \u2265\",\"targetValue\":0}],\"extra_form_data\":{},\"dashboards\":[16],\"force\"\
-  :false,\"result_format\":\"json\",\"result_type\":\"full\"},\"result_format\":\"\
-  json\",\"result_type\":\"full\"}"
+  ,\"sqlExpression\":null},\"adhoc_filters\":[{\"expressionType\":\"SIMPLE\",\"subject\"\
+  :\"approving_state\",\"operator\":\"==\",\"operatorId\":\"EQUALS\",\"comparator\"\
+  :\"passed\",\"clause\":\"WHERE\",\"sqlExpression\":null,\"isExtra\":false,\"isNew\"\
+  :false,\"datasourceWarning\":false,\"filterOptionName\":\"filter_uld7zeqwh9a_qn2vi29nkud\"\
+  }],\"header_font_size\":0.6,\"subheader_font_size\":0.4,\"y_axis_format\":\"SMART_NUMBER\"\
+  ,\"time_format\":\"smart_date\",\"conditional_formatting\":[{\"colorScheme\":\"\
+  #ACE1C4\",\"column\":\"Number of Learners\",\"operator\":\"\u2265\",\"targetValue\"\
+  :0}],\"extra_form_data\":{},\"dashboards\":[2522],\"force\":false,\"result_format\"\
+  :\"json\",\"result_type\":\"full\"},\"result_format\":\"json\",\"result_type\":\"\
+  full\"}"
 slice_name: Learners with Passing Grade
 uuid: b40cdabc-b265-48d2-913d-a9dfee0b6ab1
 version: 1.0.0