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.

Sign up for GitHub

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

Jump to bottom

New endpoint to create internal issues before submitting a bug #8443

Closed
wants to merge 7 commits into from
Closed

New endpoint to create internal issues before submitting a bug #8443

wants to merge 7 commits into from

Conversation

vrigal
Copy link
Collaborator

@vrigal vrigal commented Jan 21, 2025

No description provided.

jmaher
jmaher reviewed Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@jmaher jmaher left a comment

Choose a reason for hiding this comment

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

in general I think this needs to:

  1. convert lines to "single tracking bugs" where it can. This would mean finding a test name and ignoring the raw error message that comes after the test name.
  2. keep track of the failures (job ids should be enough), primarily so when we do create the bug it will be linked properly to all the failures.

Archaeopteryx
Archaeopteryx requested changes Jan 22, 2025
View reviewed changes
treeherder/webapp/api/internal_issue.py Outdated Show resolved Hide resolved
treeherder/webapp/api/internal_issue.py Outdated Show resolved Hide resolved
treeherder/webapp/api/internal_issue.py
def create(self, validated_data):
failure_line = validated_data["failure_line"]

# TODO: Fetch all the similar failure lines
Copy link
Collaborator

Choose a reason for hiding this comment

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

For awareness and understanding: In general, the failure lines classified with an internal issue will be similar but they could also be different because they have a common root cause which is not logged in the failure line, e.g. a random focus issue of the browser engine or test framework can cause various form tests to fail. The canonical source for occurrences remain the classifications from Bugscache.

@vrigal
Copy link
Collaborator Author

vrigal commented Jan 24, 2025

@Archaeopteryx from the discussion we had I assumed the UI to look like something like this:
Screenshot 2025-01-24 at 18-10-58 133 autoland

This is the actual battle plan in my head:

  • The + button would open a modal to fill an internal issue with a summary. An entry is also added to the M2M linking the TextLogError with the internal issue.
  • jobs/<job_id>/bug_suggestions/ will have to also return internal issues (id = null) and be expanded with occurrences count (from the M2M).
  • If someone classify a new error with the same internal issue, we keep the actual behavior but also add an entry to the M2M too in order to keep occurences updated.
  • If someone selects this internal issue from suggestions and occurrences >=3, open modal to fill a bug instead.
  • Once a bug is open, we need to find a way to merge both Bugscache entries (internal & bugzilla).

@Archaeopteryx
Copy link
Collaborator

The + button would open a modal to fill an internal issue with a summary.

The + button should not be on a new line. It should be merged with the existing bug button at the left of the failure line (it's okay to keep the bug icon). Vertical screen space is previous if there are multiple failure lines.

Once a bug is open, we need to find a way to merge both Bugscache entries (internal & bugzilla).
The bug gets created by the Treeherder server which uses it afterwards to create the BugJobMap item. This can be replaced to update the Bugscache bugzilla ID for the internal issue.

The code sheriffs have been asked for additional feedback which I will share here if there is any.

Archaeopteryx
Archaeopteryx requested changes Jan 28, 2025
View reviewed changes
...del/migrations/0036_remove_bugscacheoccurrence_unique_failureline_bug_occurrence_and_more.py
),
migrations.AddField(
model_name="bugscacheoccurrence",
name="text_log_error",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Treeherder stores classifications of jobs (tasks) with bugs (internal issues in the future) which get suggested for failure lines. Based on the discussion last week, I think we should keep that concept.
The unique combination should job + internal issue ID.

treeherder/webapp/api/internal_issue.py Outdated Show resolved Hide resolved
@vrigal
Copy link
Collaborator Author

vrigal commented Jan 28, 2025

@Archaeopteryx I started working on the frontend. One thing I noticed is that everything relies on a Bugzilla ID there (for classification an ID may be typed directly, or we would prefer to use dupe_of field to refer another bug sometimes).
I was initally thinking of using the internal ID only for classification but it will not be easy to do. I assume the frontend and API should handle both (internal or Bugzilla ID).

I can update the M2M so it links the Job to the Internal issue. Through in this case it would be better to reuse BugJobMap (Joel mentioned this). I'm unsure of the consequences of using an internal ID instead of the Bugzilla one there.

I can probably make a separate PR to remove the M2M and update BugJobMap to enforce Foreign key to Bugscache internal ID (and patching scripts that manipulates this table).

vrigal added 2 commits January 28, 2025 16:36
@vrigal
[UI] WIP: Support either internal ID or Bugzilla ID on pinned bugs
bd3ad10 
* Update redux store
* Update related components

This will also require to update the API to support both references.
@vrigal
Suggestion
e38442f 
This endpoint will be updated anyway as we now serialize occurences in
/bug_suggestions/
@Archaeopteryx
Copy link
Collaborator

One thing I noticed is that everything relies on a Bugzilla ID there (for classification an ID may be typed directly, or we would prefer to use dupe_of field to refer another bug sometimes). I was initally thinking of using the internal ID only for classification but it will not be easy to do. I assume the frontend and API should handle both (internal or Bugzilla ID).

BugJobMap can be reused. There are two consumers of its data which need to be updated (let me know if I shall take care):

  • intermittents_commenter: Creates daily or weekly report about frequently classified issues. Internal issues without bugs are low frequency and shall not cause bug updates.
  • intermittent failures view (IFV): interactive browsing + charts about classified failures. Internal issues without bug should also be excluded.

About users manually entering the ID to use for classification:
Keep plain integer values as bug numbers, use i123 as format for internal issues in the UI and check for it on the server.
The suggestions should show both:
i123 folder/test.js | single tracking bug (bug 456)
The line should link to the bug if there is one.

@vrigal
Copy link
Collaborator Author

vrigal commented Jan 28, 2025

I'm closing this. Thank you for the many explanations, from my current understanding here are the next steps I can work on (split in 3 different PRs):

  • Extend BugJobMap to count occurrences of internal issues
    • Drop the M2M
    • Move Bugzilla ref to internal FK + created on BugJobMap
    • Update API
    • Fix intermittents_commenter consumer
    • Fix intermittent failures view (IFV) consumer
    • Fix tests
  • Support internal IDs during classification
    • Serialize internal_id + occurrences in /bug_suggestions/ output
    • [UI] Display internal issues with "i123" (when bugzilla ID is missing)
      • Implies changes on the redux store
    • Support classification from either Bugzilla ID or internal ID (/bug_suggestions/ API)
  • Create or update internal bug when occurences < 3
    • Add new endpoint to create "or update" an internal issue (only takes a job_id and summary)
    • [UI] Replace the bug button by a form to fill an internal issue (1 field for the summary)
      • Display errors + feedback about occurrences count (notifications)

@vrigal vrigal closed this Jan 28, 2025
@Archaeopteryx
Copy link
Collaborator 

  * [UI] Display internal issues with "i123" (when bugzilla ID is missing)

Recommendation: Always show internal one, and bugzilla when available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmaher jmaher jmaher left review comments

@Archaeopteryx Archaeopteryx Archaeopteryx requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vrigal @Archaeopteryx @jmaher