Fix: AddWarnings panics on non-slice WarningsAttribute (fixes #8002)

[shivamtiwari3]

Summary

Fixes #8002.

AddWarnings in internal/jptrace/warning.go panics with a nil-pointer dereference when WarningsAttribute already exists in a span's attributes but is stored as a non-slice type (e.g. a plain string written by Elasticsearch or another backend).

---

Root Cause

In warning.go:13-14, the code calls currWarnings.Slice() unconditionally after confirming the attribute is present:

if currWarnings, ok := span.Attributes().Get(WarningsAttribute); ok {
    w = currWarnings.Slice()   // panics if currWarnings is not a slice

pcommon.Value.Slice() on a non-ValueTypeSlice value returns an uninitialised pcommon.Slice whose internal *State field is nil. When AppendEmpty() is subsequently called on that empty Slice, it invokes (*State).AssertMutable() on a nil pointer, producing the panic seen in the stack trace:

go.opentelemetry.io/collector/pdata/internal.(*State).AssertMutable  internal/state.go:58
go.opentelemetry.io/collector/pdata/pcommon.Slice.AppendEmpty         generated_slice.go:96
github.com/jaegertracing/jaeger/internal/jptrace.AddWarnings           warning.go:19

The same function family already handles this correctly: GetWarnings uses a switch wa.Type() check before accessing wa.Slice(). AddWarnings was missing an equivalent guard.

---

Solution

Add && currWarnings.Type() == pcommon.ValueTypeSlice to the existing condition. When the attribute exists but is not a slice, the code falls through to PutEmptySlice, which atomically overwrites the malformed attribute with a fresh, mutable slice — matching the behaviour when the attribute is absent entirely.

if currWarnings, ok := span.Attributes().Get(WarningsAttribute); ok && currWarnings.Type() == pcommon.ValueTypeSlice {
    w = currWarnings.Slice()
} else {
    w = span.Attributes().PutEmptySlice(WarningsAttribute)
}

---

Testing

• Added TestAddWarning_NonSliceAttribute in warning_test.go that reproduces the exact failure: it sets WarningsAttribute as a plain string, then calls AddWarnings and asserts require.NotPanics + correct slice content.
• All existing tests in internal/jptrace/... continue to pass.
• Run with: go test ./internal/jptrace/... -v

---

Checklist

• Fixes the root cause (not just the symptom)
• New test covers the exact failing scenario from the issue
• All existing tests pass
• No unrelated changes
• Code style matches project conventions
• Read CONTRIBUTING.md and followed its requirements

[Copilot]

Pull request overview

This PR fixes a potential nil-pointer dereference bug in the AddWarnings function that occurs when the WarningsAttribute already exists but is stored as a non-slice type (e.g., a plain string from Elasticsearch or another backend).

Changes:

• Added a type check in AddWarnings to verify the attribute is a slice before attempting to use it
• Added a comprehensive test case that verifies AddWarnings handles malformed non-slice attributes without panicking

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/jptrace/warning.go	Added type check to handle non-slice WarningsAttribute values
internal/jptrace/warning_test.go	Added test case for malformed non-slice attribute handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shivamtiwari3]

Re-pushed with Signed-off-by trailer to fix the DCO check. The commit content is unchanged.

[yurishkuro]

What happens to existing attribute with WarningsAttribute key when you do this? Doesn't this override it? The solution might be to upgrade the type from a plain string to a slice, not to override.

[shivamtiwari3]

Applied — when WarningsAttribute exists as a non-slice type, the fix now reads the existing string via AsString(), creates a new slice, prepends the old value as the first element, and then appends the new warnings. The test is updated accordingly to assert that the existing string is preserved as Slice().At(0) and the new warning appears as Slice().At(1).

[Copilot]

Pull request overview

This PR adds defensive handling to the AddWarnings function to gracefully handle cases where the WarningsAttribute already exists but is stored as a non-slice type (e.g., a plain string from Elasticsearch or other backends). Previously, the code would panic when calling .Slice() on a non-slice value. The fix detects the attribute type and upgrades non-slice values to slices while preserving the existing value.

Changes:

• Added type checking in AddWarnings to detect non-slice attributes
• When a non-slice attribute is found, it's upgraded to a slice with the existing value preserved as the first element
• Added comprehensive test case TestAddWarning_NonSliceAttribute to verify the behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/jptrace/warning.go	Added type check and upgrade logic for non-slice warning attributes
internal/jptrace/warning_test.go	Added test to verify AddWarnings handles malformed (non-slice) attributes without panicking

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.