Skip to content

issues: add new Diagnostics parameter in issues.Post #157021

@williamchoe3

Description

@williamchoe3

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 issues renderer

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-testeng-foundationsA-testingTesting tools and infrastructureC-enhancementSolution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)T-testengTestEng Team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions