From 5afe6fb75211ebaf79ad8fd76287aa09742d2932 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 12 Jul 2021 18:49:30 +0800 Subject: [PATCH] Move more code from model to model processors Move more logic out of model transformation, into dedicated model processors: - modelprocessor.SetExcludeFromGrouping matches stack frames against a regular expression, setting the "exclude_from_grouping" field - modelprocessor.SetLibraryFrame matches stack frames against a regular expression, setting the "library_frame" field - modelprocessor.SetCulprit identifies the first source mapped, non-library, stack frame and uses it to set or update the error culprit The "library_frame" field is now only set if it is true. The UI uses this field but takes omission to mean the same thing as false (i.e. "falsy") The new model processors are only installed for the RUM intake APIs. With these changes, we no longer need to identify events as originating from the RUM endpoint, so we remove the model.Span.RUM and model.Error.RUM fields. --- beater/api/mux.go | 51 +++-- beater/beater.go | 5 - ...TestPublishIntegrationErrors.approved.json | 1 - docs/data/elasticsearch/generated/errors.json | 1 - model/error.go | 59 +----- model/error_test.go | 115 ----------- model/modeldecoder/rumv3/error_test.go | 2 - model/modeldecoder/rumv3/transaction_test.go | 4 - model/modeldecoder/v2/decoder.go | 2 +- model/modeldecoder/v2/error_test.go | 2 - model/modeldecoder/v2/span_test.go | 2 - model/modelprocessor/culprit.go | 68 +++++++ model/modelprocessor/culprit_test.go | 97 ++++++++++ model/modelprocessor/excludefromgrouping.go | 72 +++++++ .../excludefromgrouping_test.go | 113 +++++++++++ model/modelprocessor/libraryframe.go | 74 +++++++ model/modelprocessor/libraryframe_test.go | 121 ++++++++++++ model/span.go | 6 +- model/stacktrace.go | 45 +---- model/stacktrace_test.go | 183 +----------------- processor/stream/processor.go | 8 - .../testIntakeIntegrationErrors.approved.json | 1 - ...stIntakeIntegrationRumErrors.approved.json | 1 - sourcemap/processor.go | 2 +- .../TestNoMatchingSourcemap.approved.json | 1 - .../TestRUMErrorSourcemapping.approved.json | 7 - .../TestRUMSpanSourcemapping.approved.json | 2 - tests/system/error.approved.json | 1 - tests/system/test_integration.py | 6 +- transform/transform.go | 9 - 30 files changed, 605 insertions(+), 456 deletions(-) create mode 100644 model/modelprocessor/culprit.go create mode 100644 model/modelprocessor/culprit_test.go create mode 100644 model/modelprocessor/excludefromgrouping.go create mode 100644 model/modelprocessor/excludefromgrouping_test.go create mode 100644 model/modelprocessor/libraryframe.go create mode 100644 model/modelprocessor/libraryframe_test.go diff --git a/beater/api/mux.go b/beater/api/mux.go index da33d95acd1..e0e8d639e96 100644 --- a/beater/api/mux.go +++ b/beater/api/mux.go @@ -21,6 +21,9 @@ import ( "context" "net/http" "net/http/pprof" + "regexp" + + "github.com/pkg/errors" "github.com/elastic/beats/v7/libbeat/beat" "github.com/elastic/beats/v7/libbeat/logp" @@ -181,18 +184,37 @@ func (r *routeBuilder) rumIntakeHandler(newProcessor func(*config.Config) *strea requestMetadataFunc = rumRequestMetadata } return func() (request.Handler, error) { - batchProcessor := r.batchProcessor + var batchProcessors modelprocessor.Chained + if len(r.cfg.RumConfig.AllowServiceNames) > 0 { + batchProcessors = append(batchProcessors, allowedServiceNamesBatchProcessor(r.cfg.RumConfig.AllowServiceNames)) + } + // The order of these processors is important. Source mapping must happen before identifying library frames, or + // frames to exclude from error grouping; identifying library frames must happen before updating the error culprit. if r.sourcemapStore != nil { - batchProcessor = modelprocessor.Chained{ - sourcemap.BatchProcessor{ - Store: r.sourcemapStore, - Timeout: r.cfg.RumConfig.SourceMapping.Timeout, - }, - batchProcessor, + batchProcessors = append(batchProcessors, sourcemap.BatchProcessor{ + Store: r.sourcemapStore, + Timeout: r.cfg.RumConfig.SourceMapping.Timeout, + }) + } + if r.cfg.RumConfig.LibraryPattern != "" { + re, err := regexp.Compile(r.cfg.RumConfig.LibraryPattern) + if err != nil { + return nil, errors.Wrap(err, "invalid library pattern regex") + } + batchProcessors = append(batchProcessors, modelprocessor.SetLibraryFrame{Pattern: re}) + } + if r.cfg.RumConfig.ExcludeFromGrouping != "" { + re, err := regexp.Compile(r.cfg.RumConfig.ExcludeFromGrouping) + if err != nil { + return nil, errors.Wrap(err, "invalid exclude from grouping regex") } + batchProcessors = append(batchProcessors, modelprocessor.SetExcludeFromGrouping{Pattern: re}) } - batchProcessor = batchProcessorWithAllowedServiceNames(batchProcessor, r.cfg.RumConfig.AllowServiceNames) - h := intake.Handler(newProcessor(r.cfg), requestMetadataFunc, batchProcessor) + if r.sourcemapStore != nil { + batchProcessors = append(batchProcessors, modelprocessor.SetCulprit{}) + } + batchProcessors = append(batchProcessors, r.batchProcessor) // r.batchProcessor always goes last + h := intake.Handler(newProcessor(r.cfg), requestMetadataFunc, batchProcessors) return middleware.Wrap(h, rumMiddleware(r.cfg, r.authenticator, r.ratelimitStore, intake.MonitoringMap)...) } } @@ -295,16 +317,12 @@ func rootMiddleware(cfg *config.Config, authenticator *auth.Authenticator) []mid } // TODO(axw) move this into the auth package when introducing anonymous auth. -func batchProcessorWithAllowedServiceNames(p model.BatchProcessor, allowedServiceNames []string) model.BatchProcessor { - if len(allowedServiceNames) == 0 { - // All service names are allowed. - return p - } +func allowedServiceNamesBatchProcessor(allowedServiceNames []string) model.BatchProcessor { m := make(map[string]bool) for _, name := range allowedServiceNames { m[name] = true } - var restrictServiceName modelprocessor.MetadataProcessorFunc = func(ctx context.Context, meta *model.Metadata) error { + return modelprocessor.MetadataProcessorFunc(func(ctx context.Context, meta *model.Metadata) error { // Restrict to explicitly allowed service names. // The list of allowed service names is not considered secret, // so we do not use constant time comparison. @@ -312,8 +330,7 @@ func batchProcessorWithAllowedServiceNames(p model.BatchProcessor, allowedServic return &stream.InvalidInputError{Message: "service name is not allowed"} } return nil - } - return modelprocessor.Chained{restrictServiceName, p} + }) } func emptyRequestMetadata(c *request.Context) model.Metadata { diff --git a/beater/beater.go b/beater/beater.go index 8f820bc863e..21c0e9a00a5 100644 --- a/beater/beater.go +++ b/beater/beater.go @@ -22,7 +22,6 @@ import ( "net" "net/http" "os" - "regexp" "runtime" "strings" "sync" @@ -628,10 +627,6 @@ func runServerWithTracerServer(runServer RunServerFunc, tracerServer *tracerServ func newTransformConfig(beatInfo beat.Info, cfg *config.Config) *transform.Config { return &transform.Config{ DataStreams: cfg.DataStreams.Enabled, - RUM: transform.RUMConfig{ - LibraryPattern: regexp.MustCompile(cfg.RumConfig.LibraryPattern), - ExcludeFromGrouping: regexp.MustCompile(cfg.RumConfig.ExcludeFromGrouping), - }, } } diff --git a/beater/test_approved_es_documents/TestPublishIntegrationErrors.approved.json b/beater/test_approved_es_documents/TestPublishIntegrationErrors.approved.json index 71f27c23a13..c21d2efd380 100644 --- a/beater/test_approved_es_documents/TestPublishIntegrationErrors.approved.json +++ b/beater/test_approved_es_documents/TestPublishIntegrationErrors.approved.json @@ -170,7 +170,6 @@ "exclude_from_grouping": false, "filename": "/webpack/file/name.py", "function": "foo", - "library_frame": false, "line": { "column": 4, "context": "line3", diff --git a/docs/data/elasticsearch/generated/errors.json b/docs/data/elasticsearch/generated/errors.json index 66d7cc5f853..6761077cc97 100644 --- a/docs/data/elasticsearch/generated/errors.json +++ b/docs/data/elasticsearch/generated/errors.json @@ -219,7 +219,6 @@ "exclude_from_grouping": false, "filename": "/webpack/file/name.py", "function": "foo", - "library_frame": false, "line": { "column": 4, "context": "line3", diff --git a/model/error.go b/model/error.go index 2c59d0bbe57..c5e36622d01 100644 --- a/model/error.go +++ b/model/error.go @@ -73,10 +73,6 @@ type Error struct { TransactionSampled *bool TransactionType string - // RUM records whether or not this is a RUM error, - // and should have its stack frames sourcemapped. - RUM bool - Experimental interface{} } @@ -111,7 +107,7 @@ func (e *Error) appendBeatEvents(ctx context.Context, cfg *transform.Config, eve } fields := mapStr{ - "error": e.fields(ctx, cfg), + "error": e.fields(), "processor": errorProcessorEntry, } @@ -157,63 +153,24 @@ func (e *Error) appendBeatEvents(ctx context.Context, cfg *transform.Config, eve }) } -func (e *Error) fields(ctx context.Context, cfg *transform.Config) common.MapStr { +func (e *Error) fields() common.MapStr { var fields mapStr fields.maybeSetString("id", e.ID) fields.maybeSetMapStr("page", e.Page.Fields()) exceptionChain := flattenExceptionTree(e.Exception) - if exception := e.exceptionFields(ctx, cfg, exceptionChain); len(exception) > 0 { + if exception := e.exceptionFields(exceptionChain); len(exception) > 0 { fields.set("exception", exception) } - fields.maybeSetMapStr("log", e.logFields(ctx, cfg)) + fields.maybeSetMapStr("log", e.logFields()) - e.updateCulprit() fields.maybeSetString("culprit", e.Culprit) fields.maybeSetMapStr("custom", customFields(e.Custom)) fields.maybeSetString("grouping_key", e.calcGroupingKey(exceptionChain)) return common.MapStr(fields) } -// TODO(axw) introduce another processor which sets library_frame -// and exclude_from_grouping, only applied for RUM. Then we get rid -// of Error.RUM and Span.RUM. -func (e *Error) updateCulprit() { - if !e.RUM { - return - } - var fr *StacktraceFrame - if e.Log != nil { - fr = findSmappedNonLibraryFrame(e.Log.Stacktrace) - } - if fr == nil && e.Exception != nil { - fr = findSmappedNonLibraryFrame(e.Exception.Stacktrace) - } - if fr == nil { - return - } - var culprit string - if fr.Filename != "" { - culprit = fr.Filename - } else if fr.Classname != "" { - culprit = fr.Classname - } - if fr.Function != "" { - culprit += fmt.Sprintf(" in %v", fr.Function) - } - e.Culprit = culprit -} - -func findSmappedNonLibraryFrame(frames []*StacktraceFrame) *StacktraceFrame { - for _, fr := range frames { - if fr.SourcemapUpdated && !fr.IsLibraryFrame() { - return fr - } - } - return nil -} - -func (e *Error) exceptionFields(ctx context.Context, cfg *transform.Config, chain []Exception) []common.MapStr { +func (e *Error) exceptionFields(chain []Exception) []common.MapStr { var result []common.MapStr for _, exception := range chain { var ex mapStr @@ -242,7 +199,7 @@ func (e *Error) exceptionFields(ctx context.Context, cfg *transform.Config, chai if n := len(exception.Stacktrace); n > 0 { frames := make([]common.MapStr, n) for i, frame := range exception.Stacktrace { - frames[i] = frame.transform(cfg, e.RUM) + frames[i] = frame.transform() } ex.set("stacktrace", frames) } @@ -252,7 +209,7 @@ func (e *Error) exceptionFields(ctx context.Context, cfg *transform.Config, chai return result } -func (e *Error) logFields(ctx context.Context, cfg *transform.Config) common.MapStr { +func (e *Error) logFields() common.MapStr { if e.Log == nil { return nil } @@ -261,7 +218,7 @@ func (e *Error) logFields(ctx context.Context, cfg *transform.Config) common.Map log.maybeSetString("param_message", e.Log.ParamMessage) log.maybeSetString("logger_name", e.Log.LoggerName) log.maybeSetString("level", e.Log.Level) - if st := e.Log.Stacktrace.transform(ctx, cfg, e.RUM); len(st) > 0 { + if st := e.Log.Stacktrace.transform(); len(st) > 0 { log.set("stacktrace", st) } return common.MapStr(log) diff --git a/model/error_test.go b/model/error_test.go index 9806b9fc41a..d11a229627e 100644 --- a/model/error_test.go +++ b/model/error_test.go @@ -214,7 +214,6 @@ func TestEventFields(t *testing.T) { Exception: &exception, Log: &log, TransactionID: trID, - RUM: true, // Service name and version are required for sourcemapping. Metadata: Metadata{ @@ -357,7 +356,6 @@ func TestEvents(t *testing.T) { HTTP: &Http{Request: &Req{Referer: referer}}, URL: &URL{Original: url}, Custom: custom, - RUM: true, }, Output: common.MapStr{ @@ -410,119 +408,6 @@ func TestEvents(t *testing.T) { } } -func TestCulprit(t *testing.T) { - c := "foo" - fct := "fct" - truthy := true - st := Stacktrace{ - &StacktraceFrame{Filename: "a", Function: fct}, - } - stUpdate := Stacktrace{ - &StacktraceFrame{Filename: "a", Function: fct}, - &StacktraceFrame{Filename: "a", LibraryFrame: &truthy, SourcemapUpdated: true}, - &StacktraceFrame{Filename: "f", Function: fct, SourcemapUpdated: true}, - &StacktraceFrame{Filename: "bar", Function: fct, SourcemapUpdated: true}, - } - tests := []struct { - event Error - culprit string - msg string - }{ - { - event: Error{Culprit: c, RUM: false}, - culprit: "foo", - msg: "Not a RUM event", - }, - { - event: Error{Culprit: c, RUM: true}, - culprit: "foo", - msg: "No Stacktrace Frame given.", - }, - { - event: Error{Culprit: c, RUM: true, Log: &Log{Stacktrace: st}}, - culprit: "foo", - msg: "Log.StacktraceFrame has no updated frame", - }, - { - event: Error{ - Culprit: c, - RUM: true, - Log: &Log{ - Stacktrace: Stacktrace{ - &StacktraceFrame{ - Filename: "f", - Classname: "xyz", - SourcemapUpdated: true, - }, - }, - }, - }, - culprit: "f", - msg: "Adapt culprit to first valid Log.StacktraceFrame filename information.", - }, - { - event: Error{ - Culprit: c, - RUM: true, - Log: &Log{ - Stacktrace: Stacktrace{ - &StacktraceFrame{ - Classname: "xyz", - SourcemapUpdated: true, - }, - }, - }, - }, - culprit: "xyz", - msg: "Adapt culprit Log.StacktraceFrame classname information.", - }, - { - event: Error{ - Culprit: c, - RUM: true, - Exception: &Exception{Stacktrace: stUpdate}, - }, - culprit: "f in fct", - msg: "Adapt culprit to first valid Exception.StacktraceFrame information.", - }, - { - event: Error{ - Culprit: c, - RUM: true, - Log: &Log{Stacktrace: st}, - Exception: &Exception{Stacktrace: stUpdate}, - }, - culprit: "f in fct", - msg: "Log and Exception StacktraceFrame given, only one changes culprit.", - }, - { - event: Error{ - Culprit: c, - RUM: true, - Log: &Log{ - Stacktrace: Stacktrace{ - &StacktraceFrame{ - Filename: "a", - Function: fct, - SourcemapUpdated: true, - }, - }, - }, - Exception: &Exception{Stacktrace: stUpdate}, - }, - culprit: "a in fct", - msg: "Log Stacktrace is prioritized over Exception StacktraceFrame", - }, - } - for idx, test := range tests { - t.Run(fmt.Sprint(idx), func(t *testing.T) { - test.event.updateCulprit() - assert.Equal(t, test.culprit, test.event.Culprit, - fmt.Sprintf("(%v) %s: expected <%v>, received <%v>", idx, test.msg, test.culprit, test.event.Culprit)) - }) - } -} - func TestErrorTransformPage(t *testing.T) { id := "123" urlExample := "http://example.com/path" diff --git a/model/modeldecoder/rumv3/error_test.go b/model/modeldecoder/rumv3/error_test.go index 6d4e0e2921d..43842e87d53 100644 --- a/model/modeldecoder/rumv3/error_test.go +++ b/model/modeldecoder/rumv3/error_test.go @@ -121,8 +121,6 @@ func TestDecodeMapToErrorModel(t *testing.T) { "Experimental", // URL parts are derived from url (separately tested) "URL", "Page.URL", - // RUM is set in stream processor - "RUM", // exception.parent is only set after calling `flattenExceptionTree` (not part of decoding) "Exception.Parent", // stacktrace original and sourcemap values are set when sourcemapping is applied diff --git a/model/modeldecoder/rumv3/transaction_test.go b/model/modeldecoder/rumv3/transaction_test.go index 8b34186c45d..1426eba05e9 100644 --- a/model/modeldecoder/rumv3/transaction_test.go +++ b/model/modeldecoder/rumv3/transaction_test.go @@ -186,8 +186,6 @@ func TestDecodeMapToTransactionModel(t *testing.T) { // URL parts are derived from page.url (separately tested) "URL", "Page.URL", // HTTP.Request.Referrer is derived from page.referer (separately tested) - // RUM is set in stream processor - "RUM", } { if strings.HasPrefix(key, s) { return true @@ -243,8 +241,6 @@ func TestDecodeMapToTransactionModel(t *testing.T) { "Stacktrace.Sourcemap", // ExcludeFromGrouping is set when processing the event "Stacktrace.ExcludeFromGrouping", - // RUM is set in stream processor - "RUM", // Transaction related information is set within the DecodeNestedTransaction method // it is separatly tested in TestDecodeNestedTransaction "TransactionID", "TraceID", "ParentID", diff --git a/model/modeldecoder/v2/decoder.go b/model/modeldecoder/v2/decoder.go index f8b9b50c198..a295ffaaa2a 100644 --- a/model/modeldecoder/v2/decoder.go +++ b/model/modeldecoder/v2/decoder.go @@ -966,7 +966,7 @@ func mapToStracktraceModel(from []stacktraceFrame, out model.Stacktrace) { } if eventFrame.LibraryFrame.IsSet() { val := eventFrame.LibraryFrame.Val - fr.LibraryFrame = &val + fr.LibraryFrame = val } if eventFrame.LineNumber.IsSet() { val := eventFrame.LineNumber.Val diff --git a/model/modeldecoder/v2/error_test.go b/model/modeldecoder/v2/error_test.go index a7a7598be55..90b49834daf 100644 --- a/model/modeldecoder/v2/error_test.go +++ b/model/modeldecoder/v2/error_test.go @@ -143,8 +143,6 @@ func TestDecodeMapToErrorModel(t *testing.T) { "Metadata", // URL parts are derived from url (separately tested) "Page.URL", - // RUM is set in stream processor - "RUM", // exception.parent is only set after calling `flattenExceptionTree` (not part of decoding) "Exception.Parent", // stacktrace original and sourcemap values are set when sourcemapping is applied diff --git a/model/modeldecoder/v2/span_test.go b/model/modeldecoder/v2/span_test.go index 3b150904184..2c10f86a2da 100644 --- a/model/modeldecoder/v2/span_test.go +++ b/model/modeldecoder/v2/span_test.go @@ -97,8 +97,6 @@ func TestDecodeMapToSpanModel(t *testing.T) { for _, s := range []string{ // experimental is tested in test 'experimental' "Experimental", - // RUM is set in stream processor - "RUM", // RepresentativeCount is tested further down in test 'sample-rate' "RepresentativeCount"} { if key == s { diff --git a/model/modelprocessor/culprit.go b/model/modelprocessor/culprit.go new file mode 100644 index 00000000000..96479cb3319 --- /dev/null +++ b/model/modelprocessor/culprit.go @@ -0,0 +1,68 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor + +import ( + "context" + + "github.com/elastic/apm-server/model" +) + +// SetCulprit is a model.BatchProcessor that sets or updates the culprit for RUM +// errors, after source mapping and identifying library frames. +type SetCulprit struct{} + +// ProcessBatch sets or updates the culprit for RUM errors. +func (s SetCulprit) ProcessBatch(ctx context.Context, b *model.Batch) error { + for _, event := range *b { + if event.Error != nil { + s.processError(ctx, event.Error) + } + } + return nil +} + +func (s SetCulprit) processError(ctx context.Context, event *model.Error) { + var culpritFrame *model.StacktraceFrame + if event.Log != nil { + culpritFrame = s.findSourceMappedNonLibraryFrame(event.Log.Stacktrace) + } + if culpritFrame == nil && event.Exception != nil { + culpritFrame = s.findSourceMappedNonLibraryFrame(event.Exception.Stacktrace) + } + if culpritFrame == nil { + return + } + culprit := culpritFrame.Filename + if culprit == "" { + culprit = culpritFrame.Classname + } + if culpritFrame.Function != "" { + culprit += " in " + culpritFrame.Function + } + event.Culprit = culprit +} + +func (s SetCulprit) findSourceMappedNonLibraryFrame(frames []*model.StacktraceFrame) *model.StacktraceFrame { + for _, frame := range frames { + if frame.SourcemapUpdated && !frame.LibraryFrame { + return frame + } + } + return nil +} diff --git a/model/modelprocessor/culprit_test.go b/model/modelprocessor/culprit_test.go new file mode 100644 index 00000000000..a514a427b97 --- /dev/null +++ b/model/modelprocessor/culprit_test.go @@ -0,0 +1,97 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/elastic/apm-server/model" + "github.com/elastic/apm-server/model/modelprocessor" +) + +func TestSetCulprit(t *testing.T) { + tests := []struct { + input model.Error + culprit string + }{{ + input: model.Error{}, + culprit: "", + }, { + input: model.Error{Culprit: "already_set"}, + culprit: "already_set", + }, { + input: model.Error{ + Culprit: "already_set", + Log: &model.Log{ + Stacktrace: model.Stacktrace{{SourcemapUpdated: false, Filename: "foo.go"}}, + }, + }, + culprit: "already_set", + }, { + input: model.Error{ + Culprit: "already_set", + Log: &model.Log{ + Stacktrace: model.Stacktrace{{SourcemapUpdated: true, LibraryFrame: true, Filename: "foo.go"}}, + }, + }, + culprit: "already_set", + }, { + input: model.Error{ + Culprit: "already_set", + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {SourcemapUpdated: true, LibraryFrame: true, Filename: "foo.go"}, + {SourcemapUpdated: true, LibraryFrame: false, Filename: "foo2.go"}, + }, + }, + }, + culprit: "foo2.go", + }, { + input: model.Error{ + Culprit: "already_set", + Log: &model.Log{ + Stacktrace: model.Stacktrace{{SourcemapUpdated: true, LibraryFrame: true, Filename: "foo.go"}}, + }, + Exception: &model.Exception{ + Stacktrace: model.Stacktrace{{SourcemapUpdated: true, LibraryFrame: false, Filename: "foo2.go"}}, + }, + }, + culprit: "foo2.go", + }, { + input: model.Error{ + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {SourcemapUpdated: true, Classname: "AbstractFactoryManagerBean", Function: "toString"}, + }, + }, + }, + culprit: "AbstractFactoryManagerBean in toString", + }} + + for _, test := range tests { + batch := model.Batch{{Error: &test.input}} + processor := modelprocessor.SetCulprit{} + err := processor.ProcessBatch(context.Background(), &batch) + assert.NoError(t, err) + assert.Equal(t, test.culprit, batch[0].Error.Culprit) + } + +} diff --git a/model/modelprocessor/excludefromgrouping.go b/model/modelprocessor/excludefromgrouping.go new file mode 100644 index 00000000000..e8e29ac8c0b --- /dev/null +++ b/model/modelprocessor/excludefromgrouping.go @@ -0,0 +1,72 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor + +import ( + "context" + "regexp" + + "github.com/elastic/apm-server/model" +) + +// SetExcludeFromGrouping is a model.BatchProcessor that identifies stack frames +// to exclude from error grouping for RUM, using a configurable regular expression. +type SetExcludeFromGrouping struct { + Pattern *regexp.Regexp +} + +// ProcessBatch processes the stack traces of spans and errors in b, updating +// the exclude_from_grouping for stack frames based on whether they have a filename +// matching the regular expression. +func (s SetExcludeFromGrouping) ProcessBatch(ctx context.Context, b *model.Batch) error { + for _, event := range *b { + switch { + case event.Span != nil: + s.processSpan(ctx, event.Span) + case event.Error != nil: + s.processError(ctx, event.Error) + } + } + return nil +} + +func (s SetExcludeFromGrouping) processSpan(ctx context.Context, event *model.Span) { + s.processStacktraceFrames(ctx, event.Stacktrace...) +} + +func (s SetExcludeFromGrouping) processError(ctx context.Context, event *model.Error) { + if event.Log != nil { + s.processStacktraceFrames(ctx, event.Log.Stacktrace...) + } + if event.Exception != nil { + s.processException(ctx, event.Exception) + } +} + +func (s SetExcludeFromGrouping) processException(ctx context.Context, exception *model.Exception) { + s.processStacktraceFrames(ctx, exception.Stacktrace...) + for _, cause := range exception.Cause { + s.processException(ctx, &cause) + } +} + +func (s SetExcludeFromGrouping) processStacktraceFrames(ctx context.Context, frames ...*model.StacktraceFrame) { + for _, frame := range frames { + frame.ExcludeFromGrouping = frame.Filename != "" && s.Pattern.MatchString(frame.Filename) + } +} diff --git a/model/modelprocessor/excludefromgrouping_test.go b/model/modelprocessor/excludefromgrouping_test.go new file mode 100644 index 00000000000..2399301ee2c --- /dev/null +++ b/model/modelprocessor/excludefromgrouping_test.go @@ -0,0 +1,113 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor_test + +import ( + "context" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/elastic/apm-server/model" + "github.com/elastic/apm-server/model/modelprocessor" +) + +func TestSetExcludeFromGrouping(t *testing.T) { + processor := modelprocessor.SetExcludeFromGrouping{ + Pattern: regexp.MustCompile("foo"), + } + + tests := []struct { + input, output model.Batch + }{{ + input: model.Batch{{Error: &model.Error{}}, {Transaction: &model.Transaction{}}}, + output: model.Batch{{Error: &model.Error{}}, {Transaction: &model.Transaction{}}}, + }, { + input: model.Batch{{ + Span: &model.Span{ + Stacktrace: model.Stacktrace{ + {Filename: "foo.go"}, + {Filename: "bar.go"}, + {}, + }, + }, + }}, + output: model.Batch{{ + Span: &model.Span{ + Stacktrace: model.Stacktrace{ + {ExcludeFromGrouping: true, Filename: "foo.go"}, + {Filename: "bar.go"}, + {}, + }, + }, + }}, + }, { + input: model.Batch{{ + Error: &model.Error{ + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {Filename: "foo.go"}, + }, + }, + }, + }, { + Error: &model.Error{ + Exception: &model.Exception{ + Stacktrace: model.Stacktrace{ + {Filename: "foo.go"}, + }, + Cause: []model.Exception{{ + Stacktrace: model.Stacktrace{ + {Filename: "foo.go"}, + }, + }}, + }, + }, + }}, + output: model.Batch{{ + Error: &model.Error{ + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {ExcludeFromGrouping: true, Filename: "foo.go"}, + }, + }, + }, + }, { + Error: &model.Error{ + Exception: &model.Exception{ + Stacktrace: model.Stacktrace{ + {ExcludeFromGrouping: true, Filename: "foo.go"}, + }, + Cause: []model.Exception{{ + Stacktrace: model.Stacktrace{ + {ExcludeFromGrouping: true, Filename: "foo.go"}, + }, + }}, + }, + }, + }}, + }} + + for _, test := range tests { + err := processor.ProcessBatch(context.Background(), &test.input) + assert.NoError(t, err) + assert.Equal(t, test.output, test.input) + } + +} diff --git a/model/modelprocessor/libraryframe.go b/model/modelprocessor/libraryframe.go new file mode 100644 index 00000000000..fa51727ff0a --- /dev/null +++ b/model/modelprocessor/libraryframe.go @@ -0,0 +1,74 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor + +import ( + "context" + "regexp" + + "github.com/elastic/apm-server/model" +) + +// SetLibraryFrame is a model.BatchProcessor that identifies stack frames +// from library code for RUM, using a configurable regular expression. +type SetLibraryFrame struct { + Pattern *regexp.Regexp +} + +// ProcessBatch processes the stack traces of spans and errors in b, updating +// the library frame flag for stack frames based on whether they have a filename +// matching the regular expression. +func (s SetLibraryFrame) ProcessBatch(ctx context.Context, b *model.Batch) error { + for _, event := range *b { + switch { + case event.Span != nil: + s.processSpan(ctx, event.Span) + case event.Error != nil: + s.processError(ctx, event.Error) + } + } + return nil +} + +func (s SetLibraryFrame) processSpan(ctx context.Context, event *model.Span) { + s.processStacktraceFrames(ctx, event.Stacktrace...) +} + +func (s SetLibraryFrame) processError(ctx context.Context, event *model.Error) { + if event.Log != nil { + s.processStacktraceFrames(ctx, event.Log.Stacktrace...) + } + if event.Exception != nil { + s.processException(ctx, event.Exception) + } +} + +func (s SetLibraryFrame) processException(ctx context.Context, exception *model.Exception) { + s.processStacktraceFrames(ctx, exception.Stacktrace...) + for _, cause := range exception.Cause { + s.processException(ctx, &cause) + } +} + +func (s SetLibraryFrame) processStacktraceFrames(ctx context.Context, frames ...*model.StacktraceFrame) { + for _, frame := range frames { + frame.Original.LibraryFrame = frame.LibraryFrame + frame.LibraryFrame = frame.Filename != "" && s.Pattern.MatchString(frame.Filename) || + frame.AbsPath != "" && s.Pattern.MatchString(frame.AbsPath) + } +} diff --git a/model/modelprocessor/libraryframe_test.go b/model/modelprocessor/libraryframe_test.go new file mode 100644 index 00000000000..dcf3d8f6098 --- /dev/null +++ b/model/modelprocessor/libraryframe_test.go @@ -0,0 +1,121 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package modelprocessor_test + +import ( + "context" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/elastic/apm-server/model" + "github.com/elastic/apm-server/model/modelprocessor" +) + +func TestSetLibraryFrames(t *testing.T) { + processor := modelprocessor.SetLibraryFrame{ + Pattern: regexp.MustCompile("foo"), + } + + tests := []struct { + input, output model.Batch + }{{ + input: model.Batch{{Error: &model.Error{}}, {Transaction: &model.Transaction{}}}, + output: model.Batch{{Error: &model.Error{}}, {Transaction: &model.Transaction{}}}, + }, { + input: model.Batch{{ + Span: &model.Span{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go"}, + {LibraryFrame: false, AbsPath: "foobar.go"}, + {LibraryFrame: true, Filename: "bar.go"}, + {LibraryFrame: true}, + }, + }, + }}, + output: model.Batch{{ + Span: &model.Span{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go", Original: model.Original{LibraryFrame: true}}, + {LibraryFrame: true, AbsPath: "foobar.go", Original: model.Original{LibraryFrame: false}}, + {LibraryFrame: false, Filename: "bar.go", Original: model.Original{LibraryFrame: true}}, + {LibraryFrame: false, Original: model.Original{LibraryFrame: true}}, + }, + }, + }}, + }, { + input: model.Batch{{ + Error: &model.Error{ + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go"}, + {LibraryFrame: false, AbsPath: "foobar.go"}, + {LibraryFrame: true, Filename: "bar.go"}, + {LibraryFrame: true}, + }, + }, + }, + }, { + Error: &model.Error{ + Exception: &model.Exception{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go"}, + }, + Cause: []model.Exception{{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go"}, + }, + }}, + }, + }, + }}, + output: model.Batch{{ + Error: &model.Error{ + Log: &model.Log{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go", Original: model.Original{LibraryFrame: true}}, + {LibraryFrame: true, AbsPath: "foobar.go", Original: model.Original{LibraryFrame: false}}, + {LibraryFrame: false, Filename: "bar.go", Original: model.Original{LibraryFrame: true}}, + {LibraryFrame: false, Original: model.Original{LibraryFrame: true}}, + }, + }, + }, + }, { + Error: &model.Error{ + Exception: &model.Exception{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go", Original: model.Original{LibraryFrame: true}}, + }, + Cause: []model.Exception{{ + Stacktrace: model.Stacktrace{ + {LibraryFrame: true, Filename: "foo.go", Original: model.Original{LibraryFrame: true}}, + }, + }}, + }, + }, + }}, + }} + + for _, test := range tests { + err := processor.ProcessBatch(context.Background(), &test.input) + assert.NoError(t, err) + assert.Equal(t, test.output, test.input) + } + +} diff --git a/model/span.go b/model/span.go index 1395f5fed0d..d85db35e079 100644 --- a/model/span.go +++ b/model/span.go @@ -71,10 +71,6 @@ type Span struct { Destination *Destination DestinationService *DestinationService - // RUM records whether or not this is a RUM span, - // and should have its stack frames sourcemapped. - RUM bool - Experimental interface{} // RepresentativeCount holds the approximate number of spans that @@ -269,7 +265,7 @@ func (e *Span) fields(ctx context.Context, cfg *transform.Config) common.MapStr // TODO(axw) we should be using a merged service object, combining // the stream metadata and event-specific service info. - if st := e.Stacktrace.transform(ctx, cfg, e.RUM); len(st) > 0 { + if st := e.Stacktrace.transform(); len(st) > 0 { fields.set("stacktrace", st) } return common.MapStr(fields) diff --git a/model/stacktrace.go b/model/stacktrace.go index 9ef562d3f50..422e944ff05 100644 --- a/model/stacktrace.go +++ b/model/stacktrace.go @@ -18,12 +18,7 @@ package model import ( - "context" - "regexp" - "github.com/elastic/beats/v7/libbeat/common" - - "github.com/elastic/apm-server/transform" ) type Stacktrace []*StacktraceFrame @@ -37,7 +32,7 @@ type StacktraceFrame struct { ContextLine string Module string Function string - LibraryFrame *bool + LibraryFrame bool Vars common.MapStr PreContext []string PostContext []string @@ -56,21 +51,21 @@ type Original struct { Lineno *int Colno *int Function string - LibraryFrame *bool + LibraryFrame bool } -func (st Stacktrace) transform(ctx context.Context, cfg *transform.Config, rum bool) []common.MapStr { +func (st Stacktrace) transform() []common.MapStr { if len(st) == 0 { return nil } frames := make([]common.MapStr, len(st)) for i, frame := range st { - frames[i] = frame.transform(cfg, rum) + frames[i] = frame.transform() } return frames } -func (s *StacktraceFrame) transform(cfg *transform.Config, rum bool) common.MapStr { +func (s *StacktraceFrame) transform() common.MapStr { var m mapStr m.maybeSetString("filename", s.Filename) m.maybeSetString("classname", s.Classname) @@ -79,15 +74,8 @@ func (s *StacktraceFrame) transform(cfg *transform.Config, rum bool) common.MapS m.maybeSetString("function", s.Function) m.maybeSetMapStr("vars", s.Vars) - if rum && cfg.RUM.LibraryPattern != nil { - s.setLibraryFrame(cfg.RUM.LibraryPattern) - } - if s.LibraryFrame != nil { - m.set("library_frame", *s.LibraryFrame) - } - - if rum && cfg.RUM.ExcludeFromGrouping != nil { - s.setExcludeFromGrouping(cfg.RUM.ExcludeFromGrouping) + if s.LibraryFrame { + m.set("library_frame", s.LibraryFrame) } m.set("exclude_from_grouping", s.ExcludeFromGrouping) @@ -114,7 +102,9 @@ func (s *StacktraceFrame) transform(cfg *transform.Config, rum bool) common.MapS m.maybeSetMapStr("sourcemap", common.MapStr(sm)) var orig mapStr - orig.maybeSetBool("library_frame", s.Original.LibraryFrame) + if s.Original.LibraryFrame { + orig.set("library_frame", s.Original.LibraryFrame) + } if s.SourcemapUpdated { orig.maybeSetString("filename", s.Original.Filename) orig.maybeSetString("classname", s.Original.Classname) @@ -127,18 +117,3 @@ func (s *StacktraceFrame) transform(cfg *transform.Config, rum bool) common.MapS return common.MapStr(m) } - -func (s *StacktraceFrame) IsLibraryFrame() bool { - return s.LibraryFrame != nil && *s.LibraryFrame -} - -func (s *StacktraceFrame) setExcludeFromGrouping(pattern *regexp.Regexp) { - s.ExcludeFromGrouping = s.Filename != "" && pattern.MatchString(s.Filename) -} - -func (s *StacktraceFrame) setLibraryFrame(pattern *regexp.Regexp) { - s.Original.LibraryFrame = s.LibraryFrame - libraryFrame := (s.Filename != "" && pattern.MatchString(s.Filename)) || - (s.AbsPath != "" && pattern.MatchString(s.AbsPath)) - s.LibraryFrame = &libraryFrame -} diff --git a/model/stacktrace_test.go b/model/stacktrace_test.go index 1a64901758c..ae5588ac817 100644 --- a/model/stacktrace_test.go +++ b/model/stacktrace_test.go @@ -18,16 +18,12 @@ package model import ( - "context" "fmt" - "regexp" "testing" "github.com/stretchr/testify/assert" "github.com/elastic/beats/v7/libbeat/common" - - "github.com/elastic/apm-server/transform" ) func TestStacktraceTransform(t *testing.T) { @@ -76,7 +72,7 @@ func TestStacktraceTransform(t *testing.T) { Classname: originalClassname, Module: originalModule, AbsPath: originalAbsPath, - LibraryFrame: newBool(true), + LibraryFrame: true, Vars: vars, }}, Output: []common.MapStr{{ @@ -151,182 +147,7 @@ func TestStacktraceTransform(t *testing.T) { } for idx, test := range tests { - output := test.Stacktrace.transform(context.Background(), &transform.Config{}, false) + output := test.Stacktrace.transform() assert.Equal(t, test.Output, output, fmt.Sprintf("Failed at idx %v; %s", idx, test.Msg)) } } - -func TestStacktraceFrameIsLibraryFrame(t *testing.T) { - assert.False(t, (&StacktraceFrame{}).IsLibraryFrame()) - assert.False(t, (&StacktraceFrame{LibraryFrame: new(bool)}).IsLibraryFrame()) - libFrame := true - assert.True(t, (&StacktraceFrame{LibraryFrame: &libFrame}).IsLibraryFrame()) -} - -func TestStacktraceFrameExcludeFromGroupingKey(t *testing.T) { - tests := []struct { - fr StacktraceFrame - pattern string - exclude bool - }{ - { - fr: StacktraceFrame{}, - pattern: "", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: "/webpack"}, - pattern: "", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: "/webpack"}, - pattern: "/webpack/tmp", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: ""}, - pattern: "^/webpack", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: "/webpack"}, - pattern: "^/webpack", - exclude: true, - }, - { - fr: StacktraceFrame{Filename: "/webpack/test/e2e/general-usecase/app.e2e-bundle.js"}, - pattern: "^/webpack", - exclude: true, - }, - { - fr: StacktraceFrame{Filename: "/filename"}, - pattern: "^/webpack", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: "/filename/a"}, - pattern: "^/webpack", - exclude: false, - }, - { - fr: StacktraceFrame{Filename: "webpack"}, - pattern: "^/webpack", - exclude: false, - }, - } - - for idx, test := range tests { - var excludePattern *regexp.Regexp - if test.pattern != "" { - excludePattern = regexp.MustCompile(test.pattern) - } - - out := test.fr.transform(&transform.Config{ - RUM: transform.RUMConfig{ExcludeFromGrouping: excludePattern}, - }, true) - exclude := out["exclude_from_grouping"] - message := fmt.Sprintf( - "(%v): Pattern: %v, Filename: %v, expected to be excluded: %v", - idx, test.pattern, test.fr.Filename, test.exclude, - ) - assert.Equal(t, test.exclude, exclude, message) - } -} - -func TestStacktraceFrameLibraryFrame(t *testing.T) { - path := "/~/a/b" - tests := []struct { - fr StacktraceFrame - libraryPattern *regexp.Regexp - libraryFrame *bool - origLibraryFrame *bool - msg string - }{ - {fr: StacktraceFrame{}, - libraryFrame: nil, - origLibraryFrame: nil, - msg: "Empty StacktraceFrame, empty config"}, - {fr: StacktraceFrame{AbsPath: path}, - libraryFrame: nil, - origLibraryFrame: nil, - msg: "No pattern"}, - {fr: StacktraceFrame{AbsPath: path}, - libraryPattern: regexp.MustCompile(""), - libraryFrame: newBool(true), - origLibraryFrame: nil, - msg: "Empty pattern"}, - {fr: StacktraceFrame{LibraryFrame: newBool(false)}, - libraryPattern: regexp.MustCompile("~"), - libraryFrame: newBool(false), - origLibraryFrame: newBool(false), - msg: "Empty StacktraceFrame"}, - {fr: StacktraceFrame{AbsPath: path, LibraryFrame: newBool(true)}, - libraryPattern: regexp.MustCompile("^~/"), - libraryFrame: newBool(false), - origLibraryFrame: newBool(true), - msg: "AbsPath given, no Match"}, - {fr: StacktraceFrame{Filename: "myFile.js", LibraryFrame: newBool(true)}, - libraryPattern: regexp.MustCompile("^~/"), - libraryFrame: newBool(false), - origLibraryFrame: newBool(true), - msg: "Filename given, no Match"}, - {fr: StacktraceFrame{AbsPath: path, Filename: "myFile.js"}, - libraryPattern: regexp.MustCompile("^~/"), - libraryFrame: newBool(false), - origLibraryFrame: nil, - msg: "AbsPath and Filename given, no Match"}, - {fr: StacktraceFrame{Filename: "/tmp"}, - libraryPattern: regexp.MustCompile("/tmp"), - libraryFrame: newBool(true), - origLibraryFrame: nil, - msg: "Filename matching"}, - {fr: StacktraceFrame{AbsPath: path, LibraryFrame: newBool(false)}, - libraryPattern: regexp.MustCompile("~/"), - libraryFrame: newBool(true), - origLibraryFrame: newBool(false), - msg: "AbsPath matching"}, - {fr: StacktraceFrame{AbsPath: path, Filename: "/a/b/c"}, - libraryPattern: regexp.MustCompile("~/"), - libraryFrame: newBool(true), - origLibraryFrame: nil, - msg: "AbsPath matching, Filename not matching"}, - {fr: StacktraceFrame{AbsPath: path, Filename: "/a/b/c"}, - libraryPattern: regexp.MustCompile("/a/b/c"), - libraryFrame: newBool(true), - origLibraryFrame: nil, - msg: "AbsPath not matching, Filename matching"}, - {fr: StacktraceFrame{AbsPath: path, Filename: "~/a/b/c"}, - libraryPattern: regexp.MustCompile("~/"), - libraryFrame: newBool(true), - origLibraryFrame: nil, - msg: "AbsPath and Filename matching"}, - } - - for _, test := range tests { - cfg := transform.Config{ - RUM: transform.RUMConfig{ - LibraryPattern: test.libraryPattern, - }, - } - out := test.fr.transform(&cfg, true)["library_frame"] - libFrame := test.fr.LibraryFrame - origLibFrame := test.fr.Original.LibraryFrame - if test.libraryFrame == nil { - assert.Nil(t, out, test.msg) - assert.Nil(t, libFrame, test.msg) - } else { - assert.Equal(t, *test.libraryFrame, out, test.msg) - assert.Equal(t, *test.libraryFrame, *libFrame, test.msg) - } - if test.origLibraryFrame == nil { - assert.Nil(t, origLibFrame, test.msg) - } else { - assert.Equal(t, *test.origLibraryFrame, *origLibFrame, test.msg) - } - } -} - -func newBool(v bool) *bool { - return &v -} diff --git a/processor/stream/processor.go b/processor/stream/processor.go index e2b06a25e8d..d5012c9b0cb 100644 --- a/processor/stream/processor.go +++ b/processor/stream/processor.go @@ -58,7 +58,6 @@ type Processor struct { MaxEventSize int streamReaderPool sync.Pool decodeMetadata decodeMetadataFunc - isRUM bool } func BackendProcessor(cfg *config.Config) *Processor { @@ -66,7 +65,6 @@ func BackendProcessor(cfg *config.Config) *Processor { Mconfig: modeldecoder.Config{Experimental: cfg.Mode == config.ModeExperimental}, MaxEventSize: cfg.MaxEventSize, decodeMetadata: v2.DecodeNestedMetadata, - isRUM: false, } } @@ -75,7 +73,6 @@ func RUMV2Processor(cfg *config.Config) *Processor { Mconfig: modeldecoder.Config{Experimental: cfg.Mode == config.ModeExperimental}, MaxEventSize: cfg.MaxEventSize, decodeMetadata: v2.DecodeNestedMetadata, - isRUM: true, } } @@ -84,7 +81,6 @@ func RUMV3Processor(cfg *config.Config) *Processor { Mconfig: modeldecoder.Config{Experimental: cfg.Mode == config.ModeExperimental}, MaxEventSize: cfg.MaxEventSize, decodeMetadata: rumv3.DecodeNestedMetadata, - isRUM: true, } } @@ -176,7 +172,6 @@ func (p *Processor) readBatch( if handleDecodeErr(err, reader, result) { continue } - event.RUM = p.isRUM *batch = append(*batch, model.APMEvent{Error: &event}) n++ case metricsetEventType: @@ -193,7 +188,6 @@ func (p *Processor) readBatch( if handleDecodeErr(err, reader, result) { continue } - event.RUM = p.isRUM *batch = append(*batch, model.APMEvent{Span: &event}) n++ case transactionEventType: @@ -210,7 +204,6 @@ func (p *Processor) readBatch( if handleDecodeErr(err, reader, result) { continue } - event.RUM = p.isRUM *batch = append(*batch, model.APMEvent{Error: &event}) n++ case rumv3MetricsetEventType: @@ -232,7 +225,6 @@ func (p *Processor) readBatch( *batch = append(*batch, model.APMEvent{Metricset: ms}) } for _, span := range event.Spans { - span.RUM = true *batch = append(*batch, model.APMEvent{Span: span}) } n += 1 + len(event.Metricsets) + len(event.Spans) diff --git a/processor/stream/test_approved_es_documents/testIntakeIntegrationErrors.approved.json b/processor/stream/test_approved_es_documents/testIntakeIntegrationErrors.approved.json index 19570c7acc9..3338630746f 100644 --- a/processor/stream/test_approved_es_documents/testIntakeIntegrationErrors.approved.json +++ b/processor/stream/test_approved_es_documents/testIntakeIntegrationErrors.approved.json @@ -169,7 +169,6 @@ "exclude_from_grouping": false, "filename": "/webpack/file/name.py", "function": "foo", - "library_frame": false, "line": { "column": 4, "context": "line3", diff --git a/processor/stream/test_approved_es_documents/testIntakeIntegrationRumErrors.approved.json b/processor/stream/test_approved_es_documents/testIntakeIntegrationRumErrors.approved.json index 502f1b0ba89..3cfbd7d8493 100644 --- a/processor/stream/test_approved_es_documents/testIntakeIntegrationRumErrors.approved.json +++ b/processor/stream/test_approved_es_documents/testIntakeIntegrationRumErrors.approved.json @@ -33,7 +33,6 @@ "exclude_from_grouping": false, "filename": "~/test/e2e/general-usecase/bundle.js.map", "function": "invokeTask", - "library_frame": false, "line": { "column": 181, "number": 1 diff --git a/sourcemap/processor.go b/sourcemap/processor.go index cbc7b1eda43..8e26a8ceb6a 100644 --- a/sourcemap/processor.go +++ b/sourcemap/processor.go @@ -85,7 +85,7 @@ func (p BatchProcessor) processError(ctx context.Context, event *model.Error) { func (p BatchProcessor) processException(ctx context.Context, service *model.Service, exception *model.Exception) { p.processStacktraceFrames(ctx, service, exception.Stacktrace...) for _, cause := range exception.Cause { - p.processStacktraceFrames(ctx, service, cause.Stacktrace...) + p.processException(ctx, service, &cause) } } diff --git a/systemtest/approvals/TestNoMatchingSourcemap.approved.json b/systemtest/approvals/TestNoMatchingSourcemap.approved.json index 9232321713c..45045a25209 100644 --- a/systemtest/approvals/TestNoMatchingSourcemap.approved.json +++ b/systemtest/approvals/TestNoMatchingSourcemap.approved.json @@ -50,7 +50,6 @@ "exclude_from_grouping": false, "filename": "test/e2e/general-usecase/bundle.js.map", "function": "\u003canonymous\u003e", - "library_frame": false, "line": { "column": 18, "number": 1 diff --git a/systemtest/approvals/TestRUMErrorSourcemapping.approved.json b/systemtest/approvals/TestRUMErrorSourcemapping.approved.json index 59b399291cb..4effabf07a1 100644 --- a/systemtest/approvals/TestRUMErrorSourcemapping.approved.json +++ b/systemtest/approvals/TestRUMErrorSourcemapping.approved.json @@ -38,7 +38,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "__webpack_require__", - "library_frame": false, "line": { "column": 0, "context": " \tfunction __webpack_require__(moduleId) {", @@ -77,7 +76,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003cunknown\u003e", - "library_frame": false, "line": { "column": 0, "context": " \t__webpack_require__.c = installedModules;", @@ -88,7 +86,6 @@ "colno": 181, "filename": "~/test/e2e/general-usecase/bundle.js.map", "function": "invokeTask", - "library_frame": false, "lineno": 1 }, "sourcemap": { @@ -115,7 +112,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003cunknown\u003e", - "library_frame": false, "line": { "column": 0, "context": " \tfunction __webpack_require__(moduleId) {", @@ -153,7 +149,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "moduleId", - "library_frame": false, "line": { "column": 0, "context": " \treturn __webpack_require__(0);", @@ -191,7 +186,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003canonymous\u003e", - "library_frame": false, "line": { "column": 0, "context": " \t\tif(installedModules[moduleId])", @@ -238,7 +232,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003canonymous\u003e", - "library_frame": false, "line": { "column": 0, "context": " \tfunction __webpack_require__(moduleId) {", diff --git a/systemtest/approvals/TestRUMSpanSourcemapping.approved.json b/systemtest/approvals/TestRUMSpanSourcemapping.approved.json index 9cd801f4cac..029a9217227 100644 --- a/systemtest/approvals/TestRUMSpanSourcemapping.approved.json +++ b/systemtest/approvals/TestRUMSpanSourcemapping.approved.json @@ -65,7 +65,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003cunknown\u003e", - "library_frame": false, "line": { "column": 0, "context": " \tfunction __webpack_require__(moduleId) {", @@ -102,7 +101,6 @@ "exclude_from_grouping": false, "filename": "webpack:///webpack/bootstrap 6002740481c9666b0d38", "function": "\u003canonymous\u003e", - "library_frame": false, "line": { "column": 0, "context": " \tfunction __webpack_require__(moduleId) {", diff --git a/tests/system/error.approved.json b/tests/system/error.approved.json index 66d7cc5f853..6761077cc97 100644 --- a/tests/system/error.approved.json +++ b/tests/system/error.approved.json @@ -219,7 +219,6 @@ "exclude_from_grouping": false, "filename": "/webpack/file/name.py", "function": "foo", - "library_frame": false, "line": { "column": 4, "context": "line3", diff --git a/tests/system/test_integration.py b/tests/system/test_integration.py index 1c97254766d..e423585e8ee 100644 --- a/tests/system/test_integration.py +++ b/tests/system/test_integration.py @@ -98,14 +98,14 @@ def test_backend_error(self): self.backend_intake_url, 'error', 4) - self.check_library_frames({"true": 1, "false": 1, "empty": 2}, index_error) + self.check_library_frames({"true": 1, "false": 0, "empty": 3}, index_error) def test_rum_error(self): self.load_docs_with_template(self.get_error_payload_path(), self.intake_url, 'error', 1) - self.check_library_frames({"true": 5, "false": 1, "empty": 0}, index_error) + self.check_library_frames({"true": 5, "false": 0, "empty": 1}, index_error) def test_backend_transaction(self): # for backend events library_frame information should not be changed, @@ -121,7 +121,7 @@ def test_rum_transaction(self): self.intake_url, 'transaction', 2) - self.check_library_frames({"true": 1, "false": 1, "empty": 0}, index_span) + self.check_library_frames({"true": 1, "false": 0, "empty": 1}, index_span) def test_enrich_backend_event(self): self.load_docs_with_template(self.get_backend_transaction_payload_path(), diff --git a/transform/transform.go b/transform/transform.go index 58ee99523e8..b4323fc98f7 100644 --- a/transform/transform.go +++ b/transform/transform.go @@ -19,7 +19,6 @@ package transform import ( "context" - "regexp" "github.com/elastic/beats/v7/libbeat/beat" ) @@ -35,12 +34,4 @@ type Config struct { // DataStreams records whether or not data streams are enabled. // If true, then data_stream fields should be added to all events. DataStreams bool - - RUM RUMConfig -} - -// RUMConfig holds RUM-related transformation configuration. -type RUMConfig struct { - LibraryPattern *regexp.Regexp - ExcludeFromGrouping *regexp.Regexp }