From ab1d0624ab347b4d3833bd85f866b63e3319fe2a Mon Sep 17 00:00:00 2001 From: Daniel Cohen Date: Wed, 1 Feb 2023 16:51:17 -0500 Subject: [PATCH 01/11] Add SendQueryParameters option to selectively enable/disable sending query parameters upstream --- v3/integrations/nrpgx5/nrpgx5.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/v3/integrations/nrpgx5/nrpgx5.go b/v3/integrations/nrpgx5/nrpgx5.go index da3f41265..5c8758cfe 100644 --- a/v3/integrations/nrpgx5/nrpgx5.go +++ b/v3/integrations/nrpgx5/nrpgx5.go @@ -68,8 +68,9 @@ func init() { type ( Tracer struct { - BaseSegment newrelic.DatastoreSegment - ParseQuery func(segment *newrelic.DatastoreSegment, query string) + BaseSegment newrelic.DatastoreSegment + ParseQuery func(segment *newrelic.DatastoreSegment, query string) + SendQueryParameters bool } nrPgxSegmentType string @@ -83,7 +84,8 @@ const ( func NewTracer() *Tracer { return &Tracer{ - ParseQuery: sqlparse.ParseQuery, + ParseQuery: sqlparse.ParseQuery, + SendQueryParameters: true, } } @@ -109,7 +111,9 @@ func (t *Tracer) TraceQueryStart(ctx context.Context, conn *pgx.Conn, data pgx.T segment := t.BaseSegment segment.StartTime = newrelic.FromContext(ctx).StartSegmentNow() segment.ParameterizedQuery = data.SQL - segment.QueryParameters = t.getQueryParameters(data.Args) + if t.SendQueryParameters { + segment.QueryParameters = t.getQueryParameters(data.Args) + } // fill Operation and Collection t.ParseQuery(&segment, data.SQL) From a4cc8744d95e357f398b1897e032d3ebf5fdd3e9 Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Wed, 1 Mar 2023 13:23:14 -0500 Subject: [PATCH 02/11] Error Group Callback This adds functionality to the agent that enables users to define a custom callback function that will be applied to all noticed errors in the agent. It must return a string value which will be used to categorize this error into an error group in New Relic One.` --- v3/go.mod | 8 + v3/newrelic/application.go | 1 - v3/newrelic/attributes.go | 2 + v3/newrelic/attributes_from_internal.go | 10 +- v3/newrelic/config.go | 17 +++ v3/newrelic/config_options.go | 8 + v3/newrelic/environment_test.go | 6 +- v3/newrelic/error_events.go | 8 +- v3/newrelic/error_group.go | 130 ++++++++++++++++ v3/newrelic/errors_from_internal.go | 42 ++++++ v3/newrelic/internal_test.go | 187 ++++++++++++++++++++++++ v3/newrelic/internal_txn.go | 37 +++-- v3/newrelic/stacktrace.go | 16 +- v3/newrelic/stacktrace_test.go | 4 +- v3/newrelic/tracing.go | 1 + 15 files changed, 445 insertions(+), 32 deletions(-) create mode 100644 v3/newrelic/error_group.go diff --git a/v3/go.mod b/v3/go.mod index 4eaf02fb4..c8c214856 100644 --- a/v3/go.mod +++ b/v3/go.mod @@ -6,3 +6,11 @@ require ( github.com/golang/protobuf v1.5.2 google.golang.org/grpc v1.49.0 ) + +require ( + golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect + golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect + golang.org/x/text v0.3.3 // indirect + google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect + google.golang.org/protobuf v1.27.1 // indirect +) diff --git a/v3/newrelic/application.go b/v3/newrelic/application.go index 6cc8cd446..3c82300b4 100644 --- a/v3/newrelic/application.go +++ b/v3/newrelic/application.go @@ -114,7 +114,6 @@ func (app *Application) RecordLog(logEvent LogData) { // as needed in the background (and will continue attempting to connect // if it wasn't immediately successful, all while allowing your application // to proceed with its primary function). -// func (app *Application) WaitForConnection(timeout time.Duration) error { if nil == app { return nil diff --git a/v3/newrelic/attributes.go b/v3/newrelic/attributes.go index ca30d8f3c..040e4df0f 100644 --- a/v3/newrelic/attributes.go +++ b/v3/newrelic/attributes.go @@ -52,6 +52,8 @@ const ( AttributeCodeFilepath = "code.filepath" // AttributeCodeLineno contains the Code Level Metrics source file line number name. AttributeCodeLineno = "code.lineno" + // AttributeErrorGroupName contains the error group name set by the user defined callback function. + AttributeErrorGroupName = "error.group.name" ) // Attributes destined for Errors and Transaction Traces: diff --git a/v3/newrelic/attributes_from_internal.go b/v3/newrelic/attributes_from_internal.go index 694f51f47..83a9f98df 100644 --- a/v3/newrelic/attributes_from_internal.go +++ b/v3/newrelic/attributes_from_internal.go @@ -457,7 +457,7 @@ func writeAttributeValueJSON(w *jsonFieldsWriter, key string, val interface{}) { } } -func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet) { +func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, additionalAttributes ...map[string]string) { if a == nil { buf.WriteString("{}") return @@ -473,6 +473,14 @@ func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet) { } } } + + // Add additional agent attributes to json + for _, additionalAttribute := range additionalAttributes { + for id, val := range additionalAttribute { + w.stringField(id, val) + } + } + buf.WriteByte('}') } diff --git a/v3/newrelic/config.go b/v3/newrelic/config.go index 1b68a4c82..d80afb19e 100644 --- a/v3/newrelic/config.go +++ b/v3/newrelic/config.go @@ -113,6 +113,23 @@ type Config struct { // as errors, and then re-panic them. By default, this is // set to false. RecordPanics bool + // ErrorGroupCallback is a user defined callback function that takes an error as an input + // and returns a string that will be applied to an error to put it in an error group. + // + // If no error group is identified for a given error, this function should return an empty string. + // If an ErrorGroupCallbeck is defined, it will be executed against every error the go agent notices that + // is not ignored. + // + // example function: + // + // func ErrorGroupCallback(errorInfo newrelic.ErrorInfo) string { + // if errorInfo.IsWeb && errorInfo.Class == "403" { + // return "client misconfigured example-group" + // } + // + // return "" + // } + ErrorGroupCallback `json:"-"` } // TransactionTracer controls the capture of transaction traces. diff --git a/v3/newrelic/config_options.go b/v3/newrelic/config_options.go index aea7d4026..dd395d03b 100644 --- a/v3/newrelic/config_options.go +++ b/v3/newrelic/config_options.go @@ -199,6 +199,14 @@ func ConfigCodeLevelMetricsPathPrefixes(prefix ...string) ConfigOption { } } +// ConfigModuleDependencyMetricsIgnoredPrefixes sets the ErrorFingerprintingCallback function +// to a function defined by the user. +func ConfigErrorFingerprintingCallback(callbackFunction ErrorGroupCallback) ConfigOption { + return func(cfg *Config) { + cfg.ErrorCollector.ErrorGroupCallback = callbackFunction + } +} + // ConfigAppLogForwardingEnabled enables or disables the collection // of logs from a user's application by the agent // Defaults: enabled=false diff --git a/v3/newrelic/environment_test.go b/v3/newrelic/environment_test.go index bc3c88d7e..d19fb1996 100644 --- a/v3/newrelic/environment_test.go +++ b/v3/newrelic/environment_test.go @@ -73,9 +73,9 @@ func TestModuleDependency(t *testing.T) { // of modules to at least check that the various options work. expectedModules := make(map[string]*debug.Module) mockedModuleList := []*debug.Module{ - &debug.Module{Path: "example/path/to/module", Version: "v1.2.3"}, - &debug.Module{Path: "github.com/another/module", Version: "v0.1.2"}, - &debug.Module{Path: "some/development/module", Version: "(develop)"}, + {Path: "example/path/to/module", Version: "v1.2.3"}, + {Path: "github.com/another/module", Version: "v0.1.2"}, + {Path: "some/development/module", Version: "(develop)"}, } for _, module := range mockedModuleList { expectedModules[module.Path] = module diff --git a/v3/newrelic/error_events.go b/v3/newrelic/error_events.go index 90419747c..46e95b609 100644 --- a/v3/newrelic/error_events.go +++ b/v3/newrelic/error_events.go @@ -39,7 +39,13 @@ func (e *errorEvent) WriteJSON(buf *bytes.Buffer) { buf.WriteByte(',') userAttributesJSON(e.Attrs, buf, destError, e.errorData.ExtraAttributes) buf.WriteByte(',') - agentAttributesJSON(e.Attrs, buf, destError) + + if e.ErrorGroup != "" { + agentAttributesJSON(e.Attrs, buf, destError, map[string]string{AttributeErrorGroupName: e.ErrorGroup}) + } else { + agentAttributesJSON(e.Attrs, buf, destError) + } + buf.WriteByte(']') } diff --git a/v3/newrelic/error_group.go b/v3/newrelic/error_group.go new file mode 100644 index 000000000..2651f4316 --- /dev/null +++ b/v3/newrelic/error_group.go @@ -0,0 +1,130 @@ +package newrelic + +import "time" + +const ( + // The error class for panics + PanicErrorClass = panicErrorKlass +) + +// ErrorInfo contains info for user defined callbacks that are relevant to an error. +// All fields are either safe to access coppies of internal agent data, or protected from direct +// access with methods and can not manipulate or distort any agent data. +type ErrorInfo struct { + errAttributes map[string]interface{} + txnAttributes *attributes + stackTrace stackTrace + isWebTransaction bool + + // TransactionName is the formatted name of a transaction that is equivilent to how it appears in + // the New Relic UI. For example, user defined transactions will be named `OtherTransaction/Go/yourTxnName`. + TransactionName string + + // Error contains the raw error object that the agent collected. + // + // Not all errors collected by the system are collected as + // error objects, like web/http errors and panics. + // In these cases, Error will be nil, but details will be captured in + // the Message and Class fields. + Error error + + // Time Occured is the time.Time when the error was noticed by the go agent + TimeOccured time.Time + + // Message will always be populated by a string message describing an error + Message string + + // Class is a string containing the New Relic error class. + // + // If an error implements errorClasser, its value will be derived from that. + // Otherwise, it will be derived from the way the error was + // collected by the agent. For http errors, this will be the + // error number. Panics will be the constant value `newrelic.PanicErrorClass`. + // If no errorClass was defined, this will be reflect.TypeOf() the root + // error object, which is commonly `*errors.errorString`. + Class string + + // Expected is true if the error was expected by the go agent + Expected bool +} + +// GetTransactionUserAttribute safely looks up a user attribute by string key from the parent transaction +// of an error. This function will return the attribute vaue as an interface{}, and a bool indicating whether the +// key was found in the attribute map. If the key was not found, then the return will be (nil, false). +func (e *ErrorInfo) GetTransactionUserAttribute(attribute string) (interface{}, bool) { + a, b := e.txnAttributes.user[attribute] + if b { + return a.value, b + } + + return nil, b +} + +// GetErrorAttribute safely looks up an error attribute by string key. The value of the attribute will be returned +// as an interface{}, and a bool indicating whether the key was found in the attribute map. If no matching key was +// found, the return will be (nil, false). +func (e *ErrorInfo) GetErrorAttribute(attribute string) (interface{}, bool) { + a, b := e.errAttributes[attribute] + return a, b +} + +// GetStackTraceFrames returns a slice of StacktraceFrame objects containing up to 100 lines of stack trace +// data gathered from the Go runtime. Calling this function may be expensive since it allocates and +// populates a new slice with stack trace data, and should be called only when needed. +func (e *ErrorInfo) GetStackTraceFrames() []StacktraceFrame { + return e.stackTrace.frames() +} + +// GetRequestURI returns the URI of the http request made during the parent transaction of this error. If no web request occured, +// this will return an empty string. +func (e *ErrorInfo) GetRequestURI() string { + if !e.isWebTransaction { + return "" + } + + val, ok := e.txnAttributes.Agent[AttributeRequestURI] + if !ok { + return "" + } + + return val.stringVal +} + +// GetRequestMethod will return the HTTP method used to make a web request if one occured during the parent transaction +// of this error. If no web request occured, then an empty string will be returned. +func (e *ErrorInfo) GetRequestMethod() string { + if !e.isWebTransaction { + return "" + } + + val, ok := e.txnAttributes.Agent[AttributeRequestMethod] + if !ok { + return "" + } + + return val.stringVal +} + +// GetHttpResponseCode will return the HTTP response code that resulted from the web request made in the parent transaction of +// this error. If no web request occured, then an empty string will be returned. +func (e *ErrorInfo) GetHttpResponseCode() string { + if !e.isWebTransaction { + return "" + } + + val, ok := e.txnAttributes.Agent[AttributeResponseCode] + if !ok { + return "" + } + + return val.stringVal +} + +// ErrorGroupCallback is a user defined callback function that takes an error as an input +// and returns a string that will be applied to an error to put it in an error group. +// +// If no error group is identified for a given error, this function should return an empty string. +// +// If an ErrorGroupCallbeck is defined, it will be executed against every error the go agent notices that +// is not ignored. +type ErrorGroupCallback func(ErrorInfo) string diff --git a/v3/newrelic/errors_from_internal.go b/v3/newrelic/errors_from_internal.go index 9b174f3f0..231974103 100644 --- a/v3/newrelic/errors_from_internal.go +++ b/v3/newrelic/errors_from_internal.go @@ -57,7 +57,9 @@ func txnErrorFromResponseCode(now time.Time, code int) errorData { type errorData struct { When time.Time Stack stackTrace + RawError error ExtraAttributes map[string]interface{} + ErrorGroup string Msg string Klass string SpanID string @@ -145,6 +147,8 @@ func mergeTxnErrors(errors *harvestErrors, errs txnErrors, txnEvent txnEvent) { if len(*errors) == cap(*errors) { return } + + e.applyErrorGroup(&txnEvent) *errors = append(*errors, &tracedError{ txnEvent: txnEvent, errorData: *e, @@ -178,3 +182,41 @@ func (errors harvestErrors) MergeIntoHarvest(h *harvest) {} func (errors harvestErrors) EndpointMethod() string { return cmdErrorData } + +// applyErrorGroup applies the error group callback function to an errorData object. It will either consume the txn object +// or the txnEvent in that order. If both are nil, nothing will happen. +func (errData *errorData) applyErrorGroup(txnEvent *txnEvent) { + if txnEvent == nil || txnEvent.errGroupCallback == nil { + return + } + + errorInfo := ErrorInfo{ + txnAttributes: txnEvent.Attrs, + TransactionName: txnEvent.FinalName, + errAttributes: errData.ExtraAttributes, + stackTrace: errData.Stack, + Error: errData.RawError, + TimeOccured: errData.When, + Message: errData.Msg, + Class: errData.Klass, + Expected: errData.Expect, + } + + // If a user defined an error group callback function, execute it to generate the error group string. + errGroup := txnEvent.errGroupCallback(errorInfo) + + if errGroup != "" { + errData.ErrorGroup = errGroup + } +} + +func (errData *errorData) scrubErrorForHighSecurity(txn *txn) { + if txn.Config.HighSecurity { + errData.Msg = highSecurityErrorMsg + } + + if !txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled() { + errData.Msg = securityPolicyErrorMsg + errData.RawError = nil + } +} diff --git a/v3/newrelic/internal_test.go b/v3/newrelic/internal_test.go index 8ce695d5d..197022701 100644 --- a/v3/newrelic/internal_test.go +++ b/v3/newrelic/internal_test.go @@ -5,6 +5,7 @@ package newrelic import ( "encoding/json" + "errors" "math" "net/http" "net/http/httptest" @@ -491,6 +492,88 @@ func TestSetName(t *testing.T) { app.ExpectMetrics(t, backgroundMetrics) } +type advancedError struct { + error +} + +func (e *advancedError) Error() string { + return e.error.Error() +} + +func (e *advancedError) ErrorClass() string { + return "test class" +} + +func (e *advancedError) ErrorAttributes() map[string]interface{} { + return map[string]interface{}{ + "testKey": "test val", + } +} + +func TestErrorWithCallback(t *testing.T) { + errorGroupFunc := func(e ErrorInfo) string { + if e.Error == nil { + t.Error("expected ErrorInfo.Error not be nil") + } + if e.Expected { + t.Error("error should not be expected") + } + + val, ok := e.GetErrorAttribute("testKey") + if !ok || val != "test val" { + t.Error("error should successfully look up user provided attribute: \"testKey\":\"test val\"") + } + + val, ok = e.GetTransactionUserAttribute("txnAttribute") + if !ok || val != "test txn attr" { + t.Error("error should successfully look up user provided attribute: \"testKey\":\"test txn attr\"") + } + + stackTrace := e.GetStackTraceFrames() + if len(stackTrace) == 0 { + t.Error("expected error stack trace to not be empty") + } + + if e.isWebTransaction { + t.Error("expected error to not be part of a web txn") + } + + AssertStringEqual(t, "ErrorInfo.TransactionName", `OtherTransaction/Go/hello`, e.TransactionName) + AssertStringEqual(t, "ErrorInfo.Message", "this is a test error", e.Message) + AssertStringEqual(t, "ErrorInfo.Class", "test class", e.Class) + + return "testGroup" + } + + app := testApp( + nil, + func(cfg *Config) { + cfg.DistributedTracer.Enabled = false + enableRecordPanics(cfg) + cfg.ErrorCollector.ErrorGroupCallback = errorGroupFunc + }, + t, + ) + + txn := app.StartTransaction("hello") + txn.AddAttribute("txnAttribute", "test txn attr") + txn.NoticeError(&advancedError{errors.New("this is a test error")}) + txn.End() + + app.ExpectErrors(t, []internal.WantError{{ + TxnName: "OtherTransaction/Go/hello", + Msg: "this is a test error", + Klass: "test class", + }}) + app.ExpectErrorEvents(t, []internal.WantEvent{{ + Intrinsics: map[string]interface{}{ + "error.class": "test class", + "error.message": "this is a test error", + "transactionName": "OtherTransaction/Go/hello", + }, + }}) +} + func deferEndPanic(txn *Transaction, panicMe interface{}) (r interface{}) { defer func() { r = recover() @@ -548,6 +631,51 @@ func TestPanicError(t *testing.T) { app.ExpectMetrics(t, backgroundErrorMetrics) } +func TestPanicErrorWithCallback(t *testing.T) { + errorGroupFunc := func(e ErrorInfo) string { + if e.Error != nil { + t.Errorf("expected ErrorInfo.Error to be nil, but got %v", e.Error) + } + AssertStringEqual(t, "ErrorInfo.TransactionName", `OtherTransaction/Go/hello`, e.TransactionName) + AssertStringEqual(t, "ErrorInfo.Message", "my msg", e.Message) + AssertStringEqual(t, "ErrorInfo.Class", PanicErrorClass, e.Class) + return "testGroup" + } + + app := testApp( + nil, + func(cfg *Config) { + cfg.DistributedTracer.Enabled = false + enableRecordPanics(cfg) + cfg.ErrorCollector.ErrorGroupCallback = errorGroupFunc + }, + t, + ) + + txn := app.StartTransaction("hello") + + e := myError{} + r := deferEndPanic(txn, e) + if r != e { + t.Error("panic not propagated", r) + } + + app.ExpectErrors(t, []internal.WantError{{ + TxnName: "OtherTransaction/Go/hello", + Msg: "my msg", + Klass: panicErrorKlass, + }}) + app.ExpectErrorEvents(t, []internal.WantEvent{{ + Intrinsics: map[string]interface{}{ + "error.class": panicErrorKlass, + "error.message": "my msg", + "transactionName": "OtherTransaction/Go/hello", + }, + }}) + app.ExpectMetrics(t, backgroundErrorMetrics) + +} + func TestPanicString(t *testing.T) { app := testApp(nil, func(cfg *Config) { enableRecordPanics(cfg) @@ -656,6 +784,65 @@ func TestResponseCodeError(t *testing.T) { app.ExpectMetrics(t, webErrorMetrics) } +func AssertStringEqual(t *testing.T, field string, expect string, actual string) { + if expect != actual { + t.Errorf("incorrect value for %s; expected: %s got: %s", field, expect, actual) + } +} + +func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { + errorGroupFunc := func(e ErrorInfo) string { + if e.Error != nil { + t.Errorf("expected ErrorInfo.Error to be nil, but got %v", e.Error) + } + AssertStringEqual(t, "ErrorInfo.TransactionName", `WebTransaction/Go/hello`, e.TransactionName) + AssertStringEqual(t, "ErrorInfo.Message", "Bad Request", e.Message) + AssertStringEqual(t, "ErrorInfo.Class", "400", e.Class) + return "testGroup" + } + + app := testApp( + nil, + func(cfg *Config) { + cfg.DistributedTracer.Enabled = false + cfg.ErrorCollector.ErrorGroupCallback = errorGroupFunc + }, + t, + ) + w := newCompatibleResponseRecorder() + txn := app.StartTransaction("hello") + rw := txn.SetWebResponse(w) + txn.SetWebRequestHTTP(helloRequest) + + rw.WriteHeader(http.StatusBadRequest) // 400 + rw.WriteHeader(http.StatusUnauthorized) // 401 + + txn.End() + + if http.StatusBadRequest != w.Code { + t.Error(w.Code) + } + + app.ExpectErrors(t, []internal.WantError{{ + TxnName: "WebTransaction/Go/hello", + Msg: "Bad Request", + Klass: "400", + }}) + app.ExpectErrorEvents(t, []internal.WantEvent{{ + Intrinsics: map[string]interface{}{ + "error.class": "400", + "error.message": "Bad Request", + "transactionName": "WebTransaction/Go/hello", + }, + AgentAttributes: mergeAttributes(helloRequestAttributes, map[string]interface{}{ + "httpResponseCode": "400", + "http.statusCode": "400", + AttributeErrorGroupName: "testGroup", + }), + }}) + app.ExpectMetrics(t, webErrorMetrics) +} + func TestResponseCode404Filtered(t *testing.T) { app := testApp(nil, ConfigDistributedTracerEnabled(false), t) w := newCompatibleResponseRecorder() diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go index 6c33bf75f..0378b22f8 100644 --- a/v3/newrelic/internal_txn.go +++ b/v3/newrelic/internal_txn.go @@ -258,6 +258,11 @@ func (thd *thread) StoreLog(log *logEvent) { txn.Lock() defer txn.Unlock() + // might want to refactor to return errAlreadyEnded + if txn.finished { + return + } + if txn.logs == nil { txn.logs = make(logEventHeap, 0, internal.MaxLogEvents) } @@ -289,7 +294,6 @@ func (txn *txn) shouldSaveTrace() bool { } func (txn *txn) MergeIntoHarvest(h *harvest) { - var priority priority if txn.BetterCAT.Enabled { priority = txn.BetterCAT.Priority @@ -314,19 +318,24 @@ func (txn *txn) MergeIntoHarvest(h *harvest) { h.TxnEvents.AddTxnEvent(alloc, priority) } + // pass the callback func down the chain + txn.txnEvent.errGroupCallback = txn.Config.ErrorCollector.ErrorGroupCallback + if txn.Reply.CollectErrors { mergeTxnErrors(&h.ErrorTraces, txn.Errors, txn.txnEvent) } if txn.Config.ErrorCollector.CaptureEvents { for _, e := range txn.Errors { + e.applyErrorGroup(&txn.txnEvent) errEvent := &errorEvent{ errorData: *e, txnEvent: txn.txnEvent, } - // Since the stack trace is not used in error events, remove the reference + // Since the stack trace and raw error object is not used in error events, remove the reference // to minimize memory. errEvent.Stack = nil + errEvent.RawError = nil h.ErrorEvents.Add(errEvent, priority) } } @@ -367,7 +376,7 @@ func headersJustWritten(thd *thread, code int, hdr http.Header) { e := txnErrorFromResponseCode(time.Now(), code) e.Stack = getStackTrace() expect := txn.appRun.responseCodeIsExpected(code) - thd.noticeErrorInternal(e, expect) + thd.noticeErrorInternal(e, nil, expect) } } @@ -426,7 +435,7 @@ func (thd *thread) End(recovered interface{}) error { if nil != recovered { e := txnErrorFromPanic(time.Now(), recovered) e.Stack = getStackTrace() - thd.noticeErrorInternal(e, false) + thd.noticeErrorInternal(e, nil, false) log.Println(string(debug.Stack())) } @@ -560,7 +569,7 @@ const ( securityPolicyErrorMsg = "message removed by security policy" ) -func (thd *thread) noticeErrorInternal(err errorData, expect bool) error { +func (thd *thread) noticeErrorInternal(errData errorData, err error, expect bool) error { txn := thd.txn if !txn.Config.ErrorCollector.Enabled { return errorsDisabled @@ -576,19 +585,15 @@ func (thd *thread) noticeErrorInternal(err errorData, expect bool) error { txn.Errors = newTxnErrors(maxTxnErrors) } - if txn.Config.HighSecurity { - err.Msg = highSecurityErrorMsg - } - - if !txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled() { - err.Msg = securityPolicyErrorMsg - } + errData.RawError = err + errData.scrubErrorForHighSecurity(txn) if txn.shouldCollectSpanEvents() { - err.SpanID = txn.CurrentSpanIdentifier(thd.thread) - addErrorAttrs(thd, err) + errData.SpanID = txn.CurrentSpanIdentifier(thd.thread) + addErrorAttrs(thd, errData) } - txn.Errors.Add(err) + + txn.Errors.Add(errData) txn.txnData.txnEvent.HasError = true //mark transaction as having an error return nil } @@ -730,7 +735,7 @@ func (thd *thread) NoticeError(input error, expect bool) error { data.ExtraAttributes = nil } - return thd.noticeErrorInternal(data, expect) + return thd.noticeErrorInternal(data, input, expect) } func (txn *txn) SetName(name string) error { diff --git a/v3/newrelic/stacktrace.go b/v3/newrelic/stacktrace.go index 2891acd67..6358f8ce4 100644 --- a/v3/newrelic/stacktrace.go +++ b/v3/newrelic/stacktrace.go @@ -21,13 +21,13 @@ func getStackTrace() stackTrace { return callers[:written] } -type stacktraceFrame struct { +type StacktraceFrame struct { Name string File string Line int64 } -func (f stacktraceFrame) formattedName() string { +func (f StacktraceFrame) formattedName() string { if strings.HasPrefix(f.Name, "go.") { // This indicates an anonymous struct. eg. // "go.(*struct { github.com/newrelic/go-agent.threadWithExtras }).NoticeError" @@ -36,7 +36,7 @@ func (f stacktraceFrame) formattedName() string { return path.Base(f.Name) } -func (f stacktraceFrame) isAgent() bool { +func (f StacktraceFrame) isAgent() bool { // Note this is not a contains conditional rather than a prefix // conditional to handle anonymous functions like: // "go.(*struct { github.com/newrelic/go-agent.threadWithExtras }).NoticeError" @@ -44,7 +44,7 @@ func (f stacktraceFrame) isAgent() bool { strings.Contains(f.Name, "github.com/newrelic/go-agent/v3/newrelic.") } -func (f stacktraceFrame) WriteJSON(buf *bytes.Buffer) { +func (f StacktraceFrame) WriteJSON(buf *bytes.Buffer) { buf.WriteByte('{') w := jsonFieldsWriter{buf: buf} if f.Name != "" { @@ -59,7 +59,7 @@ func (f stacktraceFrame) WriteJSON(buf *bytes.Buffer) { buf.WriteByte('}') } -func writeFrames(buf *bytes.Buffer, frames []stacktraceFrame) { +func writeFrames(buf *bytes.Buffer, frames []StacktraceFrame) { // Remove top agent frames. for len(frames) > 0 && frames[0].isAgent() { frames = frames[1:] @@ -80,17 +80,17 @@ func writeFrames(buf *bytes.Buffer, frames []stacktraceFrame) { buf.WriteByte(']') } -func (st stackTrace) frames() []stacktraceFrame { +func (st stackTrace) frames() []StacktraceFrame { if len(st) == 0 { return nil } frames := runtime.CallersFrames(st) // CallersFrames is only available in Go 1.7+ - fs := make([]stacktraceFrame, 0, maxStackTraceFrames) + fs := make([]StacktraceFrame, 0, maxStackTraceFrames) var frame runtime.Frame more := true for more { frame, more = frames.Next() - fs = append(fs, stacktraceFrame{ + fs = append(fs, StacktraceFrame{ Name: frame.Function, File: frame.File, Line: int64(frame.Line), diff --git a/v3/newrelic/stacktrace_test.go b/v3/newrelic/stacktrace_test.go index b286489cf..6e9c6f74d 100644 --- a/v3/newrelic/stacktrace_test.go +++ b/v3/newrelic/stacktrace_test.go @@ -37,7 +37,7 @@ func TestLongStackTraceLimitsFrames(t *testing.T) { } func TestManyStackTraceFramesLimitsOutput(t *testing.T) { - frames := make([]stacktraceFrame, maxStackTraceFrames+20) + frames := make([]StacktraceFrame, maxStackTraceFrames+20) expect := `[ {},{},{},{},{},{},{},{},{},{}, {},{},{},{},{},{},{},{},{},{}, @@ -60,7 +60,7 @@ func TestManyStackTraceFramesLimitsOutput(t *testing.T) { func TestStacktraceFrames(t *testing.T) { // This stacktrace taken from Go 1.13 - inputFrames := []stacktraceFrame{ + inputFrames := []StacktraceFrame{ { File: "/Users/will/Desktop/gopath/src/github.com/newrelic/go-agent/v3/internal/stacktrace.go", Name: "github.com/newrelic/go-agent/v3/internal.GetStackTrace", diff --git a/v3/newrelic/tracing.go b/v3/newrelic/tracing.go index 6058932b3..8baa56f97 100644 --- a/v3/newrelic/tracing.go +++ b/v3/newrelic/tracing.go @@ -35,6 +35,7 @@ type txnEvent struct { externalDuration time.Duration datastoreCallCount uint64 datastoreDuration time.Duration + errGroupCallback ErrorGroupCallback } // betterCAT stores the transaction's priority and all fields related From 51deb869a1387525ddf0aa0923df79420b32dbfb Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Tue, 7 Mar 2023 14:12:48 -0500 Subject: [PATCH 03/11] Error Group Callbacks Work With High Security When the agent is configured to protect user data from being ingested by New Relic, the callback function should still have access to data that would otherwise be scrubbed. The callback function is written by the customer, and is authorized to handle sensitive data. This will not change how sensitive data is sent to New Relic from the agent in any way. Users are strongly advised not to include sensitive data in their Error Group callback function. --- v3/newrelic/errors_from_internal.go | 38 ++++++++++++++++++++++++----- v3/newrelic/errors_test.go | 4 +-- v3/newrelic/harvest_test.go | 4 +-- v3/newrelic/internal_test.go | 32 ++++++++++++++++++++++++ v3/newrelic/internal_txn.go | 18 ++++++++++---- 5 files changed, 81 insertions(+), 15 deletions(-) diff --git a/v3/newrelic/errors_from_internal.go b/v3/newrelic/errors_from_internal.go index 231974103..cf8161303 100644 --- a/v3/newrelic/errors_from_internal.go +++ b/v3/newrelic/errors_from_internal.go @@ -142,13 +142,13 @@ func newHarvestErrors(max int) harvestErrors { } // mergeTxnErrors merges a transaction's errors into the harvest's errors. -func mergeTxnErrors(errors *harvestErrors, errs txnErrors, txnEvent txnEvent) { +func mergeTxnErrors(errors *harvestErrors, errs txnErrors, txnEvent txnEvent, hs *highSecuritySettings) { for _, e := range errs { if len(*errors) == cap(*errors) { return } - e.applyErrorGroup(&txnEvent) + e.scrubErrorForHighSecurity(hs) *errors = append(*errors, &tracedError{ txnEvent: txnEvent, errorData: *e, @@ -210,13 +210,39 @@ func (errData *errorData) applyErrorGroup(txnEvent *txnEvent) { } } -func (errData *errorData) scrubErrorForHighSecurity(txn *txn) { - if txn.Config.HighSecurity { +type highSecuritySettings struct { + enabled bool + allowRawExceptionMessages bool +} + +func (errData *errorData) scrubErrorForHighSecurity(hs *highSecuritySettings) { + if hs == nil { + return + } + + //txn.Config.HighSecurity + if hs.enabled { errData.Msg = highSecurityErrorMsg } - if !txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled() { + //!txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled() + if !hs.allowRawExceptionMessages { errData.Msg = securityPolicyErrorMsg - errData.RawError = nil } } + +func scrubbedErrorMessage(msg string, txn *txn) string { + if txn == nil { + return msg + } + + if txn.Config.HighSecurity { + return highSecurityErrorMsg + } + + if !txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled() { + return securityPolicyErrorMsg + } + + return msg +} diff --git a/v3/newrelic/errors_test.go b/v3/newrelic/errors_test.go index 9ab80bb8e..783306d18 100644 --- a/v3/newrelic/errors_test.go +++ b/v3/newrelic/errors_test.go @@ -238,7 +238,7 @@ func TestErrorsLifecycle(t *testing.T) { Priority: 0.5, }, TotalTime: 2 * time.Second, - }) + }, nil) js, err := he.Data("agentRunID", time.Now()) if nil != err { t.Error(err) @@ -344,7 +344,7 @@ func BenchmarkErrorsJSON(b *testing.B) { mergeTxnErrors(&he, ers, txnEvent{ FinalName: "WebTransaction/Go/hello", Attrs: attr, - }) + }, nil) b.ReportAllocs() b.ResetTimer() diff --git a/v3/newrelic/harvest_test.go b/v3/newrelic/harvest_test.go index 306cac976..58748fab0 100644 --- a/v3/newrelic/harvest_test.go +++ b/v3/newrelic/harvest_test.go @@ -509,7 +509,7 @@ func TestHarvestMetricsTracesReady(t *testing.T) { ers := newTxnErrors(10) ers.Add(errorData{When: time.Now(), Msg: "msg", Klass: "klass", Stack: getStackTrace()}) - mergeTxnErrors(&h.ErrorTraces, ers, txnEvent{FinalName: "finalName", Attrs: nil}) + mergeTxnErrors(&h.ErrorTraces, ers, txnEvent{FinalName: "finalName", Attrs: nil}, nil) h.TxnTraces.Witness(harvestTrace{ txnEvent: txnEvent{ @@ -612,7 +612,7 @@ func TestMergeFailedHarvest(t *testing.T) { mergeTxnErrors(&h.ErrorTraces, ers, txnEvent{ FinalName: "finalName", Attrs: nil, - }) + }, nil) h.SpanEvents.addEventPopulated(&sampleSpanEvent) if start1 != h.Metrics.metricPeriodStart { diff --git a/v3/newrelic/internal_test.go b/v3/newrelic/internal_test.go index 197022701..d69c481ce 100644 --- a/v3/newrelic/internal_test.go +++ b/v3/newrelic/internal_test.go @@ -843,6 +843,38 @@ func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { app.ExpectMetrics(t, webErrorMetrics) } +func TestResponseCodeErrorGroupCallbackWithHighSecurity(t *testing.T) { + errorGroupFunc := func(e ErrorInfo) string { + if e.Error != nil { + t.Errorf("expected ErrorInfo.Error to be nil, but got %v", e.Error) + } + AssertStringEqual(t, "ErrorInfo.TransactionName", `WebTransaction/Go/hello`, e.TransactionName) + AssertStringEqual(t, "ErrorInfo.Message", "Bad Request", e.Message) + AssertStringEqual(t, "ErrorInfo.Class", "400", e.Class) + return "testGroup" + } + + app := testApp( + nil, + func(cfg *Config) { + cfg.DistributedTracer.Enabled = false + cfg.ErrorCollector.ErrorGroupCallback = errorGroupFunc + cfg.DistributedTracer.Enabled = true + cfg.HighSecurity = true + }, + t, + ) + w := newCompatibleResponseRecorder() + txn := app.StartTransaction("hello") + rw := txn.SetWebResponse(w) + txn.SetWebRequestHTTP(helloRequest) + + rw.WriteHeader(http.StatusBadRequest) // 400 + rw.WriteHeader(http.StatusUnauthorized) // 401 + + txn.End() +} + func TestResponseCode404Filtered(t *testing.T) { app := testApp(nil, ConfigDistributedTracerEnabled(false), t) w := newCompatibleResponseRecorder() diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go index 0378b22f8..e53cf1155 100644 --- a/v3/newrelic/internal_txn.go +++ b/v3/newrelic/internal_txn.go @@ -321,13 +321,21 @@ func (txn *txn) MergeIntoHarvest(h *harvest) { // pass the callback func down the chain txn.txnEvent.errGroupCallback = txn.Config.ErrorCollector.ErrorGroupCallback + hs := &highSecuritySettings{txn.Config.HighSecurity, txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled()} + + if (txn.Reply.CollectErrors || txn.Config.ErrorCollector.CaptureEvents) && txn.Config.ErrorCollector.ErrorGroupCallback != nil { + for _, e := range txn.Errors { + e.applyErrorGroup(&txn.txnEvent) + } + } + if txn.Reply.CollectErrors { - mergeTxnErrors(&h.ErrorTraces, txn.Errors, txn.txnEvent) + mergeTxnErrors(&h.ErrorTraces, txn.Errors, txn.txnEvent, hs) } if txn.Config.ErrorCollector.CaptureEvents { for _, e := range txn.Errors { - e.applyErrorGroup(&txn.txnEvent) + e.scrubErrorForHighSecurity(hs) errEvent := &errorEvent{ errorData: *e, txnEvent: txn.txnEvent, @@ -490,8 +498,9 @@ func (thd *thread) End(recovered interface{}) error { if txn.rootSpanErrData != nil { root.AgentAttributes.addString(SpanAttributeErrorClass, txn.rootSpanErrData.Klass) - root.AgentAttributes.addString(SpanAttributeErrorMessage, txn.rootSpanErrData.Msg) + root.AgentAttributes.addString(SpanAttributeErrorMessage, scrubbedErrorMessage(txn.rootSpanErrData.Msg, txn)) } + if p := txn.BetterCAT.Inbound; nil != p { root.ParentID = txn.BetterCAT.Inbound.ID root.TrustedParentID = txn.BetterCAT.Inbound.TrustedParentID @@ -586,7 +595,6 @@ func (thd *thread) noticeErrorInternal(errData errorData, err error, expect bool } errData.RawError = err - errData.scrubErrorForHighSecurity(txn) if txn.shouldCollectSpanEvents() { errData.SpanID = txn.CurrentSpanIdentifier(thd.thread) @@ -613,7 +621,7 @@ func addErrorAttrs(t *thread, err errorData) { t.thread.RemoveErrorSpanAttribute(attr) } t.thread.AddAgentSpanAttribute(SpanAttributeErrorClass, err.Klass) - t.thread.AddAgentSpanAttribute(SpanAttributeErrorMessage, err.Msg) + t.thread.AddAgentSpanAttribute(SpanAttributeErrorMessage, scrubbedErrorMessage(err.Msg, t.txn)) } var ( From 3c1d6cabb8d2ed6f09767956802c4d4f4cb34ec4 Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Wed, 8 Mar 2023 15:49:08 -0500 Subject: [PATCH 04/11] search for deprecated statuscode attribute key --- v3/newrelic/error_group.go | 25 +++++++++++-------------- v3/newrelic/internal_test.go | 32 +++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/v3/newrelic/error_group.go b/v3/newrelic/error_group.go index 2651f4316..3bfd63f16 100644 --- a/v3/newrelic/error_group.go +++ b/v3/newrelic/error_group.go @@ -11,10 +11,9 @@ const ( // All fields are either safe to access coppies of internal agent data, or protected from direct // access with methods and can not manipulate or distort any agent data. type ErrorInfo struct { - errAttributes map[string]interface{} - txnAttributes *attributes - stackTrace stackTrace - isWebTransaction bool + errAttributes map[string]interface{} + txnAttributes *attributes + stackTrace stackTrace // TransactionName is the formatted name of a transaction that is equivilent to how it appears in // the New Relic UI. For example, user defined transactions will be named `OtherTransaction/Go/yourTxnName`. @@ -78,10 +77,6 @@ func (e *ErrorInfo) GetStackTraceFrames() []StacktraceFrame { // GetRequestURI returns the URI of the http request made during the parent transaction of this error. If no web request occured, // this will return an empty string. func (e *ErrorInfo) GetRequestURI() string { - if !e.isWebTransaction { - return "" - } - val, ok := e.txnAttributes.Agent[AttributeRequestURI] if !ok { return "" @@ -93,10 +88,6 @@ func (e *ErrorInfo) GetRequestURI() string { // GetRequestMethod will return the HTTP method used to make a web request if one occured during the parent transaction // of this error. If no web request occured, then an empty string will be returned. func (e *ErrorInfo) GetRequestMethod() string { - if !e.isWebTransaction { - return "" - } - val, ok := e.txnAttributes.Agent[AttributeRequestMethod] if !ok { return "" @@ -108,11 +99,17 @@ func (e *ErrorInfo) GetRequestMethod() string { // GetHttpResponseCode will return the HTTP response code that resulted from the web request made in the parent transaction of // this error. If no web request occured, then an empty string will be returned. func (e *ErrorInfo) GetHttpResponseCode() string { - if !e.isWebTransaction { + val, ok := e.txnAttributes.Agent[AttributeResponseCode] + if !ok { return "" } - val, ok := e.txnAttributes.Agent[AttributeResponseCode] + code := val.stringVal + if code != "" { + return code + } + + val, ok = e.txnAttributes.Agent[AttributeResponseCodeDeprecated] if !ok { return "" } diff --git a/v3/newrelic/internal_test.go b/v3/newrelic/internal_test.go index d69c481ce..10561273e 100644 --- a/v3/newrelic/internal_test.go +++ b/v3/newrelic/internal_test.go @@ -6,6 +6,7 @@ package newrelic import ( "encoding/json" "errors" + "fmt" "math" "net/http" "net/http/httptest" @@ -138,9 +139,11 @@ type compatibleResponseRecorder struct { } func newCompatibleResponseRecorder() *compatibleResponseRecorder { - return &compatibleResponseRecorder{ + recorder := compatibleResponseRecorder{ ResponseRecorder: httptest.NewRecorder(), } + + return &recorder } func (rw *compatibleResponseRecorder) Header() http.Header { @@ -534,10 +537,6 @@ func TestErrorWithCallback(t *testing.T) { t.Error("expected error stack trace to not be empty") } - if e.isWebTransaction { - t.Error("expected error to not be part of a web txn") - } - AssertStringEqual(t, "ErrorInfo.TransactionName", `OtherTransaction/Go/hello`, e.TransactionName) AssertStringEqual(t, "ErrorInfo.Message", "this is a test error", e.Message) AssertStringEqual(t, "ErrorInfo.Class", "test class", e.Class) @@ -790,7 +789,7 @@ func AssertStringEqual(t *testing.T, field string, expect string, actual string) } } -func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { +func TestResponseCodeErrorWithCallback(t *testing.T) { errorGroupFunc := func(e ErrorInfo) string { if e.Error != nil { t.Errorf("expected ErrorInfo.Error to be nil, but got %v", e.Error) @@ -798,6 +797,13 @@ func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { AssertStringEqual(t, "ErrorInfo.TransactionName", `WebTransaction/Go/hello`, e.TransactionName) AssertStringEqual(t, "ErrorInfo.Message", "Bad Request", e.Message) AssertStringEqual(t, "ErrorInfo.Class", "400", e.Class) + + val, ok := e.GetTransactionUserAttribute("test") + if !ok { + t.Errorf("expected attribute \"test\" to be found in txn attributes") + } else { + AssertStringEqual(t, "User Txn Attribute \"test\"", "test value", fmt.Sprint(val)) + } return "testGroup" } @@ -811,6 +817,7 @@ func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { ) w := newCompatibleResponseRecorder() txn := app.StartTransaction("hello") + txn.AddAttribute("test", "test value") rw := txn.SetWebResponse(w) txn.SetWebRequestHTTP(helloRequest) @@ -843,7 +850,7 @@ func TestResponseCodeErrorWithErrorGroupCallback(t *testing.T) { app.ExpectMetrics(t, webErrorMetrics) } -func TestResponseCodeErrorGroupCallbackWithHighSecurity(t *testing.T) { +func TestErrorGroupCallbackWithHighSecurity(t *testing.T) { errorGroupFunc := func(e ErrorInfo) string { if e.Error != nil { t.Errorf("expected ErrorInfo.Error to be nil, but got %v", e.Error) @@ -851,6 +858,15 @@ func TestResponseCodeErrorGroupCallbackWithHighSecurity(t *testing.T) { AssertStringEqual(t, "ErrorInfo.TransactionName", `WebTransaction/Go/hello`, e.TransactionName) AssertStringEqual(t, "ErrorInfo.Message", "Bad Request", e.Message) AssertStringEqual(t, "ErrorInfo.Class", "400", e.Class) + AssertStringEqual(t, "Request URI", "/hello", e.GetRequestURI()) + AssertStringEqual(t, "Request Method", "GET", e.GetRequestMethod()) + AssertStringEqual(t, "Response Code", "400", e.GetHttpResponseCode()) + + _, ok := e.GetTransactionUserAttribute("test") + if ok { + t.Errorf("attributes can not be recorded during high security mode") + } + return "testGroup" } @@ -866,6 +882,8 @@ func TestResponseCodeErrorGroupCallbackWithHighSecurity(t *testing.T) { ) w := newCompatibleResponseRecorder() txn := app.StartTransaction("hello") + // you may not record user attributes with high security enabled + txn.AddAttribute("test", "test value") rw := txn.SetWebResponse(w) txn.SetWebRequestHTTP(helloRequest) From 5dbe24c2ee6eaa932997d94f226553a9edd926ee Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Thu, 9 Mar 2023 12:14:11 -0500 Subject: [PATCH 05/11] User Tracking A new transaction method, SetUserID() allows you to track which users are impacted by the events that occur during a transaction. This will add an agent attribute to all child events of a transacaction that will allow users to link those events back to a specific user. --- v3/newrelic/attributes.go | 2 ++ v3/newrelic/attributes_from_internal.go | 17 +++++++++++++++++ v3/newrelic/internal_txn.go | 11 +++++++++++ v3/newrelic/transaction.go | 14 ++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/v3/newrelic/attributes.go b/v3/newrelic/attributes.go index 040e4df0f..258d23b4b 100644 --- a/v3/newrelic/attributes.go +++ b/v3/newrelic/attributes.go @@ -54,6 +54,8 @@ const ( AttributeCodeLineno = "code.lineno" // AttributeErrorGroupName contains the error group name set by the user defined callback function. AttributeErrorGroupName = "error.group.name" + // AttributeUserID tracks the user a transaction and its child events are impacting + AttributeUserID = "enduser.id" ) // Attributes destined for Errors and Transaction Traces: diff --git a/v3/newrelic/attributes_from_internal.go b/v3/newrelic/attributes_from_internal.go index 83a9f98df..ee8070d7e 100644 --- a/v3/newrelic/attributes_from_internal.go +++ b/v3/newrelic/attributes_from_internal.go @@ -5,6 +5,7 @@ package newrelic import ( "bytes" + "errors" "fmt" "math" "net/http" @@ -390,6 +391,22 @@ func validateFloat(v float64, key string) error { return nil } +// addAgentAttribute adds an agent attribute. +func addAgentAttribute(a *attributes, key, value string) error { + val, err := validateUserAttribute(key, value) + if nil != err { + return err + } + + strVal, ok := val.(string) + if !ok { + return errors.New("validated agent attribute was not returned as a string") + } + + a.Agent.Add(key, strVal, nil) + return nil +} + // addUserAttribute adds a user attribute. func addUserAttribute(a *attributes, key string, val interface{}, d destinationSet) error { val, err := validateUserAttribute(key, val) diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go index e53cf1155..0e9171284 100644 --- a/v3/newrelic/internal_txn.go +++ b/v3/newrelic/internal_txn.go @@ -545,6 +545,17 @@ func (thd *thread) End(recovered interface{}) error { return nil } +func (txn *txn) AddUserID(userID string) error { + txn.Lock() + defer txn.Unlock() + + if txn.finished { + return errAlreadyEnded + } + + return addAgentAttribute(txn.Attrs, AttributeUserID, userID) +} + func (txn *txn) AddAttribute(name string, value interface{}) error { txn.Lock() defer txn.Unlock() diff --git a/v3/newrelic/transaction.go b/v3/newrelic/transaction.go index 31f22b39e..0b94d0414 100644 --- a/v3/newrelic/transaction.go +++ b/v3/newrelic/transaction.go @@ -169,6 +169,20 @@ func (txn *Transaction) AddAttribute(key string, value interface{}) { txn.thread.logAPIError(txn.thread.AddAttribute(key, value), "add attribute", nil) } +// SetUserID is used to track the user that a transaction, and all data that is recorded as a subset of that transaction, +// belong to or interact with. This will propogate an attribute containing this information to all events that are +// a child of this transaction, like errors and spans. +func (txn *Transaction) SetUserID(userID string) { + if nil == txn { + return + } + if nil == txn.thread { + return + } + + txn.thread.logAPIError(txn.thread.AddUserID(userID), "set user ID", nil) +} + // RecordLog records the data from a single log line. // This consumes a LogData object that should be configured // with data taken from a logging framework. From fce65b39a598858eb67460e4426c0aa248f7d1fb Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Fri, 10 Mar 2023 13:18:47 -0500 Subject: [PATCH 06/11] lookup user ID in error group func --- v3/newrelic/error_group.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/v3/newrelic/error_group.go b/v3/newrelic/error_group.go index 3bfd63f16..484c1e826 100644 --- a/v3/newrelic/error_group.go +++ b/v3/newrelic/error_group.go @@ -117,6 +117,17 @@ func (e *ErrorInfo) GetHttpResponseCode() string { return val.stringVal } +// GetEnduserID will return the User ID set for the parent transaction of this error. It will return empty string +// if none was set. +func (e *ErrorInfo) GetEnduserID() string { + val, ok := e.txnAttributes.Agent[AttributeUserID] + if !ok { + return "" + } + + return val.stringVal +} + // ErrorGroupCallback is a user defined callback function that takes an error as an input // and returns a string that will be applied to an error to put it in an error group. // From 670829c7b1b5b3ea637e76cd6ff1e774ed01e669 Mon Sep 17 00:00:00 2001 From: mirackara Date: Tue, 14 Mar 2023 11:32:26 -0500 Subject: [PATCH 07/11] Increase test code coverage percentage for nrgrpc --- v3/integrations/nrgrpc/nrgrpc_server_test.go | 103 +++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/v3/integrations/nrgrpc/nrgrpc_server_test.go b/v3/integrations/nrgrpc/nrgrpc_server_test.go index cfb961cc7..470fb5f57 100644 --- a/v3/integrations/nrgrpc/nrgrpc_server_test.go +++ b/v3/integrations/nrgrpc/nrgrpc_server_test.go @@ -12,6 +12,7 @@ import ( "testing" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/test/bufconn" @@ -53,6 +54,107 @@ func newTestServerAndConn(t *testing.T, app *newrelic.Application) (*grpc.Server return s, conn } +func TestWithCustomStatusHandler(t *testing.T) { + app := testApp() + Configure(WithStatusHandler(codes.OK, WarningInterceptorStatusHandler)) + + s, conn := newTestServerAndConn(t, app.Application) + defer s.Stop() + defer conn.Close() + + client := testapp.NewTestApplicationClient(conn) + txn := app.StartTransaction("client") + ctx := newrelic.NewContext(context.Background(), txn) + _, err := client.DoUnaryUnary(ctx, &testapp.Message{}) + if err != nil { + t.Fatal("unable to call client DoUnaryUnary", err) + } + + app.ExpectMetrics(t, []internal.WantMetric{ + {Name: "Apdex", Scope: "", Forced: true, Data: nil}, + {Name: "Apdex/Go/TestApplication/DoUnaryUnary", Scope: "", Forced: false, Data: nil}, + {Name: "Custom/DoUnaryUnary", Scope: "", Forced: false, Data: nil}, + {Name: "Custom/DoUnaryUnary", Scope: "WebTransaction/Go/TestApplication/DoUnaryUnary", Forced: false, Data: nil}, + {Name: "DurationByCaller/App/123/456/HTTP/all", Scope: "", Forced: false, Data: nil}, + {Name: "DurationByCaller/App/123/456/HTTP/allWeb", Scope: "", Forced: false, Data: nil}, + {Name: "HttpDispatcher", Scope: "", Forced: true, Data: nil}, + {Name: "Supportability/TraceContext/Accept/Success", Scope: "", Forced: true, Data: nil}, + {Name: "TransportDuration/App/123/456/HTTP/all", Scope: "", Forced: false, Data: nil}, + {Name: "TransportDuration/App/123/456/HTTP/allWeb", Scope: "", Forced: false, Data: nil}, + {Name: "WebTransaction", Scope: "", Forced: true, Data: nil}, + {Name: "WebTransaction/Go/TestApplication/DoUnaryUnary", Scope: "", Forced: true, Data: nil}, + {Name: "WebTransactionTotalTime", Scope: "", Forced: true, Data: nil}, + {Name: "WebTransactionTotalTime/Go/TestApplication/DoUnaryUnary", Scope: "", Forced: false, Data: nil}, + }) + app.ExpectTxnEvents(t, []internal.WantEvent{{ + Intrinsics: map[string]interface{}{ + "guid": internal.MatchAnything, + "name": "WebTransaction/Go/TestApplication/DoUnaryUnary", + "nr.apdexPerfZone": internal.MatchAnything, + "parent.account": 123, + "parent.app": 456, + "parent.transportDuration": internal.MatchAnything, + "parent.transportType": "HTTP", + "parent.type": "App", + "parentId": internal.MatchAnything, + "parentSpanId": internal.MatchAnything, + "priority": internal.MatchAnything, + "sampled": internal.MatchAnything, + "traceId": internal.MatchAnything, + }, + UserAttributes: map[string]interface{}{ + "grpcStatusLevel": "warning", + "grpcStatusCode": "OK", + "grpcStatusMessage": internal.MatchAnything, + }, + AgentAttributes: map[string]interface{}{ + "httpResponseCode": 0, + "http.statusCode": 0, + "request.headers.contentType": "application/grpc", + "request.method": "TestApplication/DoUnaryUnary", + "request.uri": "grpc://bufnet/TestApplication/DoUnaryUnary", + }, + }}) + app.ExpectSpanEvents(t, []internal.WantEvent{ + { + Intrinsics: map[string]interface{}{ + "category": "generic", + "name": "Custom/DoUnaryUnary", + "parentId": internal.MatchAnything, + }, + UserAttributes: map[string]interface{}{}, + AgentAttributes: map[string]interface{}{}, + }, + { + Intrinsics: map[string]interface{}{ + "category": "generic", + "name": "WebTransaction/Go/TestApplication/DoUnaryUnary", + "transaction.name": "WebTransaction/Go/TestApplication/DoUnaryUnary", + "nr.entryPoint": true, + "parentId": internal.MatchAnything, + "trustedParentId": internal.MatchAnything, + }, + UserAttributes: map[string]interface{}{ + "grpcStatusLevel": "warning", + "grpcStatusCode": "OK", + }, + AgentAttributes: map[string]interface{}{ + "httpResponseCode": 0, + "http.statusCode": 0, + "parent.account": "123", + "parent.app": "456", + "parent.transportDuration": internal.MatchAnything, + "parent.transportType": "HTTP", + "parent.type": "App", + "request.headers.contentType": "application/grpc", + "request.method": "TestApplication/DoUnaryUnary", + "request.uri": "grpc://bufnet/TestApplication/DoUnaryUnary", + }, + }, + }) + Configure(WithStatusHandler(codes.OK, OKInterceptorStatusHandler)) +} + func TestUnaryServerInterceptor(t *testing.T) { app := testApp() @@ -143,6 +245,7 @@ func TestUnaryServerInterceptor(t *testing.T) { }, }, }) + } func TestUnaryServerInterceptorError(t *testing.T) { From 0b0b9bbc44bf24bc94fd85aa1356d421904f23dc Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Tue, 14 Mar 2023 14:32:27 -0400 Subject: [PATCH 08/11] patching error group attribute bug and finishing touches --- v3/go.mod | 8 -------- v3/newrelic/attributes_from_internal.go | 20 ++------------------ v3/newrelic/config.go | 5 +++-- v3/newrelic/config_options.go | 19 +++++++++++-------- v3/newrelic/error_group.go | 4 ++-- v3/newrelic/internal_txn.go | 8 +++----- 6 files changed, 21 insertions(+), 43 deletions(-) diff --git a/v3/go.mod b/v3/go.mod index c8c214856..4eaf02fb4 100644 --- a/v3/go.mod +++ b/v3/go.mod @@ -6,11 +6,3 @@ require ( github.com/golang/protobuf v1.5.2 google.golang.org/grpc v1.49.0 ) - -require ( - golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect - golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect - golang.org/x/text v0.3.3 // indirect - google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect - google.golang.org/protobuf v1.27.1 // indirect -) diff --git a/v3/newrelic/attributes_from_internal.go b/v3/newrelic/attributes_from_internal.go index ee8070d7e..b9a016300 100644 --- a/v3/newrelic/attributes_from_internal.go +++ b/v3/newrelic/attributes_from_internal.go @@ -5,7 +5,6 @@ package newrelic import ( "bytes" - "errors" "fmt" "math" "net/http" @@ -57,6 +56,7 @@ var ( AttributeCodeNamespace: usualDests, AttributeCodeFilepath: usualDests, AttributeCodeLineno: usualDests, + AttributeUserID: usualDests, // Span specific attributes SpanAttributeDBStatement: usualDests, @@ -391,22 +391,6 @@ func validateFloat(v float64, key string) error { return nil } -// addAgentAttribute adds an agent attribute. -func addAgentAttribute(a *attributes, key, value string) error { - val, err := validateUserAttribute(key, value) - if nil != err { - return err - } - - strVal, ok := val.(string) - if !ok { - return errors.New("validated agent attribute was not returned as a string") - } - - a.Agent.Add(key, strVal, nil) - return nil -} - // addUserAttribute adds a user attribute. func addUserAttribute(a *attributes, key string, val interface{}, d destinationSet) error { val, err := validateUserAttribute(key, val) @@ -479,6 +463,7 @@ func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, add buf.WriteString("{}") return } + w := jsonFieldsWriter{buf: buf} buf.WriteByte('{') for id, val := range a.Agent { @@ -499,7 +484,6 @@ func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, add } buf.WriteByte('}') - } func userAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, extraAttributes map[string]interface{}) { diff --git a/v3/newrelic/config.go b/v3/newrelic/config.go index d80afb19e..01508cc63 100644 --- a/v3/newrelic/config.go +++ b/v3/newrelic/config.go @@ -123,10 +123,11 @@ type Config struct { // example function: // // func ErrorGroupCallback(errorInfo newrelic.ErrorInfo) string { - // if errorInfo.IsWeb && errorInfo.Class == "403" { - // return "client misconfigured example-group" + // if errorInfo.Class == "403" && errorInfo.GetUserId() == "example user" { + // return "customer X payment issue" // } // + // // returning empty string causes default error grouping behavior // return "" // } ErrorGroupCallback `json:"-"` diff --git a/v3/newrelic/config_options.go b/v3/newrelic/config_options.go index dd395d03b..bcaccc097 100644 --- a/v3/newrelic/config_options.go +++ b/v3/newrelic/config_options.go @@ -199,14 +199,6 @@ func ConfigCodeLevelMetricsPathPrefixes(prefix ...string) ConfigOption { } } -// ConfigModuleDependencyMetricsIgnoredPrefixes sets the ErrorFingerprintingCallback function -// to a function defined by the user. -func ConfigErrorFingerprintingCallback(callbackFunction ErrorGroupCallback) ConfigOption { - return func(cfg *Config) { - cfg.ErrorCollector.ErrorGroupCallback = callbackFunction - } -} - // ConfigAppLogForwardingEnabled enables or disables the collection // of logs from a user's application by the agent // Defaults: enabled=false @@ -296,6 +288,17 @@ func ConfigModuleDependencyMetricsIgnoredPrefixes(prefix ...string) ConfigOption } } +// ConfigSetErrorGroupCallbackFunction set a callback function of type ErrorGroupCallback that will +// be invoked against errors at harvest time. This function overrides the default grouping behavior +// of errors into a custom, user defined group when set. Setting this may have performance implications +// for your application depending on the contents of the callback function. Do not set this if you want +// the default error grouping behavior to be executed. +func ConfigSetErrorGroupCallbackFunction(callback ErrorGroupCallback) ConfigOption { + return func(cfg *Config) { + cfg.ErrorCollector.ErrorGroupCallback = callback + } +} + // ConfigModuleDependencyMetricsRedactIgnoredPrefixes controls whether the names // of ignored module path prefixes should be redacted from the agent configuration data // reported and visible in the New Relic UI. Since one of the reasons these diff --git a/v3/newrelic/error_group.go b/v3/newrelic/error_group.go index 484c1e826..d132756fe 100644 --- a/v3/newrelic/error_group.go +++ b/v3/newrelic/error_group.go @@ -117,9 +117,9 @@ func (e *ErrorInfo) GetHttpResponseCode() string { return val.stringVal } -// GetEnduserID will return the User ID set for the parent transaction of this error. It will return empty string +// GetUserID will return the User ID set for the parent transaction of this error. It will return empty string // if none was set. -func (e *ErrorInfo) GetEnduserID() string { +func (e *ErrorInfo) GetUserID() string { val, ok := e.txnAttributes.Agent[AttributeUserID] if !ok { return "" diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go index 0e9171284..872f78a99 100644 --- a/v3/newrelic/internal_txn.go +++ b/v3/newrelic/internal_txn.go @@ -318,12 +318,10 @@ func (txn *txn) MergeIntoHarvest(h *harvest) { h.TxnEvents.AddTxnEvent(alloc, priority) } - // pass the callback func down the chain - txn.txnEvent.errGroupCallback = txn.Config.ErrorCollector.ErrorGroupCallback - hs := &highSecuritySettings{txn.Config.HighSecurity, txn.Reply.SecurityPolicies.AllowRawExceptionMessages.Enabled()} if (txn.Reply.CollectErrors || txn.Config.ErrorCollector.CaptureEvents) && txn.Config.ErrorCollector.ErrorGroupCallback != nil { + txn.txnEvent.errGroupCallback = txn.Config.ErrorCollector.ErrorGroupCallback for _, e := range txn.Errors { e.applyErrorGroup(&txn.txnEvent) } @@ -548,12 +546,12 @@ func (thd *thread) End(recovered interface{}) error { func (txn *txn) AddUserID(userID string) error { txn.Lock() defer txn.Unlock() - if txn.finished { return errAlreadyEnded } - return addAgentAttribute(txn.Attrs, AttributeUserID, userID) + txn.Attrs.Agent.Add(AttributeUserID, userID, nil) + return nil } func (txn *txn) AddAttribute(name string, value interface{}) error { From 956c7870c74609a2abd667ce7eb68710bc30586b Mon Sep 17 00:00:00 2001 From: Mirac Kara <55501260+mirackara@users.noreply.github.com> Date: Thu, 16 Mar 2023 12:56:28 -0500 Subject: [PATCH 09/11] Minor typo fix --- v3/newrelic/error_group.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v3/newrelic/error_group.go b/v3/newrelic/error_group.go index d132756fe..4a25a8aa6 100644 --- a/v3/newrelic/error_group.go +++ b/v3/newrelic/error_group.go @@ -8,7 +8,7 @@ const ( ) // ErrorInfo contains info for user defined callbacks that are relevant to an error. -// All fields are either safe to access coppies of internal agent data, or protected from direct +// All fields are either safe to access copies of internal agent data, or protected from direct // access with methods and can not manipulate or distort any agent data. type ErrorInfo struct { errAttributes map[string]interface{} From 11d6baa0fcb6e0fa37a2a98bb806db16d4241c26 Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Thu, 30 Mar 2023 13:33:15 -0400 Subject: [PATCH 10/11] release 3.21.0 of the go agent --- CHANGELOG.md | 19 +++++++++++++++++++ v3/newrelic/version.go | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80621f84b..76b40a314 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +## 3.21.0 + +### Added +* New Errors inbox features: + * User tracking: You can now see the number of users impacted by an error group. Identify the end user with the setUser method. + * Error fingerprint: Are your error occurrences grouped poorly? Set your own error fingerprint via a callback function. +* Automatically instrument Redis 9 code using the new instrumentation package nrredis-v9. +* Ability to disable reporting parameterized query in nrpgx-5 + +### Fixed +* Improved test coverage for gRPC integration, nrgrpc + +### Support Statement +New Relic recommends that you upgrade the agent regularly to ensure that you’re getting the latest features and performance benefits. Additionally, older releases will no longer be supported when they reach end-of-life. + +We also recommend using the latest version of the Go language. At minimum, you should at least be using no version of Go older than what is supported by the Go team themselves. + +See the [Go Agent EOL Policy](https://docs.newrelic.com/docs/apm/agents/go-agent/get-started/go-agent-eol-policy/) for details about supported versions of the Go Agent and third-party components. + ## 3.20.4 ### Fixed diff --git a/v3/newrelic/version.go b/v3/newrelic/version.go index 1a9079bae..40e4d3d99 100644 --- a/v3/newrelic/version.go +++ b/v3/newrelic/version.go @@ -11,7 +11,7 @@ import ( const ( // Version is the full string version of this Go Agent. - Version = "3.20.4" + Version = "3.21.0" ) var ( From 01477465fba4a11184931ff2e8e8cd036d19c548 Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Thu, 30 Mar 2023 14:08:44 -0400 Subject: [PATCH 11/11] nrreddis9 missing release 3.21.0 issues with tests caused nrredis-9 to not release --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b40a314..9108920db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ * New Errors inbox features: * User tracking: You can now see the number of users impacted by an error group. Identify the end user with the setUser method. * Error fingerprint: Are your error occurrences grouped poorly? Set your own error fingerprint via a callback function. -* Automatically instrument Redis 9 code using the new instrumentation package nrredis-v9. * Ability to disable reporting parameterized query in nrpgx-5 ### Fixed