-
Notifications
You must be signed in to change notification settings - Fork 4.1k
issues: add new Diagnostics parameter in issues.Post #157021
Description
Follow Up to observations noted here: #153962 (comment)
Roachtest uses pkg/cmd/bazci/githubpost/issues to handle formatting and posting new issues and commetns to github via issues.Post in pkg/cmd/bazci/githubpost/issues/issues.go.
// In Roachtest
github := &githubIssues{
disable: runner.config.disableIssue,
dryRun: runner.config.dryRunIssuePosting,
issuePoster: issues.Post,
teamLoader: team.DefaultLoadTeams,
}
Roachtest creates an issues.PostRequest with Message and ExtraParams which gets passed in issues.Post.
Message string
// ExtraParams contains the parameters to be included in a failure
// report, other than the defaults (git branch, test flags).
ExtraParams map[string]string
// A path to the test artifacts relative to the artifacts root. If nonempty,
// allows the poster formatter to construct a direct URL to this directory.
So previously, for roachtest this message was just a string representation of an error. i.e.
TODO insert example.
The following enhancements were made to improve the triaging experience by adding more information to the created github issue in the form of additional logs and or cluster node ip information.
#147360
#151850
#138356
#153962
To achieve this, the message string was simply appended to with this information in roachtest in test_runner.go
// getGithubMessage abstracts details on how the Github message is constructed
// TODO(wchoe): Currently, all information we want to include in the Github
// message must be passed in as a single string. This is not ideal, especially
// as we add more information or want to format based on the failure or what
// information is available. #157021 aims to address this.
func (t *testImpl) getGithubMessage(failureMsg string) string {
githubMsg := failureMsg
if githubFatalLogs := t.getGithubFatalLogs(); githubFatalLogs != "" {
githubMsg = fmt.Sprintf("%s\n%s", githubMsg, githubFatalLogs)
}
if githubIpToNodeMapping := t.getGithubIpToNodeMapping(); githubIpToNodeMapping != "" {
githubMsg = fmt.Sprintf("%s\n%s", githubMsg, githubIpToNodeMapping)
}
t.L().Printf("github message: %s", githubMsg)
return githubMsg
}
This is brittle / not the best for a few reasons outlined here: #153962 (comment)
- For each additional piece of information we want to pass along, we are creating a complicated "demultiplexing" scenario in when we are formatting the github issue body i.e. each case of information being included / not included needs to be accounted for
- order matters
- complicated regex strings
- complicated control logic in the
issuesrenderer
A better solution would be to pass along a map i.e. Diagnostics map[string]string that would have fatal node logs and ip node info as keys. Then in issues, each kv pair could be handled modularly instead of in a single long if else control statement.
Also calling out ExtraParams because that seems to be similar to what we want. I think a separate field should be added though because ExtraParams and the "diagnostic information" serve different purposes.
If we rely on key names, we could eliminate the complex regex dependency (especially for the ip node table) or move that check to somewhere in roachtest.
More information on the path of message in issues.
The journey of message in issues.
issues.PostRequest.Message and issues.PostRequest.ExtraParams gets processed and added to fields in issues.TemplateData struct i.e. issues.TemplateData.CondensedMessage and issues.TemplateData.Parameters
// TemplateData is the input on which an IssueFormatter operates. It has
// everything known about the test failure in a predigested form.
type TemplateData struct {
// Parameters includes relevant test or build parameters, such as
// build tags or cluster configuration
Parameters map[string]string
// The message, garnished with helpers that allow extracting the useful
// bots.
CondensedMessage CondensedMessage
}
Then data, which is an instance of TemplateData, get's passed into formatter.Body. formatter is a passed in implementation of the IssueFormatter interface. The unitTestFormatterTyp interface implementation's IssueFormatter.Body is called and this is what creates the rendered github issue message body.
r := &Renderer{}
if err := formatter.Body(r, data); err != nil {
// Failure is not an option.
_ = err
fmt.Fprintln(&r.buf, "\nFailed to render body: "+err.Error())
}
Jira issue: CRDB-56351