Merge pull request #611 from iamemilio/expectedErrors

iamemilio · web-flow · commit d82482211fc5 · 2022-12-14T15:30:21.000-05:00
Notice Expected Errors
diff --git a/v3/examples/server/main.go b/v3/examples/server/main.go
@@ -31,6 +31,13 @@ func noticeError(w http.ResponseWriter, r *http.Request) {
 	txn.NoticeError(errors.New("my error message"))
 }
 
+func noticeExpectedError(w http.ResponseWriter, r *http.Request) {
+	io.WriteString(w, "noticing an error")
+
+	txn := newrelic.FromContext(r.Context())
+	txn.NoticeExpectedError(errors.New("my expected error message"))
+}
+
 func noticeErrorWithAttributes(w http.ResponseWriter, r *http.Request) {
 	io.WriteString(w, "noticing an error")
 
@@ -273,6 +280,7 @@ func main() {
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/", index))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/version", versionHandler))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_error", noticeError))
+	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_expected_error", noticeExpectedError))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_error_with_attributes", noticeErrorWithAttributes))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/custom_event", customEvent))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/set_name", setName))
diff --git a/v3/newrelic/errors_from_internal.go b/v3/newrelic/errors_from_internal.go
@@ -61,6 +61,7 @@ type errorData struct {
 	Msg             string
 	Klass           string
 	SpanID          string
+	Expect          bool
 }
 
 // txnError combines error data with information about a transaction.  txnError is used for
@@ -113,7 +114,7 @@ func (h *tracedError) WriteJSON(buf *bytes.Buffer) {
 	buf.WriteByte(',')
 	buf.WriteString(`"intrinsics"`)
 	buf.WriteByte(':')
-	intrinsicsJSON(&h.txnEvent, buf)
+	intrinsicsJSON(&h.txnEvent, buf, h.errorData.Expect)
 	if nil != h.Stack {
 		buf.WriteByte(',')
 		buf.WriteString(`"stack_trace"`)
@@ -152,7 +153,7 @@ func mergeTxnErrors(errors *harvestErrors, errs txnErrors, txnEvent txnEvent) {
 }
 
 func (errors harvestErrors) Data(agentRunID string, harvestStart time.Time) ([]byte, error) {
-	if 0 == len(errors) {
+	if len(errors) == 0 {
 		return nil, nil
 	}
 	estimate := 1024 * len(errors)
diff --git a/v3/newrelic/harvest.go b/v3/newrelic/harvest.go
@@ -327,12 +327,16 @@ func createTxnMetrics(args *txnData, metrics *metricTable) {
 	}
 
 	// Error Metrics
-	if args.HasErrors() {
+	if args.NoticeErrors() {
 		metrics.addSingleCount(errorsRollupMetric.all, forced)
 		metrics.addSingleCount(errorsRollupMetric.webOrOther(args.IsWeb), forced)
 		metrics.addSingleCount(errorsPrefix+args.FinalName, forced)
 	}
 
+	if args.HasExpectedErrors() {
+		metrics.addSingleCount(expectedErrorsRollupMetric.all, forced)
+	}
+
 	// Queueing Metrics
 	if args.Queuing > 0 {
 		metrics.addDuration(queueMetric, "", args.Queuing, args.Queuing, forced)
diff --git a/v3/newrelic/harvest_test.go b/v3/newrelic/harvest_test.go
@@ -771,6 +771,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	webName := "WebTransaction/zip/zap"
 	backgroundName := "OtherTransaction/zip/zap"
 	args := &txnData{}
+	args.noticeErrors = true
 	args.Duration = 123 * time.Second
 	args.TotalTime = 150 * time.Second
 	args.ApdexThreshold = 2 * time.Second
@@ -803,6 +804,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexTolerating
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -821,6 +823,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -838,9 +841,32 @@ func TestCreateTxnMetrics(t *testing.T) {
 		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
 	})
 
+	// Verify expected errors metrics
+	args.FinalName = backgroundName
+	args.IsWeb = false
+	args.Errors = txnErrors
+	args.noticeErrors = false
+	args.expectedErrors = true
+	args.Zone = apdexNone
+	metrics = newMetricTable(100, time.Now())
+	createTxnMetrics(args, metrics)
+	expectMetrics(t, metrics, []internal.WantMetric{
+		{Name: backgroundName, Scope: "", Forced: true, Data: []float64{1, 123, 0, 123, 123, 123 * 123}},
+		{Name: backgroundRollup, Scope: "", Forced: true, Data: []float64{1, 123, 0, 123, 123, 123 * 123}},
+		{Name: "OtherTransactionTotalTime", Scope: "", Forced: true, Data: []float64{1, 150, 150, 150, 150, 150 * 150}},
+		{Name: "OtherTransactionTotalTime/zip/zap", Scope: "", Forced: false, Data: []float64{1, 150, 150, 150, 150, 150 * 150}},
+		{Name: "ErrorsExpected/all", Scope: "", Forced: true, Data: []float64{1, 0, 0, 0, 0, 0}},
+		{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: []float64{1, 123, 123, 123, 123, 123 * 123}},
+		{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 123, 123, 123, 123, 123 * 123}},
+		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
+		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
+	})
+
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = nil
+	args.noticeErrors = false
+	args.expectedErrors = false
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -889,6 +915,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexTolerating
 	metrics := newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -908,6 +935,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexTolerating
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -924,6 +952,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -940,6 +969,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
diff --git a/v3/newrelic/internal_errors_stacktrace_test.go b/v3/newrelic/internal_errors_stacktrace_test.go
@@ -64,7 +64,7 @@ func TestStackTrace(t *testing.T) {
 	}
 
 	for idx, tc := range testcases {
-		data, err := errDataFromError(tc.Error)
+		data, err := errDataFromError(tc.Error, false)
 		if err != nil {
 			t.Errorf("testcase %d: got error: %v", idx, err)
 			continue
diff --git a/v3/newrelic/internal_errors_test.go b/v3/newrelic/internal_errors_test.go
@@ -636,7 +636,7 @@ func TestErrorClass(t *testing.T) {
 	}
 
 	for idx, tc := range testcases {
-		data, err := errDataFromError(tc.Error)
+		data, err := errDataFromError(tc.Error, false)
 		if err != nil {
 			t.Errorf("testcase %d: got error: %v", idx, err)
 			continue
diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go
@@ -366,7 +366,7 @@ func headersJustWritten(thd *thread, code int, hdr http.Header) {
 	if txn.appRun.responseCodeIsError(code) {
 		e := txnErrorFromResponseCode(time.Now(), code)
 		e.Stack = getStackTrace()
-		thd.noticeErrorInternal(e)
+		thd.noticeErrorInternal(e, false)
 	}
 }
 
@@ -425,7 +425,7 @@ func (thd *thread) End(recovered interface{}) error {
 	if nil != recovered {
 		e := txnErrorFromPanic(time.Now(), recovered)
 		e.Stack = getStackTrace()
-		thd.noticeErrorInternal(e)
+		thd.noticeErrorInternal(e, false)
 		log.Println(string(debug.Stack()))
 	}
 
@@ -447,7 +447,7 @@ func (thd *thread) End(recovered interface{}) error {
 	txn.ApdexThreshold = internal.CalculateApdexThreshold(txn.Reply, txn.FinalName)
 
 	if txn.getsApdex() {
-		if txn.HasErrors() {
+		if txn.HasErrors() && txn.NoticeErrors() {
 			txn.Zone = apdexFailing
 		} else {
 			txn.Zone = calculateApdexZone(txn.ApdexThreshold, txn.Duration)
@@ -461,7 +461,7 @@ func (thd *thread) End(recovered interface{}) error {
 			"name":          txn.FinalName,
 			"duration_ms":   txn.Duration.Seconds() * 1000.0,
 			"ignored":       txn.ignore,
-			"app_connected": "" != txn.Reply.RunID,
+			"app_connected": txn.Reply.RunID != "",
 		})
 	}
 
@@ -559,12 +559,18 @@ const (
 	securityPolicyErrorMsg = "message removed by security policy"
 )
 
-func (thd *thread) noticeErrorInternal(err errorData) error {
+func (thd *thread) noticeErrorInternal(err errorData, expect bool) error {
 	txn := thd.txn
 	if !txn.Config.ErrorCollector.Enabled {
 		return errorsDisabled
 	}
 
+	if !expect {
+		thd.noticeErrors = true
+	} else {
+		thd.expectedErrors = true
+	}
+
 	if nil == txn.Errors {
 		txn.Errors = newTxnErrors(maxTxnErrors)
 	}
@@ -643,12 +649,13 @@ func errorAttributesMethod(err error) map[string]interface{} {
 	return nil
 }
 
-func errDataFromError(input error) (data errorData, err error) {
+func errDataFromError(input error, expect bool) (data errorData, err error) {
 	cause := errorCause(input)
 
 	data = errorData{
-		When: time.Now(),
-		Msg:  input.Error(),
+		When:   time.Now(),
+		Msg:    input.Error(),
+		Expect: expect,
 	}
 
 	if c := errorClassMethod(input); "" != c {
@@ -700,7 +707,7 @@ func errDataFromError(input error) (data errorData, err error) {
 	return data, nil
 }
 
-func (thd *thread) NoticeError(input error) error {
+func (thd *thread) NoticeError(input error, expect bool) error {
 	txn := thd.txn
 	txn.Lock()
 	defer txn.Unlock()
@@ -713,7 +720,7 @@ func (thd *thread) NoticeError(input error) error {
 		return errNilError
 	}
 
-	data, err := errDataFromError(input)
+	data, err := errDataFromError(input, expect)
 	if nil != err {
 		return err
 	}
@@ -722,7 +729,7 @@ func (thd *thread) NoticeError(input error) error {
 		data.ExtraAttributes = nil
 	}
 
-	return thd.noticeErrorInternal(data)
+	return thd.noticeErrorInternal(data, expect)
 }
 
 func (txn *txn) SetName(name string) error {
diff --git a/v3/newrelic/intrinsics.go b/v3/newrelic/intrinsics.go
@@ -7,13 +7,17 @@ import (
 	"bytes"
 )
 
+const (
+	expectErrorAttr = "error.expected"
+)
+
 func addOptionalStringField(w *jsonFieldsWriter, key, value string) {
 	if value != "" {
 		w.stringField(key, value)
 	}
 }
 
-func intrinsicsJSON(e *txnEvent, buf *bytes.Buffer) {
+func intrinsicsJSON(e *txnEvent, buf *bytes.Buffer, expect bool) {
 	w := jsonFieldsWriter{buf: buf}
 
 	buf.WriteByte('{')
@@ -27,6 +31,10 @@ func intrinsicsJSON(e *txnEvent, buf *bytes.Buffer) {
 		w.boolField("sampled", e.BetterCAT.Sampled)
 	}
 
+	if expect {
+		w.stringField(expectErrorAttr, "true")
+	}
+
 	if e.CrossProcess.Used() {
 		addOptionalStringField(&w, "client_cross_process_id", e.CrossProcess.ClientID)
 		addOptionalStringField(&w, "trip_id", e.CrossProcess.TripID)
diff --git a/v3/newrelic/metric_names.go b/v3/newrelic/metric_names.go
@@ -187,8 +187,8 @@ func (r rollupMetric) webOrOther(isWeb bool) string {
 }
 
 var (
-	errorsRollupMetric = newRollupMetric("Errors/")
-
+	errorsRollupMetric         = newRollupMetric("Errors/")
+	expectedErrorsRollupMetric = newRollupMetric("ErrorsExpected/")
 	// source.datanerd.us/agents/agent-specs/blob/master/APIs/external_segment.md
 	// source.datanerd.us/agents/agent-specs/blob/master/APIs/external_cat.md
 	// source.datanerd.us/agents/agent-specs/blob/master/Cross-Application-Tracing-PORTED.md
diff --git a/v3/newrelic/tracing.go b/v3/newrelic/tracing.go
@@ -63,38 +63,42 @@ func (bc *betterCAT) SetTraceAndTxnIDs(traceID string) {
 
 // txnData contains the recorded data of a transaction.
 type txnData struct {
-	txnEvent
-	IsWeb          bool
-	Name           string    // Work in progress name.
-	Errors         txnErrors // Lazily initialized.
-	Stop           time.Time
-	ApdexThreshold time.Duration
+	IsWeb              bool
+	SlowQueriesEnabled bool
+	noticeErrors       bool // If errors are not expected or ignored, then true
+	expectedErrors     bool
 
 	stamp           segmentStamp
 	threadIDCounter uint64
 
+	Name       string // Work in progress name.
+	rootSpanID string
+
+	txnEvent
+	TxnTrace txnTrace
+
+	Stop               time.Time
+	ApdexThreshold     time.Duration
+	SlowQueryThreshold time.Duration
+
+	SlowQueries *slowQueries
+
+	// These better CAT supportability fields are left outside of
+	// TxnEvent.BetterCAT to minimize the size of transaction event memory.
+	DistributedTracingSupport distributedTracingSupport
+
 	TraceIDGenerator        *internal.TraceIDGenerator
 	ShouldCollectSpanEvents func() bool
 	ShouldCreateSpanGUID    func() bool
-	rootSpanID              string
 	rootSpanErrData         *errorData
+	Errors                  txnErrors // Lazily initialized.
 	SpanEvents              []*spanEvent
 	logs                    logEventHeap
 
 	customSegments    map[string]*metricData
 	datastoreSegments map[datastoreMetricKey]*metricData
 	externalSegments  map[externalMetricKey]*metricData
 	messageSegments   map[internal.MessageMetricKey]*metricData
-
-	TxnTrace txnTrace
-
-	SlowQueriesEnabled bool
-	SlowQueryThreshold time.Duration
-	SlowQueries        *slowQueries
-
-	// These better CAT supportability fields are left outside of
-	// TxnEvent.BetterCAT to minimize the size of transaction event memory.
-	DistributedTracingSupport distributedTracingSupport
 }
 
 func (t *txnData) saveTraceSegment(end segmentEnd, name string, attrs spanAttributeMap, externalGUID string) {
@@ -320,11 +324,21 @@ const (
 	datastoreOperationUnknown = "other"
 )
 
+// NoticeErrors indicates whether the errors collected count towards error/ metrics
+func (t *txnData) NoticeErrors() bool {
+	return t.noticeErrors
+}
+
 // HasErrors indicates whether the transaction had errors.
 func (t *txnData) HasErrors() bool {
 	return len(t.Errors) > 0
 }
 
+// HasExpectedErrors is a special case where the txn has errors but we dont increment error metrics
+func (t *txnData) HasExpectedErrors() bool {
+	return t.expectedErrors
+}
+
 func (t *txnData) time(now time.Time) segmentTime {
 	// Update the stamp before using it so that a 0 stamp can be special.
 	t.stamp++
diff --git a/v3/newrelic/transaction.go b/v3/newrelic/transaction.go
diff --git a/v3/newrelic/txn_trace.go b/v3/newrelic/txn_trace.go

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ func TestStackTrace(t *testing.T) {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`for idx, tc := range testcases {`
`67`		`- data, err := errDataFromError(tc.Error)`
	`67`	`+ data, err := errDataFromError(tc.Error, false)`
`68`	`68`	`if err != nil {`
`69`	`69`	`t.Errorf("testcase %d: got error: %v", idx, err)`
`70`	`70`	`continue`
Original file line number	Diff line number	Diff line change
`@@ -636,7 +636,7 @@ func TestErrorClass(t *testing.T) {`
`636`	`636`	`}`
`637`	`637`
`638`	`638`	`for idx, tc := range testcases {`
`639`		`- data, err := errDataFromError(tc.Error)`
	`639`	`+ data, err := errDataFromError(tc.Error, false)`
`640`	`640`	`if err != nil {`
`641`	`641`	`t.Errorf("testcase %d: got error: %v", idx, err)`
`642`	`642`	`continue`