-
Notifications
You must be signed in to change notification settings - Fork 128
[SDCICD-1704] feat: expanded LLM report output #3098
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
base: main
Are you sure you want to change the base?
Conversation
christophermancini
commented
Jan 29, 2026
- include cluster information
- include failure excerpts from logs
- expand test coverage
- add integration tests for sending slack messages
- include cluster information - include failure excerpts from logs - expand test coverage - add integration tests for sending slack messages
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christophermancini The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| failureBlocks := s.extractFailureBlocks(lines, 0, totalLines) | ||
| if len(failureBlocks) > 0 { | ||
| var result strings.Builder | ||
| result.WriteString(fmt.Sprintf("Found %d test failure(s):\n\n", len(failureBlocks))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| result.WriteString(fmt.Sprintf("Found %d test failure(s):\n\n", len(failureBlocks))) | |
| result.WriteString(fmt.Sprintf("====== Log Extract ======\n"")) | |
| result.WriteString(fmt.Sprintf("Found %d test failure(s):\n\n", len(failureBlocks))) |
| message.Channel = channel | ||
|
|
||
| var builder strings.Builder | ||
| builder.WriteString("====== ☸️ Cluster Information ======\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can cluster details be sent as a last response after the logs?
I think the notification sequence is best as
- What failed ( Test Suite Information) - main message
- Briefly why (LLM cause / reco) - reply
- Evidence (log excerpt) - reply
- cluster info if you want to debug. The cluster is ephemeral so it's not the utmost important. - reply
| result.WriteString(fmt.Sprintf("Found %d test failure(s):\n\n", len(failureBlocks))) | ||
| for i, block := range failureBlocks { | ||
| if i > 0 { | ||
| result.WriteString("\n---\n\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| result.WriteString("\n---\n\n") | |
| result.WriteString("\n---\n\n```") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't apply as is, might get linter errors. But suggesting ``` to format logs as readable code in slack
| result.WriteString("\n---\n\n") | ||
| } | ||
| result.WriteString(block) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | |
| result.WriteString("```") | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't apply as is, might get linter errors. But suggesting ``` to format logs as readable code in slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a few different iterations of using the code block syntax but I couldn't get it to reliably work.
| ```bash | ||
| # Set environment variables | ||
| export LOG_ANALYSIS_SLACK_WEBHOOK="https://hooks.slack.com/workflows/..." | ||
| export LOG_ANALYSIS_SLACK_CHANNEL="C06HQR8HN0L" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to specify individual suite channels in TEST_SUITES json
|
|
||
| // readTestOutput reads the test stdout from test_output.txt, test_output.log, or build-log.txt | ||
| func (s *SlackReporter) readTestOutput(reportDir string) string { | ||
| for _, filename := range []string{"test_output.txt", "test_output.log", "build-log.txt"} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christophermancini This could be simplified I think.
https://github.com/openshift/osde2e/blob/main/cmd/osde2e/main.go#L72
https://github.com/openshift/osde2e/blob/main/cmd/osde2e/main.go#L51
We always use test_output.log
| var blocks []string | ||
|
|
||
| for i := startIdx; i < endIdx && len(blocks) < maxFailureBlocks; i++ { | ||
| if strings.Contains(lines[i], "[FAILED]") || strings.Contains(lines[i], "• [FAILED]") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christophermancini Would be nice to check for ERROR as well.
https://github.com/openshift/osde2e/blob/main/internal/aggregator/aggregator.go#L281
|
@christophermancini: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |