Skip to content

Commit f56b2e3

Browse files
MarceloRobertMarceloRobert
authored
Refactor: metrics notification structure (#1761)
* refactor: metrics notification structure Changes the email notification to always show a comparison with the last week - Changes query to be performed two times with the different period; - Changes the number formatting to show commas on large numbers; - Changes template to show the new data and to format when there is no data for a section Closes #1715 * feat: add unit tests for metrics notifications Along with the unit tests, an example of the metrics notification email was added for use in the unit tests
1 parent e1be8a0 commit f56b2e3

8 files changed

Lines changed: 600 additions & 41 deletions

File tree

‎backend/kernelCI_app/management/commands/notifications.py‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
kcidb_tests_results,
4040
)
4141
from kernelCI_app.queries.test import get_test_details_data, get_test_status_history
42+
from kernelCI_app.typeModels.metrics_notifications import MetricsReportData
4243
from kernelCI_cache.queries.notifications import (
4344
RESEND_INTERVAL,
4445
check_sent_notifications,
@@ -742,6 +743,63 @@ def generate_hardware_summary_report(
742743
)
743744

744745

746+
def _fmt_change(cur: int, prev: int, show_percentage: bool = True) -> str:
747+
"""Return a signed change string, e.g. '-246 (-17%)' or '-5'."""
748+
diff = cur - prev
749+
if diff == 0:
750+
return "0 (0%)"
751+
752+
abs_formatted_diff = f"{abs(diff):,}"
753+
signed_diff = f"+{abs_formatted_diff}" if diff > 0 else f"-{abs_formatted_diff}"
754+
755+
if show_percentage and prev != 0:
756+
percentage = round((diff / prev) * 100)
757+
percentage_sign = "+" if percentage > 0 else ""
758+
return f"{signed_diff} ({percentage_sign}{percentage}%)"
759+
760+
return signed_diff
761+
762+
763+
def compute_metrics_deltas(data: MetricsReportData) -> dict:
764+
"""Pre-compute all change strings for the metrics report template."""
765+
new_lab_keys: set[str] = set(data.lab_maps.keys()) - set(data.prev_lab_maps.keys())
766+
extinct_lab_keys: set[str] = set(data.prev_lab_maps.keys()) - set(
767+
data.lab_maps.keys()
768+
)
769+
770+
labs = {}
771+
n_total_lab_curr = 0
772+
n_total_lab_prev = 0
773+
for lab_key, lab_values in data.lab_maps.items():
774+
# Gathers change from current labs to their last week,
775+
# as well as already counting part of last week's total
776+
prev_lab_tests = (
777+
data.prev_lab_maps[lab_key].tests if lab_key in data.prev_lab_maps else 0
778+
)
779+
labs[lab_key] = _fmt_change(lab_values.tests, prev_lab_tests)
780+
781+
n_total_lab_curr += lab_values.tests
782+
n_total_lab_prev += prev_lab_tests
783+
784+
for lab_key in extinct_lab_keys:
785+
# Gathers change from extinct labs and finishes counting last week's total
786+
extinct_tests = data.prev_lab_maps[lab_key].tests
787+
labs[lab_key] = _fmt_change(0, extinct_tests)
788+
789+
n_total_lab_prev += extinct_tests
790+
791+
return {
792+
"n_trees": _fmt_change(data.n_trees, data.prev_n_trees, show_percentage=False),
793+
"n_checkouts": _fmt_change(data.n_checkouts, data.prev_n_checkouts),
794+
"n_builds": _fmt_change(data.n_builds, data.prev_n_builds),
795+
"n_tests": _fmt_change(data.n_tests, data.prev_n_tests),
796+
"n_total_lab_activity": _fmt_change(n_total_lab_curr, n_total_lab_prev),
797+
"labs": labs,
798+
"new_lab_keys": new_lab_keys,
799+
"extinct_lab_keys": extinct_lab_keys,
800+
}
801+
802+
745803
def generate_metrics_report(
746804
*,
747805
email_service,
@@ -759,17 +817,20 @@ def generate_metrics_report(
759817
start_datetime = now - timedelta(days=start_days_ago)
760818
end_datetime = now - timedelta(days=end_days_ago)
761819

762-
data = get_metrics_data(
820+
data: MetricsReportData = get_metrics_data(
763821
start_days_ago=start_days_ago,
764822
end_days_ago=end_days_ago,
765823
)
766824

825+
deltas = compute_metrics_deltas(data)
826+
767827
report = {}
768828
template = setup_jinja_template("metrics_report.txt.j2")
769829
report["content"] = template.render(
770830
**data.model_dump(),
771831
start_datetime=start_datetime.strftime("%Y-%m-%d %H:%M %Z"),
772832
end_datetime=end_datetime.strftime("%Y-%m-%d %H:%M %Z"),
833+
deltas=deltas,
773834
)
774835

775836
report["title"] = "KernelCI Metrics Report - %s" % now.strftime("%Y-%m-%d %H:%M %Z")

‎backend/kernelCI_app/management/commands/refresh_metrics_email_example.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from django.core.management.base import BaseCommand
2+
from kernelCI_app.tests.unitTests.commands.fixtures.metrics_notifications_data import (
3+
METRICS_NOTIFICATIONS_EXAMPLE_FILEPATH,
4+
)
5+
from kernelCI_app.tests.unitTests.commands.metrics_notifications_test import (
6+
TestMetricsReportTemplate,
7+
)
8+
9+
10+
class Command(BaseCommand):
11+
help = (
12+
"Refreshes the txt file used as an example and comparison "
13+
"for the metrics notification unit tests. "
14+
"Run this manually when the template changes."
15+
)
16+
17+
def handle(self, *args, **options):
18+
try:
19+
content = TestMetricsReportTemplate.render_report()
20+
with open(METRICS_NOTIFICATIONS_EXAMPLE_FILEPATH, "w") as f:
21+
f.write(content)
22+
self.stdout.write(
23+
self.style.SUCCESS(f"{METRICS_NOTIFICATIONS_EXAMPLE_FILEPATH} updated.")
24+
)
25+
except Exception as e:
26+
self.stdout.write(
27+
self.style.ERROR(f"Failed to update metrics_report_example.txt: {e}")
28+
)

‎backend/kernelCI_app/management/commands/templates/metrics_report.txt.j2‎

Lines changed: 77 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,92 @@
11
{% extends "base.txt" %}
22
{% block header %}{% endblock %}
33
{% block content -%}
4+
{#- Helper macro: comma-format an integer -#}
5+
{%- macro fmt(n) -%}{{ "{:,}".format(n) }}{%- endmacro -%}
46
KernelCI Metrics Summary
57
========================
68
Period: {{ start_datetime }} to {{ end_datetime }}
79

810

9-
ACTIVITY
10-
--------
11-
{{ "{:>6}".format(n_issues) }} issues
12-
{{ "{:>6}".format(n_incidents) }} incidents
13-
{{ "{:>6}".format(n_checkouts) }} checkouts
14-
{{ "{:>6}".format(n_builds) }} builds
15-
{{ "{:>6}".format(n_tests) }} tests
11+
COVERAGE
12+
--------
13+
{{ "{:<22}".format("") -}}{{ "{:<13}".format("This week") -}}{{ "{:<13}".format("Last week") }}Change
14+
{{ "{:<22}".format("") -}}{{ "{:<13}".format("─────────") -}}{{ "{:<13}".format("─────────") }}──────
15+
{{ "{:<22}".format(" Trees monitored") -}}{{ "{:<13}".format(fmt(n_trees)) -}}{{ "{:<13}".format(fmt(prev_n_trees)) }}{{ deltas.n_trees }}
16+
{{ "{:<22}".format(" Checkouts") -}}{{ "{:<13}".format(fmt(n_checkouts)) -}}{{ "{:<13}".format(fmt(prev_n_checkouts)) }}{{ deltas.n_checkouts }}
17+
{{ "{:<22}".format(" Builds") -}}{{ "{:<13}".format(fmt(n_builds)) -}}{{ "{:<13}".format(fmt(prev_n_builds)) }}{{ deltas.n_builds }}
18+
{{ "{:<22}".format(" Tests") -}}{{ "{:<13}".format(fmt(n_tests)) -}}{{ "{:<13}".format(fmt(prev_n_tests)) }}{{ deltas.n_tests }}
1619

1720

18-
BUILD REGRESSIONS
19-
-----------------
20-
Incidents are any occurrences of an issue.
21-
New regressions are the first incident of an issue.
21+
BUILD REGRESSIONS
22+
-----------------
23+
A "regression" is a newly detected failure (first occurrence).
24+
An "occurrence" is each subsequent hit of that same failure.
2225

23-
Origin Total Incidents New Regressions
24-
──────────────────────────────────────────────
26+
{% if build_incidents_by_origin -%}
27+
{{ "{:<14}".format(" Origin") -}}
28+
{{ "{:<15}".format("Occurrences") -}}
29+
New regressions
30+
────────────────────────────────────────────
2531
{% for origin, data in build_incidents_by_origin.items() | sort -%}
26-
{{ "{:<12}".format(origin) -}}
27-
{{ "{:<19}".format(data.total) -}}
28-
{{ data.new_regressions }}
29-
{% endfor %}
32+
{{ "{:<14}".format(" " + origin) -}}
33+
{{ "{:<15}".format(fmt(data.total)) -}}
34+
{{ fmt(data.new_regressions) }}
35+
{% endfor %} ────────────────────────────────────────────
36+
{{ "{:<14}".format(" Total") -}}
37+
{{ "{:<15}".format(fmt(build_incidents_by_origin.values() | map(attribute='total') | sum)) -}}
38+
{{ fmt(build_incidents_by_origin.values() | map(attribute='new_regressions') | sum) }}
39+
{%- else %} No build regressions to show in this period. {%- endif %}
40+
41+
42+
LAB ACTIVITY
43+
------------
44+
{%- set n_labs = lab_maps | length %}
45+
{%- set prev_n_labs = prev_lab_maps | length %}
46+
{%- set lab_diff = n_labs - prev_n_labs %}
47+
{%- if lab_diff == 0 %}
48+
{{ n_labs }} labs reported results this week (unchanged from last week).
49+
{%- elif lab_diff > 0 %}
50+
{{ n_labs }} labs reported results this week ({{ lab_diff }} more than last week).
51+
{%- else %}
52+
{{ n_labs }} labs reported results this week ({{ lab_diff | abs }} fewer than last week).
53+
{%- endif %}
3054

31-
LAB ACTIVITY
32-
------------
33-
There were {{ lab_maps | length }} labs registered. Incidents reported per lab:
55+
Labs marked with an asterisk (*) are new.
56+
Labs that stopped reporting are shown with a -100% change.
3457

35-
Origin Lab Builds Boots Tests
36-
────────────────────────────────────────────────────────────
58+
{% if n_labs -%}
59+
{{ "{:<20}".format(" Origin") -}}
60+
{{ "{:<25}".format("Lab") -}}
61+
{{ "{:<9}".format("Builds") -}}
62+
{{ "{:<9}".format("Boots") -}}
63+
{{ "{:<15}".format("Tests") -}}
64+
Change (tests)
65+
──────────────────────────────────────────────────────────────────────────────────────────
3766
{% for lab_key, lab_values in lab_maps.items() | sort -%}
38-
{{ "{:<12}".format(lab_values["origin"]) -}}
39-
{{ "{:<23}".format(lab_key) -}}
40-
{{ "{:<10}".format(lab_values["builds"]) -}}
41-
{{ "{:<10}".format(lab_values["boots"]) -}}
42-
{{ lab_values["tests"] }}
43-
{% endfor -%}
44-
────────────────────────────────────────────────────────────
45-
{{ "{:<35}".format("Total") -}}
46-
{{ "{:<10}".format(lab_maps.values() | map(attribute='builds') | sum) -}}
47-
{{ "{:<10}".format(lab_maps.values() | map(attribute='boots') | sum) -}}
48-
{{ lab_maps.values() | map(attribute='tests') | sum }}
49-
50-
{% endblock -%}
67+
{%- set display_name = lab_key + " *" if lab_key in deltas.new_lab_keys else lab_key -%}
68+
{{ "{:<20}".format(" " + lab_values["origin"]) -}}
69+
{{ "{:<25}".format(display_name) -}}
70+
{{ "{:<9}".format(fmt(lab_values["builds"])) -}}
71+
{{ "{:<9}".format(fmt(lab_values["boots"])) -}}
72+
{{ "{:<15}".format(fmt(lab_values["tests"])) -}}
73+
{{ deltas.labs.get(lab_key, "") }}
74+
{% endfor %}
75+
{%- for lab_key in deltas.extinct_lab_keys | sort -%}
76+
{%- set lab_values = prev_lab_maps[lab_key] -%}
77+
{{ "{:<20}".format(" " + lab_values["origin"]) -}}
78+
{{ "{:<25}".format(lab_key) -}}
79+
{{ "{:<9}".format(fmt(0)) -}}
80+
{{ "{:<9}".format(fmt(0)) -}}
81+
{{ "{:<15}".format(fmt(0)) -}}
82+
{{ deltas.labs.get(lab_key, "") }}
83+
{% endfor %} ──────────────────────────────────────────────────────────────────────────────────────────
84+
{{ "{:<45}".format(" Total") -}}
85+
{{ "{:<9}".format(fmt(lab_maps.values() | map(attribute='builds') | sum)) -}}
86+
{{ "{:<9}".format(fmt(lab_maps.values() | map(attribute='boots') | sum)) -}}
87+
{{ "{:<15}".format(fmt(lab_maps.values() | map(attribute='tests') | sum)) -}}
88+
{{ deltas.n_total_lab_activity }}
89+
90+
{%- endif %}
91+
92+
{%- endblock -%}

‎backend/kernelCI_app/queries/notifications.py‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -654,13 +654,25 @@ def get_metrics_data(
654654
"Warning: metrics report for more than 30 days may take a long time to generate."
655655
)
656656

657+
period_length = end_days_ago - start_days_ago
658+
prev_start_days_ago = start_days_ago - period_length
659+
prev_end_days_ago = start_days_ago
660+
657661
params = {
658662
"start_days_ago": str(start_days_ago) + " days",
659663
"end_days_ago": str(end_days_ago) + " days",
660664
}
665+
prev_params = {
666+
"start_days_ago": str(prev_start_days_ago) + " days",
667+
"end_days_ago": str(prev_end_days_ago) + " days",
668+
}
661669

662670
total_objects_query = """
663671
SELECT
672+
(SELECT COUNT(DISTINCT tree_name) FROM checkouts WHERE _timestamp BETWEEN
673+
NOW() - INTERVAL %(start_days_ago)s
674+
AND NOW() - INTERVAL %(end_days_ago)s)
675+
AS n_trees,
664676
(SELECT COUNT(*) FROM checkouts WHERE _timestamp BETWEEN
665677
NOW() - INTERVAL %(start_days_ago)s
666678
AND NOW() - INTERVAL %(end_days_ago)s)
@@ -762,25 +774,33 @@ def get_metrics_data(
762774
cursor.execute(total_objects_query, params)
763775
total_objects_result = cursor.fetchone()
764776

777+
cursor.execute(total_objects_query, prev_params)
778+
prev_total_objects_result = cursor.fetchone()
779+
765780
cursor.execute(build_incidents_query, params)
766781
build_incidents_result = cursor.fetchall()
767782

768783
cursor.execute(lab_summary_query, params)
769784
lab_summary_results = cursor.fetchall()
770785

786+
cursor.execute(lab_summary_query, prev_params)
787+
prev_lab_summary_results = cursor.fetchall()
788+
771789
try:
772790
data = MetricsReportData(
773-
n_checkouts=total_objects_result[0],
774-
n_builds=total_objects_result[1],
775-
n_tests=total_objects_result[2],
776-
n_issues=total_objects_result[3],
777-
n_incidents=total_objects_result[4],
791+
n_trees=total_objects_result[0],
792+
n_checkouts=total_objects_result[1],
793+
n_builds=total_objects_result[2],
794+
n_tests=total_objects_result[3],
795+
n_issues=total_objects_result[4],
796+
n_incidents=total_objects_result[5],
778797
build_incidents_by_origin={
779798
row[0]: BuildIncidentsByOrigin(
780799
total=row[1],
781800
new_regressions=row[2],
782801
)
783802
for row in build_incidents_result
803+
if row[1] != 0 or row[2] != 0
784804
},
785805
lab_maps={
786806
row[0]: LabMetricsData(
@@ -791,6 +811,19 @@ def get_metrics_data(
791811
)
792812
for row in lab_summary_results
793813
},
814+
prev_n_trees=prev_total_objects_result[0],
815+
prev_n_checkouts=prev_total_objects_result[1],
816+
prev_n_builds=prev_total_objects_result[2],
817+
prev_n_tests=prev_total_objects_result[3],
818+
prev_lab_maps={
819+
row[0]: LabMetricsData(
820+
origin=row[1],
821+
builds=row[2],
822+
boots=row[3],
823+
tests=row[4],
824+
)
825+
for row in prev_lab_summary_results
826+
},
794827
)
795828
except ValidationError as e:
796829
out(f"Validation error when constructing MetricsReportData: {e}")

‎backend/kernelCI_app/tests/unitTests/commands/fixtures/metrics_notifications_data.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
from kernelCI import settings
2+
3+
4+
METRICS_NOTIFICATIONS_EXAMPLE_FILEPATH = (
5+
settings.BASE_DIR
6+
/ "kernelCI_app"
7+
/ "tests"
8+
/ "unitTests"
9+
/ "commands"
10+
/ "fixtures"
11+
/ "metrics_notifications_example.txt"
12+
)

‎backend/kernelCI_app/tests/unitTests/commands/fixtures/metrics_notifications_example.txt‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
2+
3+
KernelCI Metrics Summary
4+
========================
5+
6+
7+
COVERAGE
8+
--------
9+
This week Last week Change
10+
───────── ───────── ──────
11+
Trees monitored 105 100 +5
12+
Checkouts 1,000 1,000 0 (0%)
13+
Builds 11,000 10,000 +1,000 (+10%)
14+
Tests 1,000,000 1,500,000 -500,000 (-33%)
15+
16+
17+
BUILD REGRESSIONS
18+
-----------------
19+
A "regression" is a newly detected failure (first occurrence).
20+
An "occurrence" is each subsequent hit of that same failure.
21+
22+
Origin Occurrences New regressions
23+
────────────────────────────────────────────
24+
maestro 70 1
25+
redhat 5 0
26+
────────────────────────────────────────────
27+
Total 75 1
28+
29+
30+
LAB ACTIVITY
31+
------------
32+
2 labs reported results this week (unchanged from last week).
33+
34+
Labs marked with an asterisk (*) are new.
35+
Labs that stopped reporting are shown with a -100% change.
36+
37+
Origin Lab Builds Boots Tests Change (tests)
38+
──────────────────────────────────────────────────────────────────────────────────────────
39+
maestro lava-broonie 0 25,000 475,000 -175,000 (-27%)
40+
maestro lava-collabora 0 50,000 450,000 -250,000 (-36%)
41+
──────────────────────────────────────────────────────────────────────────────────────────
42+
Total 0 75,000 925,000 -425,000 (-31%)
43+
44+
--
45+
This is an experimental report format. Please send feedback in!
46+
Talk to us at kernelci@lists.linux.dev
47+
48+
Made with love by the KernelCI team - https://kernelci.org

0 commit comments

Comments
 (0)