Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a tool to request info when a bug is moved to Core::Performance #1916

Merged
merged 11 commits into from
Mar 22, 2023

Conversation

suhaibmujahid
Copy link
Member

@suhaibmujahid suhaibmujahid commented Mar 4, 2023

Resolves #1887

Checklist

  • Type annotations added to new functions
  • Docs added to functions touched in main classes
  • Dry-run produced the expected results
  • The to-be-announced tag added if this is worth announcing

@suhaibmujahid

This comment was marked as outdated.

@marco-c
Copy link
Contributor

marco-c commented Mar 6, 2023

What if the information is already present in the bug?

@suhaibmujahid
Copy link
Member Author

What if the information is already present in the bug?

In that scenario the needinfo will be cleared with/without a confirmation that the information is already in the bug.

We could add a sentence at the end to request a response: "If the requested information is already in the bug, please confirm that."

@julienw wdyt?

suhaibmujahid and others added 2 commits March 6, 2023 10:49
Co-authored-by: Marco Castelluccio <mcastelluccio@mozilla.com>
@julienw
Copy link

julienw commented Mar 8, 2023

Yeah, this sounds good to me. We can adjust later if we see this case happening too much.

@marco-c
Copy link
Contributor

marco-c commented Mar 8, 2023

Could we add some basic checks to prevent some obvious cases? E.g. if an attachment with "memory" in its name is in the bug, we don't ask for a memory report; if there is a "share.firefox.dev" in any of the comments, we don't ask for a performance profile.

@julienw
Copy link

julienw commented Mar 8, 2023

This would work for me too :-)

@suhaibmujahid
Copy link
Member Author

Could we add some basic checks to prevent some obvious cases? E.g. if an attachment with "memory" in its name is in the bug, we don't ask for a memory report; if there is a "share.firefox.dev" in any of the comments, we don't ask for a performance profile.

Nice idea! However, doing simple checks could have high false negatives. I suggest monitoring the cases after deploying this and adjusting later if this introduces much noise.

I filed an issue to follow up on this idea (i.e., #1926).

@marco-c wdyt?

@marco-c
Copy link
Contributor

marco-c commented Mar 9, 2023

I would add some simple checks now, the ones that are very unlikely to have false negatives and are easy to implement.

  • "share.firefox.dev" in the comments is definitely a link to a profile, so it is very unlikely to cause false negatives.
  • if you go to about:memory and click on "Measure and save...", Firefox will save a memory-report.json.gz file on your disk, so "memory-report.json.gz" in the attachments is also very unlikely to cause false negatives.

@suhaibmujahid
Copy link
Member Author

suhaibmujahid commented Mar 11, 2023

Dry-run

2023-03-10 23:31:06,393 - INFO - Run tool moved_to_performance.py
2023-03-10 23:31:07,932 - INFO - Tool moved_to_performance.py has finished.

The following bugs were moved into the Core::Performance component; a comment to request more information was added:

BugSummary
1821555 Both FB Marketplace & Twitter lock up for 30-45 seconds every time I try to scroll.
1821424 Memory leakage 110.0.01 Win11 22H2
1820078 I try to access https://mysafaricost.com on firefox browser and it redirects always on some pages and stops and displays a broken site at the end while it opens on another browser.
1795009 Open multiple tabs becomes slow

Comment example - bug 1795009

This bug was moved into the Performance component.

:zero.arst, could you make sure the following information is on this bug?

  • ✅ For slowness or high CPU usage, capture a profile with http://profiler.firefox.com/, upload it and share the link here.
  • For memory usage issues, capture a memory dump from about:memory and attach it to this bug.
  • Troubleshooting information: Go to about:support, click "Copy raw data to clipboard", paste it into a file, save it, and attach the file here.

If the requested information is already in the bug, please confirm that.

Thank you.

@suhaibmujahid
Copy link
Member Author

@julienw in the context of doing some checks to avoid asking for information that is already present in the bug, do you think we should consider only content that was submitted be the reporter?

@marco-c
Copy link
Contributor

marco-c commented Mar 14, 2023

I never used a checklist on Bugzilla, does it actually work? Can a user mark something in a checklist as done? Do they need specific permissions to do that if the comment is from somebody else (the bot in this case)?

@julienw
Copy link

julienw commented Mar 14, 2023

@julienw in the context of doing some checks to avoid asking for information that is already present in the bug, do you think we should consider only content that was submitted be the reporter?

Yes, this sounds reasonable.
I wonder if we should also have a time limit. For example if the information is older than say 6 months, we could assume this is outdated and still ask about it.

@suhaibmujahid
Copy link
Member Author

I wonder if we should also have a time limit. For example if the information is older than say 6 months, we could assume this is outdated and still ask about it.

Done in 2a7cae1

@suhaibmujahid
Copy link
Member Author

I never used a checklist on Bugzilla, does it actually work? Can a user mark something in a checklist as done? Do they need specific permissions to do that if the comment is from somebody else (the bot in this case)?

Unfonratintly, Bugzilla does not support markdown checklists.

@marco-c
Copy link
Contributor

marco-c commented Mar 21, 2023

Maybe when the data is present but not recent enough, we should adjust the comment to ask to refresh the info (otherwise people will ignore it and say that it is already present)

@suhaibmujahid
Copy link
Member Author

Maybe when the data is present but not recent enough, we should adjust the comment to ask to refresh the info (otherwise people will ignore it and say that it is already present)

Do you think 13820a7 would be enough? If not, I can implement what you suggested.

@suhaibmujahid suhaibmujahid merged commit d87082d into mozilla:master Mar 22, 2023
@suhaibmujahid suhaibmujahid deleted the moved-performance branch March 22, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants