Hyperfleet-361 : Use updated broker with logger context inheritance#45
Hyperfleet-361 : Use updated broker with logger context inheritance#45tirthct wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@pkg/logger/logger.go`:
- Around line 544-600: The MockLoggerWithContext methods (Debug, Debugf, Info,
Infof, Warn, Warnf, Error, Errorf, Fatal, Fatalf) can dereference nil
CapturedLogs/CapturedContexts pointers and panic; update each method (or add a
private helper used by them) to nil-check CapturedLogs and CapturedContexts and
initialize them to new slices when nil (e.g., if m.CapturedLogs == nil {
m.CapturedLogs = &[]string{} } and similarly for CapturedContexts) before
appending, ensuring Fatal/Fatalf still write to stderr and call os.Exit after
safe appends.
In `@test/integration/integration_test.go`:
- Line 405: The test function name has a typo: rename
TextIntegration_BrokerLoggerContext to TestIntegration_BrokerLoggerContext so
Go's test runner recognizes it; update the function declaration for
TextIntegration_BrokerLoggerContext(t *testing.T) to
TestIntegration_BrokerLoggerContext(t *testing.T) (keeping the same body and
signature) to ensure the test is executed by the testing framework.
- Around line 445-454: The sentinelConfig in the test is missing the Topic field
which leaves it empty and can make assertions pass for the wrong reason; update
the sentinelConfig (the variable of type config.SentinelConfig created here) to
include a non-empty Topic (e.g., "clusters.events" or another test-appropriate
topic) so published events include that topic and the test assertion at the
later check validates the real topic value; ensure you set the Topic field on
the same sentinelConfig struct initialization so code referencing
sentinelConfig.Topic receives the intended value.
- Around line 523-524: The log format string in the t.Logf call is truncated
("tr") and does not include the trace field or its argument; update the format
to include the full field name (e.g., "trace=%v") and add the corresponding
entry["trace"] to the argument list so the message prints decision_reason,
topic, subset and trace; modify the t.Logf call that references entry to reflect
this.
- Around line 456-457: The test function name is misspelled as
TextIntegration_BrokerLoggerContext so Go's test runner won't run it; rename the
function declaration and any references from TextIntegration_BrokerLoggerContext
to TestIntegration_BrokerLoggerContext (update the function identifier where
it's defined and any calls or references to that identifier) so the test is
discoverable and executes.
🧹 Nitpick comments (1)
internal/sentinel/sentinel_test.go (1)
414-425: Incomplete context verification.The test sets
subset(line 393) andspan_id(line 395) on the context but only verifiesdecision_reason,topic, andtrace_id. Consider adding assertions for the remaining context values for complete coverage.Proposed addition
if traceID, ok := brokerCtx.Value(logger.TraceIDCtxKey).(string); !ok || traceID != "trace-123" { t.Errorf("trace_id not propagated: got %v", traceID) } + + if subset, ok := brokerCtx.Value(logger.SubsetCtxKey).(string); !ok || subset != "clusters" { + t.Errorf("subset not propagated: got %v", subset) + } + + if spanID, ok := brokerCtx.Value(logger.SpanIDCtxKey).(string); !ok || spanID != "span-456" { + t.Errorf("span_id not propagated: got %v", spanID) + } }
| type MockLoggerWithContext struct { | ||
| CapturedLogs *[]string | ||
| CapturedContexts *[]context.Context | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Debug(ctx context.Context, message string) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, message) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Debugf(ctx context.Context, format string, args ...interface{}) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Info(ctx context.Context, message string) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, message) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Infof(ctx context.Context, format string, args ...interface{}) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Warn(ctx context.Context, message string) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, message) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Warnf(ctx context.Context, format string, args ...interface{}) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Error(ctx context.Context, message string) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, message) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Errorf(ctx context.Context, format string, args ...interface{}) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Fatal(ctx context.Context, message string) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, message) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", message) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| func (m *MockLoggerWithContext) Fatalf(ctx context.Context, format string, args ...interface{}) { | ||
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | ||
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | ||
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", fmt.Sprintf(format, args...)) | ||
| os.Exit(1) |
There was a problem hiding this comment.
Guard MockLoggerWithContext against nil slice pointers.
Lines 544–600 will panic if a zero-value MockLoggerWithContext is used because CapturedLogs/CapturedContexts are nil pointers. Adding a small guard avoids NPEs and keeps the mock safe by default.
🛡️ Suggested nil-safe capture helper
type MockLoggerWithContext struct {
CapturedLogs *[]string
CapturedContexts *[]context.Context
}
+func (m *MockLoggerWithContext) capture(ctx context.Context, msg string) {
+ if m.CapturedLogs == nil {
+ m.CapturedLogs = &[]string{}
+ }
+ if m.CapturedContexts == nil {
+ m.CapturedContexts = &[]context.Context{}
+ }
+ *m.CapturedLogs = append(*m.CapturedLogs, msg)
+ *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+}
+
func (m *MockLoggerWithContext) Debug(ctx context.Context, message string) {
- *m.CapturedLogs = append(*m.CapturedLogs, message)
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, message)
}
func (m *MockLoggerWithContext) Debugf(ctx context.Context, format string, args ...interface{}) {
- *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...))
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, fmt.Sprintf(format, args...))
}
func (m *MockLoggerWithContext) Info(ctx context.Context, message string) {
- *m.CapturedLogs = append(*m.CapturedLogs, message)
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, message)
}
func (m *MockLoggerWithContext) Infof(ctx context.Context, format string, args ...interface{}) {
- *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...))
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, fmt.Sprintf(format, args...))
}
func (m *MockLoggerWithContext) Warn(ctx context.Context, message string) {
- *m.CapturedLogs = append(*m.CapturedLogs, message)
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, message)
}
func (m *MockLoggerWithContext) Warnf(ctx context.Context, format string, args ...interface{}) {
- *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...))
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, fmt.Sprintf(format, args...))
}
func (m *MockLoggerWithContext) Error(ctx context.Context, message string) {
- *m.CapturedLogs = append(*m.CapturedLogs, message)
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, message)
}
func (m *MockLoggerWithContext) Errorf(ctx context.Context, format string, args ...interface{}) {
- *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...))
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, fmt.Sprintf(format, args...))
}
func (m *MockLoggerWithContext) Fatal(ctx context.Context, message string) {
- *m.CapturedLogs = append(*m.CapturedLogs, message)
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, message)
fmt.Fprintf(os.Stderr, "FATAL: %s\n", message)
os.Exit(1)
}
func (m *MockLoggerWithContext) Fatalf(ctx context.Context, format string, args ...interface{}) {
- *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...))
- *m.CapturedContexts = append(*m.CapturedContexts, ctx)
+ m.capture(ctx, fmt.Sprintf(format, args...))
fmt.Fprintf(os.Stderr, "FATAL: %s\n", fmt.Sprintf(format, args...))
os.Exit(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type MockLoggerWithContext struct { | |
| CapturedLogs *[]string | |
| CapturedContexts *[]context.Context | |
| } | |
| func (m *MockLoggerWithContext) Debug(ctx context.Context, message string) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, message) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Debugf(ctx context.Context, format string, args ...interface{}) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Info(ctx context.Context, message string) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, message) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Infof(ctx context.Context, format string, args ...interface{}) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Warn(ctx context.Context, message string) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, message) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Warnf(ctx context.Context, format string, args ...interface{}) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Error(ctx context.Context, message string) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, message) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Errorf(ctx context.Context, format string, args ...interface{}) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Fatal(ctx context.Context, message string) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, message) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", message) | |
| os.Exit(1) | |
| } | |
| func (m *MockLoggerWithContext) Fatalf(ctx context.Context, format string, args ...interface{}) { | |
| *m.CapturedLogs = append(*m.CapturedLogs, fmt.Sprintf(format, args...)) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", fmt.Sprintf(format, args...)) | |
| os.Exit(1) | |
| type MockLoggerWithContext struct { | |
| CapturedLogs *[]string | |
| CapturedContexts *[]context.Context | |
| } | |
| func (m *MockLoggerWithContext) capture(ctx context.Context, msg string) { | |
| if m.CapturedLogs == nil { | |
| m.CapturedLogs = &[]string{} | |
| } | |
| if m.CapturedContexts == nil { | |
| m.CapturedContexts = &[]context.Context{} | |
| } | |
| *m.CapturedLogs = append(*m.CapturedLogs, msg) | |
| *m.CapturedContexts = append(*m.CapturedContexts, ctx) | |
| } | |
| func (m *MockLoggerWithContext) Debug(ctx context.Context, message string) { | |
| m.capture(ctx, message) | |
| } | |
| func (m *MockLoggerWithContext) Debugf(ctx context.Context, format string, args ...interface{}) { | |
| m.capture(ctx, fmt.Sprintf(format, args...)) | |
| } | |
| func (m *MockLoggerWithContext) Info(ctx context.Context, message string) { | |
| m.capture(ctx, message) | |
| } | |
| func (m *MockLoggerWithContext) Infof(ctx context.Context, format string, args ...interface{}) { | |
| m.capture(ctx, fmt.Sprintf(format, args...)) | |
| } | |
| func (m *MockLoggerWithContext) Warn(ctx context.Context, message string) { | |
| m.capture(ctx, message) | |
| } | |
| func (m *MockLoggerWithContext) Warnf(ctx context.Context, format string, args ...interface{}) { | |
| m.capture(ctx, fmt.Sprintf(format, args...)) | |
| } | |
| func (m *MockLoggerWithContext) Error(ctx context.Context, message string) { | |
| m.capture(ctx, message) | |
| } | |
| func (m *MockLoggerWithContext) Errorf(ctx context.Context, format string, args ...interface{}) { | |
| m.capture(ctx, fmt.Sprintf(format, args...)) | |
| } | |
| func (m *MockLoggerWithContext) Fatal(ctx context.Context, message string) { | |
| m.capture(ctx, message) | |
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", message) | |
| os.Exit(1) | |
| } | |
| func (m *MockLoggerWithContext) Fatalf(ctx context.Context, format string, args ...interface{}) { | |
| m.capture(ctx, fmt.Sprintf(format, args...)) | |
| fmt.Fprintf(os.Stderr, "FATAL: %s\n", fmt.Sprintf(format, args...)) | |
| os.Exit(1) | |
| } |
🤖 Prompt for AI Agents
In `@pkg/logger/logger.go` around lines 544 - 600, The MockLoggerWithContext
methods (Debug, Debugf, Info, Infof, Warn, Warnf, Error, Errorf, Fatal, Fatalf)
can dereference nil CapturedLogs/CapturedContexts pointers and panic; update
each method (or add a private helper used by them) to nil-check CapturedLogs and
CapturedContexts and initialize them to new slices when nil (e.g., if
m.CapturedLogs == nil { m.CapturedLogs = &[]string{} } and similarly for
CapturedContexts) before appending, ensuring Fatal/Fatalf still write to stderr
and call os.Exit after safe appends.
| t.Logf("TSL syntax validation completed - received correct format: %s", receivedSearchParam) | ||
| } | ||
|
|
||
| func TextIntegration_BrokerLoggerContext(t *testing.T) { |
There was a problem hiding this comment.
Critical: Test function name typo - test will not run.
The function is named TextIntegration_BrokerLoggerContext (with "Text") instead of TestIntegration_BrokerLoggerContext (with "Test"). Go's testing framework only recognizes functions starting with Test, so this test will be silently skipped.
🐛 Proposed fix
-func TextIntegration_BrokerLoggerContext(t *testing.T) {
+func TestIntegration_BrokerLoggerContext(t *testing.T) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TextIntegration_BrokerLoggerContext(t *testing.T) { | |
| func TestIntegration_BrokerLoggerContext(t *testing.T) { |
🤖 Prompt for AI Agents
In `@test/integration/integration_test.go` at line 405, The test function name has
a typo: rename TextIntegration_BrokerLoggerContext to
TestIntegration_BrokerLoggerContext so Go's test runner recognizes it; update
the function declaration for TextIntegration_BrokerLoggerContext(t *testing.T)
to TestIntegration_BrokerLoggerContext(t *testing.T) (keeping the same body and
signature) to ensure the test is executed by the testing framework.
| sentinelConfig := &config.SentinelConfig{ | ||
| ResourceType: "clusters", | ||
| PollInterval: 100 * time.Millisecond, | ||
| MaxAgeNotReady: 10 * time.Second, | ||
| MaxAgeReady: 30 * time.Minute, | ||
| ResourceSelector: []config.LabelSelector{ | ||
| {Label: "region", Value: "us-east"}, | ||
| {Label: "env", Value: "production"}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Missing Topic field in sentinel configuration.
The sentinelConfig is missing the Topic field, which means it will default to an empty string. This could cause unexpected behavior when publishing events and make the test assertion at line 510 (entry["topic"] == nil) pass for the wrong reason.
Proposed fix
sentinelConfig := &config.SentinelConfig{
ResourceType: "clusters",
PollInterval: 100 * time.Millisecond,
MaxAgeNotReady: 10 * time.Second,
MaxAgeReady: 30 * time.Minute,
+ Topic: "test-topic",
ResourceSelector: []config.LabelSelector{
{Label: "region", Value: "us-east"},
{Label: "env", Value: "production"},
},
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sentinelConfig := &config.SentinelConfig{ | |
| ResourceType: "clusters", | |
| PollInterval: 100 * time.Millisecond, | |
| MaxAgeNotReady: 10 * time.Second, | |
| MaxAgeReady: 30 * time.Minute, | |
| ResourceSelector: []config.LabelSelector{ | |
| {Label: "region", Value: "us-east"}, | |
| {Label: "env", Value: "production"}, | |
| }, | |
| } | |
| sentinelConfig := &config.SentinelConfig{ | |
| ResourceType: "clusters", | |
| PollInterval: 100 * time.Millisecond, | |
| MaxAgeNotReady: 10 * time.Second, | |
| MaxAgeReady: 30 * time.Minute, | |
| Topic: "test-topic", | |
| ResourceSelector: []config.LabelSelector{ | |
| {Label: "region", Value: "us-east"}, | |
| {Label: "env", Value: "production"}, | |
| }, | |
| } |
🤖 Prompt for AI Agents
In `@test/integration/integration_test.go` around lines 445 - 454, The
sentinelConfig in the test is missing the Topic field which leaves it empty and
can make assertions pass for the wrong reason; update the sentinelConfig (the
variable of type config.SentinelConfig created here) to include a non-empty
Topic (e.g., "clusters.events" or another test-appropriate topic) so published
events include that topic and the test assertion at the later check validates
the real topic value; ensure you set the Topic field on the same sentinelConfig
struct initialization so code referencing sentinelConfig.Topic receives the
intended value.
| // Create Sentinel with our logger and real RabbitMQ broker | ||
| s := sentinel.NewSentinel(sentinelConfig, hyperfleetClient, decisionEngine, helper.RabbitMQ.Publisher(), log) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, examine the test around lines 456-457 with more context
cat -n test/integration/integration_test.go | sed -n '430,470p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 1750
🏁 Script executed:
#!/bin/bash
# Check how RabbitMQ is used in the NewHelper function
rg -n "NewHelper" test/integration/ -A 5 | head -50Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 2554
🏁 Script executed:
#!/bin/bash
# Find the testcontainer.go file and check publisher initialization
find test -name "testcontainer.go" -o -name "*testcontainer*"Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Get more context on the test around line 417-457 to see logger setup
cat -n test/integration/integration_test.go | sed -n '410,460p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 2380
🏁 Script executed:
#!/bin/bash
# Check the complete NewHelper function and RabbitMQ initialization
cat -n test/integration/testcontainer.go | sed -n '120,200p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 1389
🏁 Script executed:
#!/bin/bash
# Search for RabbitMQ Publisher creation in testcontainer
rg -n "Publisher\(\)" test/integration/testcontainer.go -B 5 -A 5Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 447
🏁 Script executed:
#!/bin/bash
# Look for where foundBrokerOperationLog is checked in the test
rg -n "foundBrokerOperationLog\|brokerOperationLog" test/integration/integration_test.go -B 3 -A 3Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Search for any log assertions in the test, particularly around publishing
rg -n "logBuffer\|AssertLog\|assert.*log" test/integration/integration_test.go -B 2 -A 2Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Check what the test actually validates after creating the sentinel
cat -n test/integration/integration_test.go | sed -n '455,500p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 1523
🏁 Script executed:
#!/bin/bash
# Search the entire test function to see all assertions
rg -n "func TestContextInheritance" test/integration/integration_test.go -A 100 | head -120Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Continue reading the test to see how foundBrokerOperationLog is used
cat -n test/integration/integration_test.go | sed -n '500,550p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 2422
🏁 Script executed:
#!/bin/bash
# Check the full test function name and its assertions
rg -n "func Test" test/integration/integration_test.go | grep -i contextRepository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Search for messages that might match broker operations
rg -n "broker\|publish\|message" test/integration/integration_test.go | head -20Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Check the rest of the test assertions
cat -n test/integration/integration_test.go | sed -n '545,560p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 697
🏁 Script executed:
#!/bin/bash
# Let's understand the test function name and purpose
rg -n "func TestContextInheritance\|func Test.*ntegration" test/integration/integration_test.go -B 2 -A 5Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Check if NewRabbitMQTestContainer creates a publisher and with what logger
cat -n test/integration/testcontainer.go | sed -n '1,120p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 4340
🏁 Script executed:
#!/bin/bash
# Check what the default logger output is configured to
rg -n "NewHyperFleetLogger\|GetGlobalConfig" pkg/logger/ -B 2 -A 5Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Verify if broker.NewPublisher actually logs anything
find . -name "*.go" -path "*/broker/*" | head -5Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Check if there's a way the logger is shared/passed through
rg -n "component.*sentinel\|Component.*sentinel" test/integration/integration_test.goRepository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Let me check the logger implementation to understand how it works
find . -path "*/pkg/logger*" -name "*.go" | head -10Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Check if there's any logger passing through the broker interface
rg -n "type Publisher\|interface Publisher" --type go | grep -i brokerRepository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 66
🏁 Script executed:
#!/bin/bash
# See if the custom logger is actually being used anywhere for broker operations
cat -n test/integration/integration_test.go | sed -n '400,430p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 1289
🏁 Script executed:
#!/bin/bash
# Verify the test function name more carefully
rg -n "^func Text" test/integration/integration_test.goRepository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Check if this is indeed a typo or intentional
rg -n "^func Test" test/integration/integration_test.go | head -10Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Get the complete function definition for context
cat -n test/integration/integration_test.go | sed -n '405,410p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 331
🏁 Script executed:
#!/bin/bash
# Double-check this is the test being reviewed - lines 405-560
git log --oneline test/integration/integration_test.go | head -5Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Verify the test function name is indeed the issue
cat -n test/integration/integration_test.go | sed -n '405,406p'Repository: openshift-hyperfleet/hyperfleet-sentinel
Length of output: 181
Test function name is invalid and will not execute.
The test function TextIntegration_BrokerLoggerContext (line 405) starts with Text instead of Test, which prevents Go's test runner from recognizing and executing it. Rename the function to TestIntegration_BrokerLoggerContext.
🤖 Prompt for AI Agents
In `@test/integration/integration_test.go` around lines 456 - 457, The test
function name is misspelled as TextIntegration_BrokerLoggerContext so Go's test
runner won't run it; rename the function declaration and any references from
TextIntegration_BrokerLoggerContext to TestIntegration_BrokerLoggerContext
(update the function identifier where it's defined and any calls or references
to that identifier) so the test is discoverable and executes.
| t.Logf("Found Sentinel event log with context: decision_reason=%v topic=%v subset=%v tr", | ||
| entry["decision_reason"], entry["topic"], entry["subset"]) |
There was a problem hiding this comment.
Truncated log message.
The log message appears to be incomplete, ending with "tr" instead of including the full trace information. This looks like an editing error.
Proposed fix
- t.Logf("Found Sentinel event log with context: decision_reason=%v topic=%v subset=%v tr",
- entry["decision_reason"], entry["topic"], entry["subset"])
+ t.Logf("Found Sentinel event log with context: decision_reason=%v topic=%v subset=%v trace_id=%v span_id=%v",
+ entry["decision_reason"], entry["topic"], entry["subset"], entry["trace_id"], entry["span_id"])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| t.Logf("Found Sentinel event log with context: decision_reason=%v topic=%v subset=%v tr", | |
| entry["decision_reason"], entry["topic"], entry["subset"]) | |
| t.Logf("Found Sentinel event log with context: decision_reason=%v topic=%v subset=%v trace_id=%v span_id=%v", | |
| entry["decision_reason"], entry["topic"], entry["subset"], entry["trace_id"], entry["span_id"]) |
🤖 Prompt for AI Agents
In `@test/integration/integration_test.go` around lines 523 - 524, The log format
string in the t.Logf call is truncated ("tr") and does not include the trace
field or its argument; update the format to include the full field name (e.g.,
"trace=%v") and add the corresponding entry["trace"] to the argument list so the
message prints decision_reason, topic, subset and trace; modify the t.Logf call
that references entry to reflect this.
1 participant
Changed
Tested
Summary by CodeRabbit
API Changes
Warning/Warningf→Warn/WarnfNew Features
Dependencies
Documentation