diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 45c1ea060d..d9502a4168 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -551,3 +551,36 @@ jobs: go run ./cmd/tools/ci-notify alert \ --run-id "${{ github.run_id }}" \ --slack-webhook "$SLACK_WEBHOOK" + + notify-data-race: + name: Notify Slack on Data Race + # Runs on both passing and failing main runs: the race detector can trip on + # one attempt and pass on a retry, leaving the run green while a data race + # was still recorded in the test-summary artifacts. + if: | + always() && + github.ref == 'refs/heads/main' && + (needs.test-status.result == 'success' || needs.test-status.result == 'failure') + needs: test-status + runs-on: ubuntu-latest + permissions: + contents: read + actions: read + steps: + - name: Checkout code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Set up Go + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 + with: + go-version-file: "go.mod" + cache: true + + - name: Send Slack notification + env: + SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} + GH_TOKEN: ${{ github.token }} + run: | + go run ./cmd/tools/ci-notify data-race \ + --run-id "${{ github.run_id }}" \ + --slack-webhook "$SLACK_WEBHOOK" diff --git a/tools/ci-notify/app.go b/tools/ci-notify/app.go index 905d4a080a..ab61242cde 100644 --- a/tools/ci-notify/app.go +++ b/tools/ci-notify/app.go @@ -51,6 +51,30 @@ func NewCliApp() *cli.App { }, Action: runAlertCommand, }, + { + Name: "data-race", + Usage: "Send a notification when data races are detected in a CI run", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: FlagRunID, + Usage: "GitHub Actions run ID", + Required: true, + EnvVars: []string{"GITHUB_RUN_ID"}, + }, + &cli.StringFlag{ + Name: FlagSlackWebhook, + Usage: "Slack webhook URL", + Required: false, // Not required for dry-run + EnvVars: []string{"SLACK_WEBHOOK"}, + }, + &cli.BoolFlag{ + Name: FlagDryRun, + Usage: "Print message without sending to Slack", + Value: false, + }, + }, + Action: runDataRaceCommand, + }, { Name: "digest", Usage: "Generate digest report for CI runs", @@ -157,6 +181,78 @@ func runAlertCommand(c *cli.Context) error { return nil } +func runDataRaceCommand(c *cli.Context) error { + // Set up logger + logger, err := zap.NewProduction() + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to create logger: %v\n", err) + return nil // Don't fail CI if notification fails + } + defer func() { _ = logger.Sync() }() + + runID := c.String(FlagRunID) + slackWebhook := c.String(FlagSlackWebhook) + dryRun := c.Bool(FlagDryRun) + + logger.Info("Starting `ci-notify data-race`", + zap.String("run_id", runID), + zap.Bool("dry_run", dryRun), + ) + + // Build data race report + report, err := BuildDataRaceReport(runID) + if err != nil { + logger.Error("Failed to build data race report", zap.Error(err)) + // Don't fail CI if notification fails + return nil + } + + logger.Info("Built data race report", + zap.Int("data_races", len(report.DataRaces)), + ) + + // Nothing to report: stay silent so green runs don't spam the channel. + if len(report.DataRaces) == 0 { + logger.Info("No data races detected; skipping notification") + return nil + } + + // Handle dry-run mode + if dryRun { + logger.Info("Dry-run mode: printing message to stdout") + message := BuildDataRaceMessage(report) + fmt.Println(message.RenderMarkdown()) + fmt.Println("\n--- Slack JSON Payload ---") + payload, err := marshalIndent(message) + if err != nil { + logger.Error("Failed to marshal message for display", zap.Error(err)) + return nil + } + fmt.Println(payload) + return nil + } + + // Validate webhook URL + if slackWebhook == "" { + logger.Error("Slack webhook URL is required when not in dry-run mode") + // Don't fail CI if notification fails + return nil + } + + // Build and send Slack message + message := BuildDataRaceMessage(report) + + logger.Info("Sending Slack notification") + if err := message.Send(slackWebhook); err != nil { + logger.Error("Failed to send Slack message", zap.Error(err)) + // Don't fail CI if notification fails + return nil + } + + logger.Info("Slack notification sent successfully") + return nil +} + func runDigestCommand(c *cli.Context) error { // Set up logger logger, err := zap.NewProduction() diff --git a/tools/ci-notify/artifacts.go b/tools/ci-notify/artifacts.go index 26afd98be7..a1f62e7c58 100644 --- a/tools/ci-notify/artifacts.go +++ b/tools/ci-notify/artifacts.go @@ -19,6 +19,7 @@ const ( summaryArtifactPrefix = "test-summary-json--" summaryFileSuffix = "test-summary.json" summaryKindOOM = "OOM" + summaryKindDataRace = "DATA RACE" ) var trailingFailureSuffixRegex = regexp.MustCompile(`\s*\([^)]+\)$`) @@ -28,27 +29,31 @@ type testSummary struct { } type summaryRow struct { - Kind string `json:"kind"` - Name string `json:"name"` - Final bool `json:"final"` + Kind string `json:"kind"` + Name string `json:"name"` + Details string `json:"details"` + Final bool `json:"final"` } -func getFailures(ctx context.Context, runID int64) ([]string, error) { +// forEachSummaryZip downloads every test-summary artifact for the run and +// invokes fn with each artifact's name and the local path of its downloaded zip. +// Artifacts that fail to download are skipped. All downloads share a single temp +// dir that is removed when the function returns. +func forEachSummaryZip(ctx context.Context, runID int64, fn func(artifactName, zipPath string)) error { ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() artifacts, err := github.ListRunArtifacts(ctx, temporalRepository, runID) if err != nil { - return nil, err + return err } tempDir, err := os.MkdirTemp("", "ci-notify-artifacts-*") if err != nil { - return nil, err + return err } defer func() { _ = os.RemoveAll(tempDir) }() - var failures []string for _, artifact := range artifacts { if artifact.Expired || !strings.HasPrefix(artifact.Name, summaryArtifactPrefix) { continue @@ -58,40 +63,58 @@ func getFailures(ctx context.Context, runID int64) ([]string, error) { if err != nil { continue } - - artifactFailures, err := failuresFromZip(zipPath) - if err != nil { - continue - } - failures = append(failures, artifactFailures...) + fn(artifact.Name, zipPath) } + return nil +} + +func getFailures(ctx context.Context, runID int64) ([]string, error) { + var failures []string + err := forEachSummaryZip(ctx, runID, func(_, zipPath string) { + artifactFailures, err := failuresFromZip(zipPath) + if err != nil { + return + } + failures = append(failures, artifactFailures...) + }) + if err != nil { + return nil, err + } return uniqueSorted(failures), nil } func failuresFromZip(zipPath string) ([]string, error) { + rows, err := summaryRowsFromZip(zipPath) + if err != nil { + return nil, err + } + return reportableFailures(rows), nil +} + +func summaryRowsFromZip(zipPath string) ([]summaryRow, error) { reader, err := zip.OpenReader(zipPath) if err != nil { return nil, fmt.Errorf("failed to open artifact zip %s: %w", zipPath, err) } defer func() { _ = reader.Close() }() - var failures []string + var rows []summaryRow for _, file := range reader.File { if file.FileInfo().IsDir() || !strings.HasSuffix(file.Name, summaryFileSuffix) { continue } - fileFailures, err := failuresFromZipFile(file) + fileRows, err := summaryRowsFromZipFile(file) if err != nil { continue } - failures = append(failures, fileFailures...) + rows = append(rows, fileRows...) } - return failures, nil + return rows, nil } -func failuresFromZipFile(file *zip.File) ([]string, error) { +func summaryRowsFromZipFile(file *zip.File) ([]summaryRow, error) { rc, err := file.Open() if err != nil { return nil, fmt.Errorf("failed to open %s in artifact zip: %w", file.Name, err) @@ -102,7 +125,7 @@ func failuresFromZipFile(file *zip.File) ([]string, error) { if err := json.NewDecoder(rc).Decode(&summary); err != nil { return nil, fmt.Errorf("failed to parse %s in artifact zip: %w", file.Name, err) } - return reportableFailures(summary.Rows), nil + return summary.Rows, nil } func reportableFailures(rows []summaryRow) []string { diff --git a/tools/ci-notify/datarace.go b/tools/ci-notify/datarace.go new file mode 100644 index 0000000000..5fab9c3779 --- /dev/null +++ b/tools/ci-notify/datarace.go @@ -0,0 +1,254 @@ +package cinotify + +import ( + "context" + "fmt" + "regexp" + "slices" + "strings" + "time" + + "go.temporal.io/server/tools/common/github" +) + +// maxRaceSites caps how many conflicting-access lines we surface per race in +// case the test-runner merged several race reports into one alert. +const maxRaceSites = 6 + +var ( + // raceAccessRegex matches a race detector access header. The current access + // is "Read at"/"Write at"; the conflicting one is "Previous read at"/ + // "Previous write at". At least one is always a write, but a race can be + // write/write, so both kinds must be matched. + raceAccessRegex = regexp.MustCompile(`^(Read|Write|Previous read|Previous write) at 0x[0-9a-fA-F]+ by (goroutine \d+|main goroutine)`) + // raceFileLineRegex matches a ".go:" stack frame location. + raceFileLineRegex = regexp.MustCompile(`(\S+\.go):(\d+)`) + // repoPathPrefixRegex strips the CI checkout prefix (e.g. + // /home/runner/work/temporal/temporal/) so paths read as service/history/…. + repoPathPrefixRegex = regexp.MustCompile(`^.*/temporal/temporal/`) +) + +// temporalModulePrefix is trimmed from race locations so we show, e.g., +// "service/history.TestFoo" instead of the fully-qualified package path. +const temporalModulePrefix = "go.temporal.io/server/" + +// DataRace is a single data race detected by the Go race detector during a CI +// run. The test-runner surfaces these into the JUnit ALERTS suite and the +// test-summary.json artifact (see tools/testrunner/junit.go). +type DataRace struct { + // Location is the package-qualified test where the race was detected, + // e.g. "service/history.TestFoo". + Location string + // Details is the race detector's report (stacktraces), possibly truncated. + Details string + // JobID is the GitHub Actions job that reported the race, used to link + // directly to the offending job. Empty when it can't be determined. + JobID string +} + +// DataRaceReport aggregates the data races found in a single CI run along with +// the commit context needed to attribute and link them. +type DataRaceReport struct { + Run github.Run + Author string + Title string + DataRaces []DataRace +} + +// BuildDataRaceReport fetches the run's test summaries, extracts any data races, +// and enriches them with commit metadata. It returns a report with an empty +// DataRaces slice when no races were detected. +func BuildDataRaceReport(runID string) (*DataRaceReport, error) { + run, err := getWorkflowRun(runID) + if err != nil { + return nil, err + } + + races, err := getDataRaces(context.Background(), run.DatabaseID) + if err != nil { + return nil, err + } + + report := &DataRaceReport{ + Run: *run, + Title: run.DisplayTitle, + DataRaces: races, + } + + // Commit metadata is best-effort: a missing author must not suppress the alert. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if commit, err := github.GetCommit(ctx, temporalRepository, run.HeadSHA); err == nil { + report.Author = commit.Commit.Author.Name + if title := commit.Title(); title != "" { + report.Title = title + } + } + + return report, nil +} + +// getDataRaces downloads the run's test-summary artifacts and returns the unique +// set of data races reported across all of them. +func getDataRaces(ctx context.Context, runID int64) ([]DataRace, error) { + var races []DataRace + err := forEachSummaryZip(ctx, runID, func(artifactName, zipPath string) { + rows, err := summaryRowsFromZip(zipPath) + if err != nil { + return + } + races = append(races, dataRacesFromRows(rows, jobIDFromArtifactName(artifactName))...) + }) + if err != nil { + return nil, err + } + return uniqueDataRaces(races), nil +} + +// dataRacesFromRows extracts data race rows from a parsed test summary, tagging +// each with the job that produced the artifact. +func dataRacesFromRows(rows []summaryRow, jobID string) []DataRace { + var races []DataRace + for _, row := range rows { + if row.Kind != summaryKindDataRace { + continue + } + races = append(races, DataRace{ + Location: dataRaceLocation(row.Name), + Details: row.Details, + JobID: jobID, + }) + } + return races +} + +// dataRaceLocation reduces a test-runner alert name like +// "DATA RACE: Data race detected — in go.temporal.io/server/service/history.TestFoo" +// to just the package-qualified test, e.g. "service/history.TestFoo". +func dataRaceLocation(name string) string { + loc := strings.TrimSpace(strings.TrimPrefix(name, summaryKindDataRace+":")) + if _, after, ok := strings.Cut(loc, "— in "); ok { + loc = strings.TrimSpace(after) + } + return strings.TrimPrefix(loc, temporalModulePrefix) +} + +// raceSite is one conflicting memory access from a race detector report. +type raceSite struct { + access string // "Read", "Write", "Previous read", or "Previous write" + goroutine string // "goroutine 8" or "main goroutine" + location string // "service/history/mutable_state.go:127" +} + +// parseRaceSites reduces a race detector report to the conflicting memory +// accesses and the source line each occurred at. It handles read/write and +// write/write races. Returns nil when the report can't be parsed, so callers can +// degrade to just the location and job link. +func parseRaceSites(details string) []raceSite { + lines := strings.Split(details, "\n") + var sites []raceSite + for i, line := range lines { + m := raceAccessRegex.FindStringSubmatch(strings.TrimSpace(line)) + if m == nil { + continue + } + if loc := firstGoFrame(lines[i+1:]); loc != "" { + sites = append(sites, raceSite{access: m[1], goroutine: m[2], location: loc}) + if len(sites) == maxRaceSites { + break + } + } + } + return sites +} + +// raceSites formats the conflicting accesses for display, e.g. +// +// Read at (goroutine 8): service/history/mutable_state.go:127 +// Previous write at (goroutine 7): service/history/mutable_state.go:130 +func raceSites(details string) []string { + sites := parseRaceSites(details) + out := make([]string, 0, len(sites)) + for _, s := range sites { + out = append(out, fmt.Sprintf("%s at (%s): %s", s.access, s.goroutine, s.location)) + } + return out +} + +// raceAffectedLines returns the sorted, de-duplicated source lines involved in a +// race. Unlike the raw report, this excludes memory addresses and goroutine ids +// (which differ every run), so it identifies a race stably across shards. +func raceAffectedLines(details string) []string { + sites := parseRaceSites(details) + locations := make([]string, 0, len(sites)) + for _, s := range sites { + locations = append(locations, s.location) + } + slices.Sort(locations) + return slices.Compact(locations) +} + +// firstGoFrame returns ".go:" for the first application stack frame +// following an access header, skipping the race detector's runtime shim frames +// (e.g. runtime.raceread in race_amd64.s). It stops at the blank line that ends +// the access's stack. +func firstGoFrame(lines []string) string { + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + return "" // end of this access's stack; no application frame found + } + m := raceFileLineRegex.FindStringSubmatch(trimmed) + if m == nil || strings.Contains(m[1], "/src/runtime/") { + continue // function-name line, assembly shim, or Go runtime internals + } + return trimRepoPath(m[1]) + ":" + m[2] + } + return "" +} + +// trimRepoPath strips the CI checkout prefix from an absolute source path. +func trimRepoPath(path string) string { + return repoPathPrefixRegex.ReplaceAllString(path, "") +} + +// jobIDFromArtifactName extracts the job ID from a test-summary artifact name of +// the form "test-summary-json--------". +// Returns "" when the name doesn't carry a job ID. +func jobIDFromArtifactName(name string) string { + parts := strings.Split(name, "--") + if len(parts) < 3 { + return "" + } + return parts[2] +} + +// uniqueDataRaces removes duplicate races (the same race is reported by every +// shard/attempt that hit it) while preserving first-seen order. Races are keyed +// by their test and affected source lines rather than the raw report, so the +// same race still de-duplicates despite the differing memory addresses and +// goroutine ids the detector prints each run. +func uniqueDataRaces(races []DataRace) []DataRace { + seen := make(map[string]struct{}, len(races)) + var unique []DataRace + for _, race := range races { + key := raceFingerprint(race) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + unique = append(unique, race) + } + return unique +} + +// raceFingerprint is a stable identity for a race: its test plus the set of +// affected source lines. Falls back to the raw report when no source lines can +// be parsed, so unparseable reports aren't over-merged. +func raceFingerprint(race DataRace) string { + lines := raceAffectedLines(race.Details) + if len(lines) == 0 { + return race.Location + "\n" + race.Details + } + return race.Location + "\n" + strings.Join(lines, "\n") +} diff --git a/tools/ci-notify/datarace_test.go b/tools/ci-notify/datarace_test.go new file mode 100644 index 0000000000..55631387f2 --- /dev/null +++ b/tools/ci-notify/datarace_test.go @@ -0,0 +1,210 @@ +package cinotify + +import ( + "archive/zip" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "go.temporal.io/server/tools/common/github" +) + +func TestDataRacesFromRows(t *testing.T) { + rows := []summaryRow{ + {Kind: "Failed", Name: "TestFoo (final)", Final: true}, + { + Kind: "DATA RACE", + Name: "DATA RACE: Data race detected — in go.temporal.io/server/service/history.TestBar", + Details: "WARNING: DATA RACE\nRead at 0x00c0...", + }, + {Kind: "OOM", Name: "OOM prevention"}, + {Kind: "PANIC", Name: "PANIC: boom"}, + } + + races := dataRacesFromRows(rows, "job-1") + + require.Len(t, races, 1) + require.Equal(t, "service/history.TestBar", races[0].Location) + require.Equal(t, "job-1", races[0].JobID) + require.Contains(t, races[0].Details, "WARNING: DATA RACE") +} + +func TestDataRacesFromRowsNone(t *testing.T) { + rows := []summaryRow{ + {Kind: "Failed", Name: "TestFoo (final)", Final: true}, + {Kind: "OOM", Name: "OOM prevention"}, + } + require.Empty(t, dataRacesFromRows(rows, "job-1")) +} + +func TestDataRaceLocation(t *testing.T) { + require.Equal(t, + "service/history.TestBar", + dataRaceLocation("DATA RACE: Data race detected — in go.temporal.io/server/service/history.TestBar"), + ) + // A race not attributed to a temporal package keeps its qualified name. + require.Equal(t, + "example.com/foo.TestBaz", + dataRaceLocation("DATA RACE: Data race detected — in example.com/foo.TestBaz"), + ) + // A name without the "— in" locator falls back to the summary text. + require.Equal(t, "Data race detected", dataRaceLocation("DATA RACE: Data race detected")) +} + +func TestJobIDFromArtifactName(t *testing.T) { + require.Equal(t, "12345", jobIDFromArtifactName("test-summary-json--999--12345--1--unit-test")) + require.Empty(t, jobIDFromArtifactName("test-summary-json--999")) +} + +func TestUniqueDataRaces(t *testing.T) { + // The same race reported by two shards: identical source lines, but the race + // detector prints different memory addresses and goroutine ids each run. + shard1 := readWriteRaceDetails + shard2 := strings.NewReplacer( + "0x00c0001121e8", "0xdeadbeef", + "goroutine 8", "goroutine 42", + "goroutine 7", "goroutine 99", + ).Replace(shard1) + + // A race at a different source line is a distinct race. + otherLine := strings.ReplaceAll(shard1, "mutable_state.go:130", "mutable_state.go:200") + + races := []DataRace{ + {Location: "service/history.TestMS", Details: shard1, JobID: "1"}, + {Location: "service/history.TestMS", Details: shard2, JobID: "2"}, // dup of shard1 + {Location: "service/history.TestMS", Details: otherLine}, // different line + {Location: "service/matching.TestCache", Details: shard1}, // different test + } + + unique := uniqueDataRaces(races) + + require.Len(t, unique, 3) + require.Equal(t, "1", unique[0].JobID) // first occurrence wins + require.Equal(t, "service/history.TestMS", unique[1].Location) + require.Equal(t, "service/matching.TestCache", unique[2].Location) +} + +func TestSummaryRowsFromZipParsesDataRaceDetails(t *testing.T) { + dir := t.TempDir() + zipPath := filepath.Join(dir, "artifact.zip") + file, err := os.Create(zipPath) + require.NoError(t, err) + + writer := zip.NewWriter(file) + summaryFile, err := writer.Create("test-summary.json") + require.NoError(t, err) + _, err = summaryFile.Write([]byte(`{ + "rows": [ + { + "kind": "DATA RACE", + "name": "DATA RACE: Data race detected — in go.temporal.io/server/pkg.TestRacy", + "details": "WARNING: DATA RACE\nWrite at 0x00c000" + } + ] +}`)) + require.NoError(t, err) + require.NoError(t, writer.Close()) + require.NoError(t, file.Close()) + + rows, err := summaryRowsFromZip(zipPath) + require.NoError(t, err) + + races := dataRacesFromRows(rows, "job-9") + require.Len(t, races, 1) + require.Equal(t, "pkg.TestRacy", races[0].Location) + require.Contains(t, races[0].Details, "WARNING: DATA RACE") +} + +// readWriteRaceDetails is a read/write race whose top frame is the race +// detector's runtime shim (which must be skipped) and whose source paths use the +// CI checkout prefix (which must be trimmed). +const readWriteRaceDetails = `================== +WARNING: DATA RACE +Read at 0x00c0001121e8 by goroutine 8: + runtime.raceread() + /usr/local/go/src/runtime/race_amd64.s:260 +0x21 + go.temporal.io/server/service/history.(*ms).Get() + /home/runner/work/temporal/temporal/service/history/mutable_state.go:127 +0x2c + +Previous write at 0x00c0001121e8 by goroutine 7: + runtime.racewrite() + /usr/local/go/src/runtime/race_amd64.s:269 +0x21 + go.temporal.io/server/service/history.(*ms).Set() + /home/runner/work/temporal/temporal/service/history/mutable_state.go:130 +0x3c + +Goroutine 8 (running) created at: + go.temporal.io/server/service/history.TestMutableState() + /home/runner/work/temporal/temporal/service/history/mutable_state_test.go:41 +0x88 +==================` + +// writeWriteRaceDetails is a race between two concurrent writes. +const writeWriteRaceDetails = `================== +WARNING: DATA RACE +Write at 0x00c000000180 by goroutine 7: + go.temporal.io/server/service/matching.(*cache).put() + /home/runner/work/temporal/temporal/service/matching/cache.go:88 +0x66 + +Previous write at 0x00c000000180 by goroutine 12: + go.temporal.io/server/service/matching.(*cache).put() + /home/runner/work/temporal/temporal/service/matching/cache.go:88 +0x66 +==================` + +func TestRaceSites(t *testing.T) { + require.Equal(t, []string{ + "Read at (goroutine 8): service/history/mutable_state.go:127", + "Previous write at (goroutine 7): service/history/mutable_state.go:130", + }, raceSites(readWriteRaceDetails)) + + // A write/write race is still a race; both writes must be surfaced. + require.Equal(t, []string{ + "Write at (goroutine 7): service/matching/cache.go:88", + "Previous write at (goroutine 12): service/matching/cache.go:88", + }, raceSites(writeWriteRaceDetails)) + + // Unparseable details yield no sites; the caller falls back to location + link. + require.Empty(t, raceSites("some unrelated text")) +} + +func TestBuildDataRaceMessageLinksToJob(t *testing.T) { + report := &DataRaceReport{ + Run: github.Run{ + DatabaseID: 123456, + HeadSHA: "abc1234567890defghijk", + URL: "https://github.com/temporalio/temporal/actions/runs/123456", + }, + Author: "Test Author", + Title: "Some commit title", + DataRaces: []DataRace{ + {Location: "service/history.TestMutableState", Details: readWriteRaceDetails, JobID: "789"}, + }, + } + + rendered := BuildDataRaceMessage(report).RenderMarkdown() + + require.Contains(t, rendered, "Data Race Detected on Main Branch") + require.Contains(t, rendered, "Test Author") + require.Contains(t, rendered, "service/history.TestMutableState") + require.Contains(t, rendered, "Read at (goroutine 8): service/history/mutable_state.go:127") + require.Contains(t, rendered, "Previous write at (goroutine 7): service/history/mutable_state.go:130") + require.Contains(t, rendered, "abc1234") // short SHA link + // Links to the specific job, not just the top-level run. + require.Contains(t, rendered, "actions/runs/123456/job/789") + // Runtime shim frames and raw stacktrace noise are not dumped into Slack. + require.NotContains(t, rendered, "race_amd64.s") +} + +func TestBuildDataRaceMessageFallsBackToRunLink(t *testing.T) { + report := &DataRaceReport{ + Run: github.Run{DatabaseID: 123456, HeadSHA: "abc1234567890"}, + DataRaces: []DataRace{{Location: "pkg.TestRacy", Details: "unparseable"}}, // no JobID + } + + rendered := BuildDataRaceMessage(report).RenderMarkdown() + + require.Contains(t, rendered, "Unknown") // author + require.Contains(t, rendered, "pkg.TestRacy") + require.Contains(t, rendered, "actions/runs/123456") + require.NotContains(t, rendered, "/job/") +} diff --git a/tools/ci-notify/slack.go b/tools/ci-notify/slack.go index ec17b2ee87..9deb03b58e 100644 --- a/tools/ci-notify/slack.go +++ b/tools/ci-notify/slack.go @@ -2,8 +2,10 @@ package cinotify import ( "fmt" + "strconv" "strings" + "go.temporal.io/server/tools/common/github" "go.temporal.io/server/tools/common/slack" ) @@ -70,6 +72,52 @@ func FormatMessageForDebug(report *FailureReport) string { return sb.String() } +// BuildDataRaceMessage creates a Slack message announcing data races on main. +func BuildDataRaceMessage(report *DataRaceReport) *slack.Message { + runID := strconv.FormatInt(report.Run.DatabaseID, 10) + commitURL := github.CommitURL(temporalRepository, report.Run.HeadSHA) + + message := slack.NewMessage(fmt.Sprintf("Data Race Detected on Main (%d)", len(report.DataRaces))) + message.AddSection(":rotating_light: *Data Race Detected on Main Branch* :rotating_light:") + message.AddFields( + fmt.Sprintf("*Commit:*\n<%s|%s>", commitURL, report.Run.ShortSHA()), + fmt.Sprintf("*Author:*\n%s", orUnknown(report.Author)), + ) + if report.Title != "" { + message.AddSection(fmt.Sprintf("*Commit message:*\n%s", report.Title)) + } + + for _, race := range report.DataRaces { + var sb strings.Builder + if race.Location != "" { + fmt.Fprintf(&sb, "*%s*", race.Location) + } + for _, site := range raceSites(race.Details) { + fmt.Fprintf(&sb, "\n• %s", site) + } + fmt.Fprintf(&sb, "\n<%s|View job logs>", raceLink(runID, race)) + message.AddSection(sb.String()) + } + + return message +} + +func orUnknown(s string) string { + if s == "" { + return "Unknown" + } + return s +} + +// raceLink points at the specific job that reported the race so the alert is +// directly actionable, falling back to the run when the job is unknown. +func raceLink(runID string, race DataRace) string { + if race.JobID != "" { + return github.JobURL(temporalRepository, runID, race.JobID) + } + return github.RunURL(temporalRepository, runID) +} + // BuildSuccessReportMessage creates a Slack message for success report func BuildSuccessReportMessage(report *DigestReport) *slack.Message { message := slack.NewMessage(fmt.Sprintf("Weekly CI Report - %s Branch", report.Branch))