Fix panic when reading malformed warning attribute #7293
Conversation
Signed-off-by: Yuri Shkuro <github@ysh.us>
internal/jptrace/warning_test.go
Outdated
| func TestGetWarnings_EmptySpan(t *testing.T) { | ||
| span := ptrace.NewSpan() | ||
| span.Attributes().PutStr(WarningsAttribute, "warning-1") | ||
| actual := GetWarnings(span) | ||
| assert.Nil(t, actual) | ||
| } |
There was a problem hiding this comment.
The test name TestGetWarnings_EmptySpan doesn't match what the test is actually doing. The span isn't empty - it has a string warning attribute set.
More importantly, there's a discrepancy between the test expectation and the updated implementation. The test expects nil to be returned, but according to the new code in warning.go, when a string attribute is set, it should return a slice containing that string ([]string{"warning-1"}).
Either:
- Update the test expectation to
assert.Equal(t, []string{"warning-1"}, actual)to match the implementation, or - Rename the test to better describe what it's testing (e.g.,
TestGetWarnings_StringAttribute)
The current test doesn't properly validate the new fallback behavior for non-slice warning attributes.
| func TestGetWarnings_EmptySpan(t *testing.T) { | |
| span := ptrace.NewSpan() | |
| span.Attributes().PutStr(WarningsAttribute, "warning-1") | |
| actual := GetWarnings(span) | |
| assert.Nil(t, actual) | |
| } | |
| func TestGetWarnings_StringAttribute(t *testing.T) { | |
| span := ptrace.NewSpan() | |
| span.Attributes().PutStr(WarningsAttribute, "warning-1") | |
| actual := GetWarnings(span) | |
| assert.Equal(t, []string{"warning-1"}, actual) | |
| } | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7293 +/- ##
==========================================
- Coverage 96.21% 96.19% -0.03%
==========================================
Files 374 374
Lines 22686 22691 +5
==========================================
- Hits 21828 21827 -1
- Misses 641 645 +4
- Partials 217 219 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Which problem is this PR solving?
User reported panic
Description of the changes
How was this change tested?