From ab532dd7286f597ae8037f41043a736fee7dad8a Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Wed, 16 Jan 2019 14:50:08 +0800 Subject: [PATCH] apm: record transaction type on errors When an error is captured within a sampled transaction, record the transaction type on the error. The transaction type is taken at the time the CaptureError function is called, or when Error.SetTransaction is called. --- CHANGELOG.md | 1 + error.go | 57 +++++++++++++++++++++++++++++--------------------- error_test.go | 8 +++++++ gocontext.go | 35 ++++++++++++++++++------------- modelwriter.go | 3 +++ 5 files changed, 65 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 830dff45c..c3aad8e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Introduce `ELASTIC_APM_CAPTURE_HEADERS` to control HTTP header capture (#418) - module/apmzap: introduce zap log correlation and exception-tracking hook (#426) - type Error implements error interface (#399) + - Add "transaction.type" to errors (#433) ## [v1.1.3](https://github.com/elastic/apm-agent-go/releases/tag/v1.1.3) diff --git a/error.go b/error.go index 03c583f6b..9ba0e4ec3 100644 --- a/error.go +++ b/error.go @@ -172,6 +172,7 @@ type ErrorData struct { exception exceptionData log ErrorLogRecord transactionSampled bool + transactionType string // exceptionStacktraceFrames holds the number of stacktrace // frames for the exception; stacktrace may hold frames for @@ -219,17 +220,6 @@ type ErrorData struct { Context Context } -// SetTransaction sets TraceID, TransactionID, and ParentID to the transaction's IDs. -// -// SetTransaction has no effect if called with an ended transaction. -func (e *Error) SetTransaction(tx *Transaction) { - tx.mu.RLock() - if !tx.ended() { - e.setTransactionData(tx.TransactionData) - } - tx.mu.RUnlock() -} - // Cause returns original error assigned to Error, nil if Error or Error.cause is nil. // https://godoc.org/github.com/pkg/errors#Cause func (e *Error) Cause() error { @@ -248,30 +238,49 @@ func (e *Error) Error() string { return "[EMPTY]" } -func (e *Error) setTransactionData(td *TransactionData) { - e.TraceID = td.traceContext.Trace - e.ParentID = td.traceContext.Span - e.TransactionID = e.ParentID - e.transactionSampled = td.traceContext.Options.Recorded() +// SetTransaction sets TraceID, TransactionID, and ParentID to the transaction's +// IDs, and records the transaction's Type and whether or not it was sampled. +// +// SetTransaction has no effect if called with an ended transaction. +func (e *Error) SetTransaction(tx *Transaction) { + tx.mu.RLock() + if !tx.ended() { + e.setSpanData(tx.TransactionData, nil) + } + tx.mu.RUnlock() } -// SetSpan sets TraceID, TransactionID, and ParentID to the span's IDs. If you call -// this, it is not necessary to call SetTransaction. +// SetSpan sets TraceID, TransactionID, and ParentID to the span's IDs. +// +// If you call both SetTransaction and SetSpan, SetSpan must be called second +// in order to set the error's ParentID correctly. When calling SetSpan, it is +// only necessary to call SetTransaction in order to set the error's transaction +// type. // // SetSpan has no effect if called with an ended span. func (e *Error) SetSpan(s *Span) { s.mu.RLock() if !s.ended() { - e.setSpanData(s.SpanData) + e.setSpanData(nil, s.SpanData) } s.mu.RUnlock() } -func (e *Error) setSpanData(s *SpanData) { - e.TraceID = s.traceContext.Trace - e.ParentID = s.traceContext.Span - e.TransactionID = s.transactionID - e.transactionSampled = true // by virtue of there being a span +func (e *Error) setSpanData(td *TransactionData, sd *SpanData) { + if sd != nil { + e.TraceID = sd.traceContext.Trace + e.ParentID = sd.traceContext.Span + e.TransactionID = sd.transactionID + e.transactionSampled = true // by virtue of there being a span + } else if td != nil { + e.TraceID = td.traceContext.Trace + e.ParentID = td.traceContext.Span + e.TransactionID = e.ParentID + e.transactionSampled = td.traceContext.Options.Recorded() + } + if e.transactionSampled && td != nil { + e.transactionType = td.Type + } } // Send enqueues the error for sending to the Elastic APM server. diff --git a/error_test.go b/error_test.go index 7335e9960..f90d63ad9 100644 --- a/error_test.go +++ b/error_test.go @@ -184,6 +184,7 @@ func TestErrorNilError(t *testing.T) { func TestErrorTransactionSampled(t *testing.T) { _, _, errors := apmtest.WithTransaction(func(ctx context.Context) { + apm.TransactionFromContext(ctx).Type = "foo" apm.CaptureError(ctx, errors.New("boom")).Send() span, ctx := apm.StartSpan(ctx, "name", "type") @@ -192,6 +193,8 @@ func TestErrorTransactionSampled(t *testing.T) { }) assertErrorTransactionSampled(t, errors[0], true) assertErrorTransactionSampled(t, errors[1], true) + assert.Equal(t, "foo", errors[0].Transaction.Type) + assert.Equal(t, "foo", errors[1].Transaction.Type) } func TestErrorTransactionNotSampled(t *testing.T) { @@ -222,6 +225,11 @@ func TestErrorTransactionSampledNoTransaction(t *testing.T) { func assertErrorTransactionSampled(t *testing.T, e model.Error, sampled bool) { assert.Equal(t, &sampled, e.Transaction.Sampled) + if sampled { + assert.NotEmpty(t, e.Transaction.Type) + } else { + assert.Empty(t, e.Transaction.Type) + } } func makeError(msg string) error { diff --git a/gocontext.go b/gocontext.go index 226139e0c..4780f438b 100644 --- a/gocontext.go +++ b/gocontext.go @@ -88,27 +88,32 @@ func CaptureError(ctx context.Context, err error) *Error { if err == nil { return nil } - var e = &Error{ - cause: err, - err: err.Error(), + + var tracer *Tracer + var txData *TransactionData + var spanData *SpanData + if tx := TransactionFromContext(ctx); tx != nil { + tx.mu.RLock() + defer tx.mu.RUnlock() + if !tx.ended() { + txData = tx.TransactionData + tracer = tx.tracer + } } if span := SpanFromContext(ctx); span != nil { span.mu.RLock() + defer span.mu.RUnlock() if !span.ended() { - e = span.tracer.NewError(err) - e.setSpanData(span.SpanData) + spanData = span.SpanData + tracer = span.tracer } - span.mu.RUnlock() - } else if tx := TransactionFromContext(ctx); tx != nil { - tx.mu.RLock() - if !tx.ended() { - e = tx.tracer.NewError(err) - e.setTransactionData(tx.TransactionData) - } - tx.mu.RUnlock() } - if e.ErrorData != nil { - e.Handled = true + if tracer == nil { + return &Error{cause: err, err: err.Error()} } + + e := tracer.NewError(err) + e.Handled = true + e.setSpanData(txData, spanData) return e } diff --git a/modelwriter.go b/modelwriter.go index eaca72dee..65bac54a1 100644 --- a/modelwriter.go +++ b/modelwriter.go @@ -154,6 +154,9 @@ func (w *modelWriter) buildModelError(out *model.Error, e *ErrorData) { if !e.TransactionID.isZero() { out.Transaction.Sampled = &e.transactionSampled + if e.transactionSampled { + out.Transaction.Type = e.transactionType + } } w.modelStacktrace = w.modelStacktrace[:0]