Skip to content

Commit b253fb1

Browse files
MarceloRobertMarceloRobert
authored
Refactor: right align metrics email (#1943)
* refactor: right align metrics email Readjusts the email formatting so that the larger number sections have a right-aligned text Part of #1938 Signed-off-by: Marcelo Robert Santos <4mrSantos@gmail.com> * feat: use dynamic spacing on metrics email Calculates the space required for the data and labels within jinja to avoid empty spaces or crammed values in the Coverage and Test Labs Activity sections It is possible to do it in Python but then we would need the fmt macro outside as well. A value such as 1000 might count as 4 chars in python but it is displayed as a 5-char 1,000 in jinja. Also, these spaces only make sense for the jinja template anyway, so I think it is correct to keep them within it. Signed-off-by: Marcelo Robert Santos <4mrSantos@gmail.com> * refactor: right align build regressions in metrics email Closes #1938 Signed-off-by: Marcelo Robert Santos <4mrSantos@gmail.com> --------- Signed-off-by: Marcelo Robert Santos <4mrSantos@gmail.com>
1 parent 34a2c77 commit b253fb1

4 files changed

Lines changed: 144 additions & 90 deletions

File tree

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -778,18 +778,18 @@ def generate_hardware_summary_report(
778778

779779

780780
def _fmt_change(cur: int, prev: int, show_percentage: bool = True) -> str:
781-
"""Return a signed change string, e.g. '-246 (-17%)' or '-5'."""
781+
"""Return a signed change string, e.g. '-246 (-17%)' or '-5'."""
782782
diff = cur - prev
783783
if diff == 0:
784-
return "0 (0%)"
784+
return "0"
785785

786786
abs_formatted_diff = f"{abs(diff):,}"
787787
signed_diff = f"+{abs_formatted_diff}" if diff > 0 else f"-{abs_formatted_diff}"
788788

789789
if show_percentage and prev != 0:
790790
percentage = round((diff / prev) * 100)
791791
percentage_sign = "+" if percentage > 0 else ""
792-
return f"{signed_diff} ({percentage_sign}{percentage}%)"
792+
return f"{signed_diff} ({percentage_sign}{percentage}%)"
793793

794794
return signed_diff
795795

@@ -864,18 +864,13 @@ def generate_metrics_report(
864864

865865
deltas = compute_metrics_deltas(data)
866866

867-
# Compute the text spacing for the labs so it isn't too big or too small
868-
lab_spacing = max((len(lab_key) for lab_key in data.lab_maps.keys()), default=0)
869-
lab_spacing += 7 # add spacing for leading tab and possible * mark for new labs
870-
871867
report = {}
872868
template = setup_jinja_template("metrics_report.txt.j2")
873869
report["content"] = template.render(
874870
**data.model_dump(),
875871
start_datetime=start_datetime.strftime("%Y-%m-%d %H:%M %Z"),
876872
end_datetime=end_datetime.strftime("%Y-%m-%d %H:%M %Z"),
877873
deltas=deltas,
878-
lab_spacing=lab_spacing,
879874
)
880875

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

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

Lines changed: 112 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,42 @@ Period: {{ start_datetime }} to {{ end_datetime }}
1010

1111
COVERAGE
1212
--------
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 }}
13+
{%- set static_spacing = 2 -%}
14+
{%- set current_week_space = [n_builds|string|length, n_boots|string|length, n_tests|string|length, "This week"|length] | max + static_spacing %}
15+
{%- set previous_week_space = [prev_n_builds|string|length, prev_n_boots|string|length, prev_n_tests|string|length, "Last week"|length] | max + static_spacing %}
16+
{%- set change_space = [deltas.n_builds|string|length, deltas.n_boots|string|length, deltas.n_tests|string|length, "Change"|length] | max + static_spacing -%}
17+
{%- set label_space = 15 %} {#- length of the longest label -#}
18+
19+
{#- Each group is a row, each line is a column #}
20+
{{ "{:<{width}}".format("", width=label_space) -}}
21+
{{ "{:>{width}}".format("This week", width=current_week_space) -}}
22+
{{ "{:>{width}}".format("Last week", width=previous_week_space) -}}
23+
{{ "{:>{width}}".format("Change", width=change_space) }}
24+
{#-#}
25+
{{ "{:<{width}}".format("", width=label_space) -}}
26+
{{ "{:>{width}}".format("─" * (current_week_space-static_spacing), width=current_week_space) -}}
27+
{{ "{:>{width}}".format("─" * (previous_week_space-static_spacing), width=previous_week_space) -}}
28+
{{ "{:>{width}}".format("─" * (change_space-static_spacing), width=change_space) }}
29+
{#-#}
30+
{{"{:<{width}}".format("Trees monitored", width=label_space) -}}
31+
{{ "{:>{width}}".format(fmt(n_trees), width=current_week_space) -}}
32+
{{ "{:>{width}}".format(fmt(prev_n_trees), width=previous_week_space) -}}
33+
{{ "{:>{width}}".format(deltas.n_trees, width=change_space) }}
34+
{#-#}
35+
{{ "{:<{width}}".format("Checkouts", width=label_space) -}}
36+
{{ "{:>{width}}".format(fmt(n_checkouts), width=current_week_space) -}}
37+
{{ "{:>{width}}".format(fmt(prev_n_checkouts), width=previous_week_space) -}}
38+
{{ "{:>{width}}".format(deltas.n_checkouts, width=change_space) }}
39+
{#-#}
40+
{{ "{:<{width}}".format("Builds", width=label_space) -}}
41+
{{ "{:>{width}}".format(fmt(n_builds), width=current_week_space) -}}
42+
{{ "{:>{width}}".format(fmt(prev_n_builds), width=previous_week_space) -}}
43+
{{ "{:>{width}}".format(deltas.n_builds, width=change_space) }}
44+
{#-#}
45+
{{ "{:<{width}}".format("Tests", width=label_space) -}}
46+
{{ "{:>{width}}".format(fmt(n_tests), width=current_week_space) -}}
47+
{{ "{:>{width}}".format(fmt(prev_n_tests), width=previous_week_space) -}}
48+
{{ "{:>{width}}".format(deltas.n_tests, width=change_space) }}
1949

2050

2151
BUILD REGRESSIONS
@@ -24,30 +54,37 @@ Period: {{ start_datetime }} to {{ end_datetime }}
2454

2555
{% if build_incidents_by_origin -%}
2656

27-
{{ "{:<12}".format(" Origin") -}}
28-
{{ "{:<18}".format("Regressions") -}}
29-
{{ "{:<19}".format("Affected") -}}
30-
Affected builds by top issues
31-
{{ "{:<12}".format("") -}}
32-
{{ "{:<18}".format("(known + new)") -}}
33-
{{ "{:<19}".format("Builds (total)") -}}
34-
{{ "{:<8}".format("#1") -}}{{ "{:<8}".format("#2") -}}#3
35-
─────────────────────────────────────────────────────────────────────────
57+
{% set origin_space = 12 -%}
58+
{% set known_reg_space = 6 -%}
59+
{% set new_reg_space = 6 -%}
60+
{% set total_reg_space = 6 -%}
61+
{% set reg_space = known_reg_space + new_reg_space + total_reg_space -%}
62+
{% set affected_space = 16 -%}
63+
{% set ind_issues_space = 10 -%}
64+
{{ "{:<{width}}".format(" Origin", width=origin_space) -}}
65+
{{ "{:>{width}}".format("Regressions", width=reg_space) -}}
66+
{{ "{:>{width}}".format("Affected", width=affected_space) -}}
67+
{{ "{:>{width}}".format("Affected builds by top issues", width=ind_issues_space*3 + 1) }} {#- +1 to get to a 2-space gap, same for the #1 issue and the space after total incidents #}
68+
{{ "{:<{width}}".format("", width=origin_space) -}}
69+
{{ "{:>{width}}".format("(known + new)", width=reg_space) -}}
70+
{{ "{:>{width}}".format("Builds (total)", width=affected_space) -}}
71+
{{ "{:>{width}}".format("#1", width=ind_issues_space + 1) -}}{{ "{:>{width}}".format("#2", width=ind_issues_space) -}}{{ "{:>{width}}".format("#3", width=ind_issues_space) }}
72+
───────────────────────────────────────────────────────────────────────────
3673
{% for origin, data in build_incidents_by_origin.items() | sort -%}
37-
{{ "{:<12}".format(" " + origin) -}}
38-
{{ "{:<4}".format(fmt(data['n_existing_issues'])) -}} +{{" "}}
39-
{{- "{:<4}".format(fmt(data['n_new_issues'])) -}} ={{" "}}
40-
{{- "{:<6}".format(fmt(data['n_total_issues'])) -}}
41-
{{ "{:<19}".format(fmt(data['total_incidents'])) -}}
74+
{{ "{:<{width}}".format(" " + origin, width=origin_space) -}}
75+
{{ "{:>{width}}".format(fmt(data['n_existing_issues']) + " +", width=known_reg_space) }}
76+
{{- "{:>{width}}".format(fmt(data['n_new_issues']) + " =", width=new_reg_space) }}
77+
{{- "{:>{width}}".format(fmt(data['n_total_issues']), width=total_reg_space) -}}
78+
{{ "{:>{width}}".format(fmt(data['total_incidents']), width=affected_space) -}} {{ " " -}}
4279
{% for issue_key, data in top_issues_by_origin.get(origin, {}).items() -%}
43-
{{ "{:<8}".format(fmt(data['total_incidents'])) -}}
80+
{{ "{:>{width}}".format(fmt(data['total_incidents']), width=ind_issues_space) -}}
4481
{% endfor %}
45-
{% endfor %} ─────────────────────────────────────────────────────────────────────────
46-
{{ "{:<12}".format(" Total") -}}
47-
{{ "{:<4}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_existing_issues') | sum)) -}} +{{" "}}
48-
{{- "{:<4}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_new_issues') | sum)) -}} ={{" "}}
49-
{{- "{:<6}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_total_issues') | sum)) -}}
50-
{{ fmt(build_incidents_by_origin.values() | map(attribute='total_incidents') | sum) }}
82+
{% endfor %} ───────────────────────────────────────────────────────────────────────────
83+
{{ "{:<{width}}".format(" Total", width=origin_space) -}}
84+
{{ "{:>{width}}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_existing_issues') | sum) + " +", width=known_reg_space) }}
85+
{{- "{:>{width}}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_new_issues') | sum) + " =", width=new_reg_space) }}
86+
{{- "{:>{width}}".format(fmt(build_incidents_by_origin.values() | map(attribute='n_total_issues') | sum), width=total_reg_space) -}}
87+
{{ "{:>{width}}".format(fmt(build_incidents_by_origin.values() | map(attribute='total_incidents') | sum), width=affected_space) }}
5188
{%- else %} No build regressions to show in this period. {%- endif %}
5289

5390

@@ -80,34 +117,57 @@ Affected builds by top issues
80117

81118
Labs marked with an asterisk (*) are new.
82119

83-
{% if n_labs -%}
84-
{{ "{:<{width}}".format(" Lab", width=lab_spacing) -}}
85-
{{ "{:<16}".format("Covered builds") -}}
86-
{{ "{:<9}".format("Boots") -}}
87-
{{ "{:<15}".format("Tests") -}}
88-
Change (tests)
89-
────────────────────────────────────────────────────────────────────────────────
120+
{% if n_labs and lab_maps -%}
121+
{#- Compute the text spacing in jinja instead of python to consider the `fmt` macro -#}
122+
{%- set static_spacing = 3 -%}
123+
{%- set lab_names_len = lab_maps.keys() | map('length') | max -%}
124+
{%- set lab_names_space = [lab_names_len, "Lab"|length ] | max + static_spacing + 2 -%} {#- +2 for the possible ` *` -#}
125+
126+
{#- a namespace is required to store the values outside of the loop -#}
127+
{#- initialize with the label length so we consider them as well -#}
128+
{%- set ns = namespace(lab_builds_len="Covered builds"|length, lab_boots_len="Boots"|length, lab_tests_len="Tests"|length) -%}
129+
{%- for v in lab_maps.values() -%}
130+
{%- set ns.lab_builds_len = [ns.lab_builds_len, (fmt(v.builds))|length] | max -%}
131+
{%- set ns.lab_boots_len = [ns.lab_boots_len, (fmt(v.boots))|length] | max -%}
132+
{%- set ns.lab_tests_len = [ns.lab_tests_len, (fmt(v.tests))|length] | max -%}
133+
{%- endfor -%}
134+
{%- set lab_builds_space = ns.lab_builds_len + static_spacing -%}
135+
{%- set lab_boots_space = ns.lab_boots_len + static_spacing -%}
136+
{%- set lab_tests_space = ns.lab_tests_len + static_spacing -%}
137+
138+
{%- set lab_change_len = deltas.labs.values() | map('length') | max -%}
139+
{%- set lab_change_space = [lab_change_len, "Change (tests)"|length] | max + static_spacing -%}
140+
141+
{%- set hrule_length = lab_names_space + lab_builds_space + lab_boots_space + lab_tests_space + lab_change_space - 2 -%}
142+
143+
{{ "{:<{width}}".format(" Lab", width=lab_names_space) -}}
144+
{{ "{:>{width}}".format("Covered builds", width=lab_builds_space) -}}
145+
{{ "{:>{width}}".format("Boots", width=lab_boots_space) -}}
146+
{{ "{:>{width}}".format("Tests", width=lab_tests_space) -}}
147+
{{ "{:>{width}}".format("Change (tests)", width=lab_change_space) }}
148+
{{ "─" * hrule_length}}
90149
{% for lab_key, lab_values in lab_maps.items() | sort -%}
91-
{%- set display_name = lab_key + " *" if lab_key in deltas.new_lab_keys else lab_key -%}
92-
{{ "{:<{width}}".format(" " + display_name, width=lab_spacing) -}}
93-
{{ "{:<16}".format(fmt(lab_values["builds"])) -}}
94-
{{ "{:<9}".format(fmt(lab_values["boots"])) -}}
95-
{{ "{:<15}".format(fmt(lab_values["tests"])) -}}
96-
{{ deltas.labs.get(lab_key, "") }}
150+
{%- set display_name = lab_key + " *" if lab_key in deltas.new_lab_keys else lab_key -%}
151+
{{ "{:<{width}}".format(" " + display_name, width=lab_names_space) -}}
152+
{{ "{:>{width}}".format(fmt(lab_values["builds"]), width=lab_builds_space) -}}
153+
{{ "{:>{width}}".format(fmt(lab_values["boots"]), width=lab_boots_space) -}}
154+
{{ "{:>{width}}".format(fmt(lab_values["tests"]), width=lab_tests_space) -}}
155+
{{ "{:>{width}}".format(deltas.labs.get(lab_key, ""), width=lab_change_space) }}
97156
{% endfor %}
98157
{%- for lab_key in deltas.extinct_lab_keys | sort -%}
99-
{%- set lab_values = prev_lab_maps[lab_key] -%}
100-
{{ "{:<{width}}".format(" " + lab_key, width=lab_spacing) -}}
101-
{{ "{:<16}".format(fmt(0)) -}}
102-
{{ "{:<9}".format(fmt(0)) -}}
103-
{{ "{:<15}".format(fmt(0)) -}}
104-
{{ deltas.labs.get(lab_key, "") }}
105-
{% endfor %} ────────────────────────────────────────────────────────────────────────────────
106-
{{ "{:<{width}}".format(" Total", width=lab_spacing) -}}
107-
{{ "{:<16}".format(fmt(lab_maps.values() | map(attribute='builds') | sum)) -}}
108-
{{ "{:<9}".format(fmt(lab_maps.values() | map(attribute='boots') | sum)) -}}
109-
{{ "{:<15}".format(fmt(lab_maps.values() | map(attribute='tests') | sum)) -}}
110-
{{ deltas.n_total_lab_activity }}
158+
{%- set lab_values = prev_lab_maps[lab_key] -%}
159+
{{ "{:<{width}}".format(" " + lab_key, width=lab_names_space) -}}
160+
{{ "{:>{width}}".format(fmt(0), width=lab_builds_space) -}}
161+
{{ "{:>{width}}".format(fmt(0), width=lab_boots_space) -}}
162+
{{ "{:>{width}}".format(fmt(0), width=lab_tests_space) -}}
163+
{{ "{:>{width}}".format(deltas.labs.get(lab_key, ""), width=lab_change_space) }}
164+
{% endfor %}
165+
{{- " " + "─" * hrule_length}}
166+
{{ "{:<{width}}".format(" Total", width=lab_names_space) -}}
167+
{{ "{:>{width}}".format(fmt(lab_maps.values() | map(attribute='builds') | sum), width=lab_builds_space) -}}
168+
{{ "{:>{width}}".format(fmt(lab_maps.values() | map(attribute='boots') | sum), width=lab_boots_space) -}}
169+
{{ "{:>{width}}".format(fmt(lab_maps.values() | map(attribute='tests') | sum), width=lab_tests_space) -}}
170+
{{ "{:>{width}}".format(deltas.n_total_lab_activity, width=lab_change_space) }}
111171

112172
{%- endif %}
113173

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

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,25 @@ KernelCI Metrics Summary
44

55
COVERAGE
66
--------
7-
This week Last week Change
8-
───────── ───────── ──────
9-
Trees monitored 105 100 +5
10-
Checkouts 1,000 1,000 0 (0%)
11-
Builds 11,000 10,000 +1,000 (+10%)
12-
Tests 1,000,000 1,500,000 -500,000 (-33%)
7+
This week Last week Change
8+
───────── ───────── ───────────────
9+
Trees monitored 105 100 +5
10+
Checkouts 1,000 1,000 0
11+
Builds 11,000 10,000 +1,000 (+10%)
12+
Tests 1,000,000 1,500,000 -500,000 (-33%)
1313

1414

1515
BUILD REGRESSIONS
1616
-----------------
1717
A "regression" is defined as a reported problem affecting 0 or multiple builds.
1818

19-
Origin Regressions Affected Affected builds by top issues
20-
(known + new) Builds (total) #1 #2 #3
21-
─────────────────────────────────────────────────────────────────────────
22-
maestro 1 + 1 = 2 70 50 20
23-
redhat 1 + 0 = 1 5 5
24-
─────────────────────────────────────────────────────────────────────────
25-
Total 2 + 1 = 3 75
19+
Origin Regressions Affected Affected builds by top issues
20+
(known + new) Builds (total) #1 #2 #3
21+
───────────────────────────────────────────────────────────────────────────
22+
maestro 1 + 1 = 2 70 50 20
23+
redhat 1 + 0 = 1 5 5
24+
───────────────────────────────────────────────────────────────────────────
25+
Total 2 + 1 = 3 75
2626

2727

2828
TOP REGRESSIONS PER ORIGIN
@@ -44,12 +44,12 @@ KernelCI Metrics Summary
4444

4545
Labs marked with an asterisk (*) are new.
4646

47-
Lab Covered builds Boots Tests Change (tests)
48-
────────────────────────────────────────────────────────────────────────────────
49-
lava-broonie 0 25,000 475,000 -175,000 (-27%)
50-
lava-collabora 0 50,000 450,000 -250,000 (-36%)
51-
────────────────────────────────────────────────────────────────────────────────
52-
Total 0 75,000 925,000 -425,000 (-31%)
47+
Lab Covered builds Boots Tests Change (tests)
48+
───────────────────────────────────────────────────────────────────────
49+
lava-broonie 0 25,000 475,000 -175,000 (-27%)
50+
lava-collabora 0 50,000 450,000 -250,000 (-36%)
51+
───────────────────────────────────────────────────────────────────────
52+
Total 0 75,000 925,000 -425,000 (-31%)
5353

5454
--
5555
This is an experimental report format. Please send feedback in!

0 commit comments

Comments
 (0)