Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 19 additions & 6 deletions bugbot/rules/perfalert_inactive_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import collections

import numpy
from libmozdata import utils as lmdutils
from libmozdata.bugzilla import Bugzilla

from bugbot import logger, utils
Expand All @@ -12,21 +14,31 @@


class PerfAlertInactiveRegression(BzCleaner):
def __init__(self, nweeks=1):
def __init__(self, ndays=3):
super().__init__()
self.nweeks = nweeks
self.extra_ni = {"nweeks": self.nweeks}
self.ndays = ndays
self.extra_ni = {"ndays": self.ndays}
self.private_regressor_ids: set[str] = set()

def description(self):
return f"PerfAlert regressions with {self.nweeks} week(s) of inactivity"
return f"PerfAlert regressions with {self.ndays} day(s) of inactivity"

def handle_bug(self, bug, data):
if len(bug["regressed_by"]) != 1:
# either we don't have access to the regressor,
# or there's more than one, either way leave things alone
return

# Skip bugs that haven't been inactive for enough business days
if (
numpy.busday_count(
lmdutils.get_date_ymd(bug["last_change_time"]).date(),
lmdutils.get_date("today"),
)
<= self.ndays
):
return

data[str(bug["id"])] = {
"regressor_id": bug["regressed_by"][0],
}
Expand All @@ -39,6 +51,7 @@ def get_bz_params(self, date):
fields = [
"id",
"regressed_by",
"last_change_time",
]

# Find all bugs that have perf-alert, and regression in their keywords. Only
Expand All @@ -58,8 +71,8 @@ def get_bz_params(self, date):
"o4": "nowords",
"v4": "backlog-deferred",
"f5": "days_elapsed",
"o5": "greaterthan",
"v5": self.nweeks * 7,
"o5": "greaterthaneq",
"v5": self.ndays,
"status": ["UNCONFIRMED", "NEW", "REOPENED"],
"resolution": ["---"],
}
Expand Down
27 changes: 20 additions & 7 deletions bugbot/rules/telemetryalert_inactive_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@
# You can obtain one at http://mozilla.org/MPL/2.0/.


import numpy
from libmozdata import utils as lmdutils
from libmozdata.bugzilla import BugzillaUser

from bugbot.bzcleaner import BzCleaner
from bugbot.user_activity import UserActivity, UserStatus


class TelemetryAlertInactiveRegression(BzCleaner):
def __init__(self, nweeks=1):
def __init__(self, ndays=3):
super().__init__()
self.nweeks = nweeks
self.extra_ni = {"nweeks": self.nweeks}
self.ndays = ndays
self.extra_ni = {"ndays": self.ndays}

def description(self):
return f"Telemetry alerts with {self.nweeks} week(s) of inactivity"
return f"Telemetry alerts with {self.ndays} day(s) of inactivity"

def get_extra_for_needinfo_template(self):
return self.extra_ni
Expand All @@ -30,10 +32,11 @@ def get_bz_params(self, date):
fields = [
"id",
"history",
"last_change_time",
]

# Find all bugs that have a telemetry-alert keyword, have not changed in the
# last week, and do not have the backlog-deferred keyword set
# last few days, and do not have the backlog-deferred keyword set
params = {
"include_fields": fields,
"f3": "keywords",
Expand All @@ -43,8 +46,8 @@ def get_bz_params(self, date):
"o4": "nowords",
"v4": "backlog-deferred",
"f5": "days_elapsed",
"o5": "greaterthan",
"v5": self.nweeks * 7,
"o5": "greaterthaneq",
"v5": self.ndays,
"status": ["UNCONFIRMED", "NEW", "REOPENED"],
"resolution": ["---"],
}
Expand Down Expand Up @@ -78,6 +81,16 @@ def get_probe_owner(self, bug_history):
return probe_owner

def handle_bug(self, bug, data):
# Skip bugs that haven't been inactive for enough business days
if (
numpy.busday_count(
lmdutils.get_date_ymd(bug["last_change_time"]).date(),
lmdutils.get_date("today"),
)
<= self.ndays
):
return

probe_owner = self.get_probe_owner(bug["history"])
if not probe_owner:
# Could not find a probe owner for some reason
Expand Down
6 changes: 0 additions & 6 deletions scripts/cron_run_daily.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,9 @@ python -m bugbot.rules.accessibilitybug --production
# Try to detect potential performance-related bugs using bugbug
python -m bugbot.rules.performancebug --production

# Try to detect potential performance alerts that have been inactive for too long
python -m bugbot.rules.perfalert_inactive_regression --production

# Send an email about all performance alerts who were recently resolved
python -m bugbot.rules.perfalert_resolved_regression --production

# Try to detect potential telemetry alerts that have been inactive for too long
python -m bugbot.rules.telemetryalert_inactive_regression --production

# Update the webcompat score fields
python -m bugbot.rules.webcompat_score --production

Expand Down
6 changes: 6 additions & 0 deletions scripts/cron_run_weekdays.sh
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,10 @@ python -m bugbot.rules.severity_high_performance_impact --production
# Request potential missing info when a bug is moved to Core::Performance
python -m bugbot.rules.moved_to_performance --production

# Try to detect potential performance alerts that have been inactive for too long
python -m bugbot.rules.perfalert_inactive_regression --production

# Try to detect potential telemetry alerts that have been inactive for too long
python -m bugbot.rules.telemetryalert_inactive_regression --production

source ./scripts/cron_common_end.sh
2 changes: 1 addition & 1 deletion templates/perfalert_inactive_regression.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<p>
The following {{ plural('bug is a performance alert which has had', data, pword='bugs are performance alerts which have had') }} no activity in the last {{ extra['nweeks'] * 7 }} days:
The following {{ plural('bug is a performance alert which has had', data, pword='bugs are performance alerts which have had') }} no activity in the last {{ extra['ndays'] }} days:
</p>
<table {{ table_attrs }}>
<thead>
Expand Down
4 changes: 3 additions & 1 deletion templates/perfalert_inactive_regression_needinfo.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
It has been over {{ extra["nweeks"] * 7 }} days with no activity on this performance regression.
It has been over {{ extra["ndays"] }} days with no activity on this performance regression.

:{{ nickname }}, since you are the author of the regressor, bug {{ extra[bugid]["regressor_id"] }}, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. [See this documentation for more information on how to handle regressions](https://firefox-source-docs.mozilla.org/testing/perfdocs/perftest-in-a-nutshell.html#help-i-have-a-regression-what-do-i-do).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrealted: the docs here are not accessible, they contain screenshots without descriptions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on this comment? I can get a bug filed to fix this on the mozilla-central side.


If it’s unclear when the alert may be resolved, then it’s recommended to file a follow-up bug, and close the alert as INCOMPLETE.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only mention the INCOMPLETE case and not the other 4?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is a bit different than the actual definition of INCOMPLETE and how people are using INCOMPLETE in general.
INCOMPLETE usually means there is not enough information to proceed.
Why not let people keep the alert bug open instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need engineers to come to a resolution for the performance alerts within a reasonable time and not leave them open for an unknown amount of time. Often times, bugs in this situation end up becoming stale and I end up closing them. Having this option lets them close the bug to stop being nagged by the bot, and it gives us more up-to-date information about what performance alerts still may need to be resolved. If developers absolutely want to keep a bug open, they have the option of adding a keyword but we don't want to share that as broadly since it could be misused (they can reach out to performance sheriffs about this if need be).

Regarding the INCOMPLETE status, I think that fits the definition you stated since the developers likely haven't gathered enough information about why the regression happened, about how the regression should be resolved, or the impact of a proposed fix so they can't resolve it at this time. Main reason we're adding it here is because we've had questions about this case so we thought it would be useful to surface the information better in the comment here rather than have devs read the docs. I can include the others, but seems like that'll make the comment a bit bloated.


For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in [#perftest](https://matrix.to/#/#perftest:mozilla.org), or [#perfsheriffs](https://matrix.to/#/#perfsheriffs:mozilla.org) on Element.

{{ documentation }}
2 changes: 1 addition & 1 deletion templates/telemetryalert_inactive_regression.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<p>
The following {{ plural('bug is a telemetry alert which has had', data, pword='bugs are telemetry alerts which have had') }} no activity in the last {{ extra['nweeks'] * 7 }} days:
The following {{ plural('bug is a telemetry alert which has had', data, pword='bugs are telemetry alerts which have had') }} no activity in the last {{ extra['ndays'] }} days:
</p>
<table {{ table_attrs }}>
<thead>
Expand Down
4 changes: 3 additions & 1 deletion templates/telemetryalert_inactive_regression_needinfo.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
It has been over {{ extra["nweeks"] * 7 }} days with no activity on this telemetry alert.
It has been over {{ extra["ndays"] }} days with no activity on this telemetry alert.

:{{ nickname }}, since you are the owner of the probe that alerted, could you please provide a progress update?

If there are any questions about how to proceed for finding a culprit please check comment #0 for information on how to find help.

If it’s unclear when the alert may be resolved, then it’s recommended to file a follow-up bug, and close the alert as INCOMPLETE.

{{ documentation }}