From 9f2725c4f8f6c32eccf83a8ada5c8b0d46385fd4 Mon Sep 17 00:00:00 2001 From: rubvs Date: Tue, 6 Aug 2024 09:51:22 -0400 Subject: [PATCH 1/5] fix otel exception message propagation (#13603) --- model/modelprocessor/errormessage.go | 2 +- model/modelprocessor/errormessage_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/model/modelprocessor/errormessage.go b/model/modelprocessor/errormessage.go index efc1c293..6094bbe9 100644 --- a/model/modelprocessor/errormessage.go +++ b/model/modelprocessor/errormessage.go @@ -32,7 +32,7 @@ func (s SetErrorMessage) ProcessBatch(ctx context.Context, b *modelpb.Batch) err for i := range *b { event := (*b)[i] if event.Error != nil { - event.Message = s.setErrorMessage(event) + event.Error.Message = s.setErrorMessage(event) } } return nil diff --git a/model/modelprocessor/errormessage_test.go b/model/modelprocessor/errormessage_test.go index 60757e18..dd94d82e 100644 --- a/model/modelprocessor/errormessage_test.go +++ b/model/modelprocessor/errormessage_test.go @@ -59,7 +59,6 @@ func TestSetErrorMessage(t *testing.T) { processor := modelprocessor.SetErrorMessage{} err := processor.ProcessBatch(context.Background(), &batch) assert.NoError(t, err) - assert.Equal(t, test.message, batch[0].Message) + assert.Equal(t, test.message, batch[0].Error.Message) } - } From 087d2446e0fc848d1d65d0ec4632027ad51608a2 Mon Sep 17 00:00:00 2001 From: Ruben van Staden Date: Sun, 18 Aug 2024 21:21:02 -0400 Subject: [PATCH 2/5] set event message to error message if empty --- model/modelprocessor/errormessage.go | 3 + model/modelprocessor/errormessage_test.go | 76 +++++++++++++++++++---- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/model/modelprocessor/errormessage.go b/model/modelprocessor/errormessage.go index 6094bbe9..1d4134e4 100644 --- a/model/modelprocessor/errormessage.go +++ b/model/modelprocessor/errormessage.go @@ -33,6 +33,9 @@ func (s SetErrorMessage) ProcessBatch(ctx context.Context, b *modelpb.Batch) err event := (*b)[i] if event.Error != nil { event.Error.Message = s.setErrorMessage(event) + if event.GetMessage() == "" { + event.Message = event.Error.Message + } } } return nil diff --git a/model/modelprocessor/errormessage_test.go b/model/modelprocessor/errormessage_test.go index dd94d82e..c03170c4 100644 --- a/model/modelprocessor/errormessage_test.go +++ b/model/modelprocessor/errormessage_test.go @@ -27,38 +27,90 @@ import ( "github.com/elastic/apm-data/model/modelprocessor" ) +// Copy the error message to the event message if event message is empty. func TestSetErrorMessage(t *testing.T) { tests := []struct { - input *modelpb.Error - message string + input *modelpb.Error + expected string }{{ - input: &modelpb.Error{}, - message: "", + input: &modelpb.Error{}, + expected: "", }, { - input: &modelpb.Error{Log: &modelpb.ErrorLog{Message: "log_message"}}, - message: "log_message", + input: &modelpb.Error{Log: &modelpb.ErrorLog{Message: "log_message"}}, + expected: "log_message", }, { - input: &modelpb.Error{Exception: &modelpb.Exception{Message: "exception_message"}}, - message: "exception_message", + input: &modelpb.Error{Exception: &modelpb.Exception{Message: "exception_message"}}, + expected: "exception_message", }, { input: &modelpb.Error{ Log: &modelpb.ErrorLog{}, Exception: &modelpb.Exception{Message: "exception_message"}, }, - message: "exception_message", + expected: "exception_message", }, { input: &modelpb.Error{ Log: &modelpb.ErrorLog{Message: "log_message"}, Exception: &modelpb.Exception{Message: "exception_message"}, }, - message: "log_message", + expected: "log_message", }} - for _, test := range tests { batch := modelpb.Batch{{Error: test.input}} processor := modelprocessor.SetErrorMessage{} err := processor.ProcessBatch(context.Background(), &batch) assert.NoError(t, err) - assert.Equal(t, test.message, batch[0].Error.Message) + assert.Equal(t, test.expected, batch[0].Message) + assert.Equal(t, test.expected, batch[0].Error.Message) + } +} + +// If event message is non-empty, do not override with error message. +func TestSetEventMessage(t *testing.T) { + tests := []struct { + input *modelpb.APMEvent + expected string + }{{ + input: &modelpb.APMEvent{ + Message: "message", + }, + expected: "message", + }, { + input: &modelpb.APMEvent{ + Error: &modelpb.Error{Log: &modelpb.ErrorLog{Message: "log_message"}}, + Message: "message", + }, + expected: "message", + }, { + input: &modelpb.APMEvent{ + Error: &modelpb.Error{Exception: &modelpb.Exception{Message: "exception_message"}}, + Message: "message", + }, + expected: "message", + }, { + input: &modelpb.APMEvent{ + Error: &modelpb.Error{ + Log: &modelpb.ErrorLog{}, + Exception: &modelpb.Exception{Message: "exception_message"}, + }, + Message: "message", + }, + expected: "message", + }, { + input: &modelpb.APMEvent{ + Error: &modelpb.Error{ + Log: &modelpb.ErrorLog{Message: "log_message"}, + Exception: &modelpb.Exception{Message: "exception_message"}, + }, + Message: "message", + }, + expected: "message", + }} + + for _, test := range tests { + batch := modelpb.Batch{test.input} + processor := modelprocessor.SetErrorMessage{} + err := processor.ProcessBatch(context.Background(), &batch) + assert.NoError(t, err) + assert.Equal(t, test.expected, batch[0].Message) } } From 066a706979a7d49cb96db3f9085dab2f56bac517 Mon Sep 17 00:00:00 2001 From: Ruben van Staden Date: Sun, 25 Aug 2024 16:14:51 -0400 Subject: [PATCH 3/5] fix error message propagation --- model/modelprocessor/errormessage.go | 3 +- model/modelprocessor/errormessage_test.go | 94 +++++++++-------------- 2 files changed, 38 insertions(+), 59 deletions(-) diff --git a/model/modelprocessor/errormessage.go b/model/modelprocessor/errormessage.go index 1d4134e4..952e4453 100644 --- a/model/modelprocessor/errormessage.go +++ b/model/modelprocessor/errormessage.go @@ -32,9 +32,8 @@ func (s SetErrorMessage) ProcessBatch(ctx context.Context, b *modelpb.Batch) err for i := range *b { event := (*b)[i] if event.Error != nil { - event.Error.Message = s.setErrorMessage(event) if event.GetMessage() == "" { - event.Message = event.Error.Message + event.Message = s.setErrorMessage(event) } } } diff --git a/model/modelprocessor/errormessage_test.go b/model/modelprocessor/errormessage_test.go index c03170c4..80845b91 100644 --- a/model/modelprocessor/errormessage_test.go +++ b/model/modelprocessor/errormessage_test.go @@ -27,83 +27,61 @@ import ( "github.com/elastic/apm-data/model/modelprocessor" ) -// Copy the error message to the event message if event message is empty. -func TestSetErrorMessage(t *testing.T) { - tests := []struct { - input *modelpb.Error - expected string - }{{ - input: &modelpb.Error{}, - expected: "", - }, { - input: &modelpb.Error{Log: &modelpb.ErrorLog{Message: "log_message"}}, - expected: "log_message", - }, { - input: &modelpb.Error{Exception: &modelpb.Exception{Message: "exception_message"}}, - expected: "exception_message", - }, { - input: &modelpb.Error{ - Log: &modelpb.ErrorLog{}, - Exception: &modelpb.Exception{Message: "exception_message"}, - }, - expected: "exception_message", - }, { - input: &modelpb.Error{ - Log: &modelpb.ErrorLog{Message: "log_message"}, - Exception: &modelpb.Exception{Message: "exception_message"}, - }, - expected: "log_message", - }} - for _, test := range tests { - batch := modelpb.Batch{{Error: test.input}} - processor := modelprocessor.SetErrorMessage{} - err := processor.ProcessBatch(context.Background(), &batch) - assert.NoError(t, err) - assert.Equal(t, test.expected, batch[0].Message) - assert.Equal(t, test.expected, batch[0].Error.Message) - } -} - -// If event message is non-empty, do not override with error message. func TestSetEventMessage(t *testing.T) { tests := []struct { - input *modelpb.APMEvent - expected string + desc string + input *modelpb.APMEvent + expectedMessage string + expectedLogMessage string + expectedExceptionMessage string }{{ + desc: "keep event message when no error", input: &modelpb.APMEvent{ + Error: &modelpb.Error{ + Log: &modelpb.ErrorLog{}, + Exception: &modelpb.Exception{}, + }, Message: "message", }, - expected: "message", - }, { - input: &modelpb.APMEvent{ - Error: &modelpb.Error{Log: &modelpb.ErrorLog{Message: "log_message"}}, - Message: "message", - }, - expected: "message", + expectedMessage: "message", + expectedLogMessage: "", + expectedExceptionMessage: "", }, { + desc: "keep event message when error", input: &modelpb.APMEvent{ - Error: &modelpb.Error{Exception: &modelpb.Exception{Message: "exception_message"}}, + Error: &modelpb.Error{ + Log: &modelpb.ErrorLog{Message: "log_message"}, + Exception: &modelpb.Exception{Message: "exception_message"}, + }, Message: "message", }, - expected: "message", + expectedMessage: "message", + expectedLogMessage: "log_message", + expectedExceptionMessage: "exception_message", }, { + desc: "propagate log error if event message empty", input: &modelpb.APMEvent{ Error: &modelpb.Error{ - Log: &modelpb.ErrorLog{}, - Exception: &modelpb.Exception{Message: "exception_message"}, + Log: &modelpb.ErrorLog{Message: "log_message"}, + Exception: &modelpb.Exception{}, }, - Message: "message", + Message: "", }, - expected: "message", + expectedMessage: "log_message", + expectedLogMessage: "log_message", + expectedExceptionMessage: "", }, { + desc: "propagate exception error if event message is empty", input: &modelpb.APMEvent{ Error: &modelpb.Error{ - Log: &modelpb.ErrorLog{Message: "log_message"}, + Log: &modelpb.ErrorLog{}, Exception: &modelpb.Exception{Message: "exception_message"}, }, - Message: "message", + Message: "", }, - expected: "message", + expectedMessage: "exception_message", + expectedLogMessage: "", + expectedExceptionMessage: "exception_message", }} for _, test := range tests { @@ -111,6 +89,8 @@ func TestSetEventMessage(t *testing.T) { processor := modelprocessor.SetErrorMessage{} err := processor.ProcessBatch(context.Background(), &batch) assert.NoError(t, err) - assert.Equal(t, test.expected, batch[0].Message) + assert.Equal(t, test.expectedMessage, batch[0].Message) + assert.Equal(t, test.expectedLogMessage, batch[0].Error.Log.Message) + assert.Equal(t, test.expectedExceptionMessage, batch[0].Error.Exception.Message) } } From 1b5ee6710adf1001b118eecd70b5b33db91518f5 Mon Sep 17 00:00:00 2001 From: Ruben van Staden Date: Thu, 29 Aug 2024 09:56:41 -0400 Subject: [PATCH 4/5] remove superfluous checks in unit test --- model/modelprocessor/errormessage_test.go | 26 ++++++----------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/model/modelprocessor/errormessage_test.go b/model/modelprocessor/errormessage_test.go index 80845b91..67d173bd 100644 --- a/model/modelprocessor/errormessage_test.go +++ b/model/modelprocessor/errormessage_test.go @@ -29,11 +29,9 @@ import ( func TestSetEventMessage(t *testing.T) { tests := []struct { - desc string - input *modelpb.APMEvent - expectedMessage string - expectedLogMessage string - expectedExceptionMessage string + desc string + input *modelpb.APMEvent + expectedMessage string }{{ desc: "keep event message when no error", input: &modelpb.APMEvent{ @@ -43,9 +41,7 @@ func TestSetEventMessage(t *testing.T) { }, Message: "message", }, - expectedMessage: "message", - expectedLogMessage: "", - expectedExceptionMessage: "", + expectedMessage: "message", }, { desc: "keep event message when error", input: &modelpb.APMEvent{ @@ -55,9 +51,7 @@ func TestSetEventMessage(t *testing.T) { }, Message: "message", }, - expectedMessage: "message", - expectedLogMessage: "log_message", - expectedExceptionMessage: "exception_message", + expectedMessage: "message", }, { desc: "propagate log error if event message empty", input: &modelpb.APMEvent{ @@ -67,9 +61,7 @@ func TestSetEventMessage(t *testing.T) { }, Message: "", }, - expectedMessage: "log_message", - expectedLogMessage: "log_message", - expectedExceptionMessage: "", + expectedMessage: "log_message", }, { desc: "propagate exception error if event message is empty", input: &modelpb.APMEvent{ @@ -79,9 +71,7 @@ func TestSetEventMessage(t *testing.T) { }, Message: "", }, - expectedMessage: "exception_message", - expectedLogMessage: "", - expectedExceptionMessage: "exception_message", + expectedMessage: "exception_message", }} for _, test := range tests { @@ -90,7 +80,5 @@ func TestSetEventMessage(t *testing.T) { err := processor.ProcessBatch(context.Background(), &batch) assert.NoError(t, err) assert.Equal(t, test.expectedMessage, batch[0].Message) - assert.Equal(t, test.expectedLogMessage, batch[0].Error.Log.Message) - assert.Equal(t, test.expectedExceptionMessage, batch[0].Error.Exception.Message) } } From 40a1fa787f2ba3d57967ef438528099e47d6643c Mon Sep 17 00:00:00 2001 From: Ruben van Staden Date: Thu, 29 Aug 2024 11:41:27 -0400 Subject: [PATCH 5/5] rename unit test --- model/modelprocessor/errormessage_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/modelprocessor/errormessage_test.go b/model/modelprocessor/errormessage_test.go index 67d173bd..8788ac4a 100644 --- a/model/modelprocessor/errormessage_test.go +++ b/model/modelprocessor/errormessage_test.go @@ -27,7 +27,7 @@ import ( "github.com/elastic/apm-data/model/modelprocessor" ) -func TestSetEventMessage(t *testing.T) { +func TestSetErrorMessage(t *testing.T) { tests := []struct { desc string input *modelpb.APMEvent