Include timestamp in test cluster warn/error output#20673
Include timestamp in test cluster warn/error output#20673andrross wants to merge 1 commit intoopensearch-project:mainfrom
Conversation
Previously the code to capture and dedup error and warn messages from test nodes would remove the timestamp from the message. This made debugging timing issues difficult. For example, the message would look like: ``` » WARN ][o.o.d.FileBasedSeedHostsProvider] [v2.19.5-0] expected, but did not find, a dynamic hosts list at [/home/ubuntu/workplace/opensearch-project/OpenSearch/qa/rolling-upgrade/build/testclusters/v2.19.5-0/config/unicast_hosts.txt] » ↑ repeated 2 times ↑ ``` With this change it looks like: ``` » [2026-02-18T20:04:29,167][WARN ][o.o.d.FileBasedSeedHostsProvider] [v2.19.5-0] expected, but did not find, a dynamic hosts list at [/home/ubuntu/workplace/opensearch-project/OpenSearch/qa/rolling-upgrade/build/testclusters/v2.19.5-0/config/unicast_hosts.txt] » ↑ repeated 2 times ↑ ``` Signed-off-by: Andrew Ross <andrross@amazon.com>
📝 WalkthroughWalkthroughRefactors log parsing in OpenSearchNode to use a new LogMessage wrapper that preserves both full and normalized message text, replacing String-based keys with LogMessage keys in the errors and warnings map. Custom equality and hashing based on normalized messages enables proper deduplication and counting. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
|
❗ AI-powered Code-Diff-Analyzer found issues on commit 82e3350.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java
🧰 Additional context used
🪛 GitHub Actions: Gradle Check (Jenkins)
buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java
[warning] 137-137: AI Analysis: Adding 'System::setSecurityManager' to ignored warning messages. Suppressing this warning could hide security policy modifications during tests.
[warning] 134-134: AI Analysis: Suppressing warnings about restricted methods in java.lang.foreign.Linker (FFI). Suppressing these warnings could hide native code access concerns.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: Analyze (java)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: detect-breaking-change
🔇 Additional comments (2)
buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java (2)
1009-1064: LGTM — full message preservation with normalized dedup looks solid.The LogMessage-based aggregation keeps timestamps in output while retaining dedup behavior. This aligns with the PR objective and reads cleanly.
1066-1078: LGTM — normalized equality on LogMessage is appropriate.Custom
equals/hashCodebased onnormalizedMessageis a good fit for dedup semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java`:
- Around line 133-139: The test log suppression list in OpenSearchNode currently
includes security-relevant entries (e.g., "java.lang.foreign.Linker",
"System::setSecurityManager") which can hide important warnings; instead,
restrict these suppressions by adding a scoped gating mechanism in
OpenSearchNode such as a JVM/version check or an opt-in flag (e.g., add a
boolean like allowSecuritySuppressions or check Runtime.version()/javaVendor)
and only append those two entries to the ignoredMessages list when the gate
passes; update the code paths that build the ignore list (references:
OpenSearchNode, the ignoredMessages/ignore list construction) to document the
gate and ensure default behavior leaves these warnings visible.
buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java
Show resolved
Hide resolved
andrross
added
the
skip-diff-analyzer
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20673 +/- ##
============================================
+ Coverage 73.19% 73.28% +0.09%
- Complexity 71924 72030 +106
============================================
Files 5781 5781
Lines 329292 329403 +111
Branches 47514 47527 +13
============================================
+ Hits 241026 241407 +381
+ Misses 68925 68618 -307
- Partials 19341 19378 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Previously the code to capture and dedup error and warn messages from test nodes would remove the timestamp from the message. This made debugging timing issues difficult. For example, the message would look like:
With this change it looks like:
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.