Skip to content

Commit

Permalink
Merge pull request #611 from iamemilio/expectedErrors
Browse files Browse the repository at this point in the history
Notice Expected Errors
  • Loading branch information
iamemilio committed Dec 14, 2022
2 parents dc72e67 + e6ffdc6 commit d824822
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 61 deletions.
8 changes: 8 additions & 0 deletions v3/examples/server/main.go
Expand Up @@ -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")

Expand Down Expand Up @@ -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))
Expand Down
5 changes: 3 additions & 2 deletions v3/newrelic/errors_from_internal.go
Expand Up @@ -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
Expand Down Expand Up @@ -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"`)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion v3/newrelic/harvest.go
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions v3/newrelic/harvest_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion v3/newrelic/internal_errors_stacktrace_test.go
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion v3/newrelic/internal_errors_test.go
Expand Up @@ -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
Expand Down
29 changes: 18 additions & 11 deletions v3/newrelic/internal_txn.go
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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()))
}

Expand All @@ -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)
Expand All @@ -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 != "",
})
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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
}
Expand All @@ -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 {
Expand Down
10 changes: 9 additions & 1 deletion v3/newrelic/intrinsics.go
Expand Up @@ -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('{')
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions v3/newrelic/metric_names.go
Expand Up @@ -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
Expand Down
48 changes: 31 additions & 17 deletions v3/newrelic/tracing.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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++
Expand Down

0 comments on commit d824822

Please sign in to comment.