diff --git a/CHANGELOG.md b/CHANGELOG.md index c24673e08..aed73577e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased](https://github.com/elastic/apm-agent-go/compare/v1.0.0...master) + - Stop pooling Transaction/Span/Error, introduce internal pooled objects (#319) + ## [v1.0.0](https://github.com/elastic/apm-agent-go/releases/tag/v1.0.0) - Implement v2 intake protocol (#180) diff --git a/docs/api.asciidoc b/docs/api.asciidoc index 55885ec56..1d11f96cc 100644 --- a/docs/api.asciidoc +++ b/docs/api.asciidoc @@ -90,7 +90,7 @@ transaction := apm.DefaultTracer.StartTransactionOptions("GET /", "request", opt ==== `func (*Transaction) End()` End enqueues the transaction for sending to the Elastic APM server. -The Transaction must not be used after this. +The Transaction must not be modified after this. The transaction's duration will be calculated as the amount of time elapsed since the transaction was started until this call. To override @@ -239,7 +239,7 @@ span, ctx := apm.StartSpan(ctx, "SELECT FROM foo", "db.mysql.query") [[span-end]] ==== `func (*Span) End()` -End marks the span as complete; it must not be used after this. +End marks the span as complete. The Span must not be modified after this. The span's duration will be calculated as the amount of time elapsed since the span was started until this call. To override this behaviour, @@ -250,8 +250,8 @@ the span's Duration field may be set before calling End. ==== `func (*Span) Dropped() bool` Dropped indicates whether or not the span is dropped, meaning it will not be reported to -the APM server. Spans are dropped when the created via a nil or non-sampled transaction, -or one whose max spans limit has been reached. +the APM server. Spans are dropped when the created with a nil, ended, or non-sampled +transaction, or one whose max spans limit has been reached. [float] [[span-tracecontext]] @@ -427,21 +427,20 @@ e.Send() [[error-set-transaction]] ==== `func (*Error) SetTransaction(*Transaction)` -SetTransaction associates the error with the given transaction. The transaction's End method must -not yet have been called. +SetTransaction associates the error with the given transaction. [float] [[error-set-span]] ==== `func (*Error) SetSpan(*Span)` SetSpan associates the error with the given span, and the span's transaction. When calling SetSpan, -it is not necessary to also call SetTransaction. The span's End method must not yet have been called. +it is not necessary to also call SetTransaction. [float] [[error-send]] ==== `func (*Error) Send()` -Send enqueues the error for sending to the Elastic APM server. The Error must not be used after this. +Send enqueues the error for sending to the Elastic APM server. [float] [[tracer-recovered]] diff --git a/error.go b/error.go index 6c7c5c989..08809c7a6 100644 --- a/error.go +++ b/error.go @@ -99,9 +99,9 @@ func (t *Tracer) NewErrorLog(r ErrorLogRecord) *Error { // newError returns a new Error associated with the Tracer. func (t *Tracer) newError() *Error { - e, _ := t.errorPool.Get().(*Error) + e, _ := t.errorDataPool.Get().(*ErrorData) if e == nil { - e = &Error{ + e = &ErrorData{ tracer: t, Context: Context{ captureBodyMask: CaptureBodyErrors, @@ -109,11 +109,19 @@ func (t *Tracer) newError() *Error { } } e.Timestamp = time.Now() - return e + return &Error{ErrorData: e} } // Error describes an error occurring in the monitored service. type Error struct { + // ErrorData holds the error data. This field is set to nil when + // the error's Send method is called. + *ErrorData +} + +// ErrorData holds the details for an error, and is embedded inside Error. +// When the error is sent, its ErrorData field will be set to nil. +type ErrorData struct { model model.Error tracer *Tracer stacktrace []stacktrace.Frame @@ -163,42 +171,56 @@ type Error struct { // SetTransaction sets TraceID, TransactionID, and ParentID to the transaction's IDs. // -// This must be called before tx.End(). After SetTransaction returns, e may be sent -// and tx ended in either order. +// SetTransaction has no effect if called with an ended transaction. func (e *Error) SetTransaction(tx *Transaction) { - e.TraceID = tx.traceContext.Trace - e.ParentID = tx.traceContext.Span + tx.mu.RLock() + if !tx.ended() { + e.setTransactionData(tx.TransactionData) + } + tx.mu.RUnlock() +} + +func (e *Error) setTransactionData(td *TransactionData) { + e.TraceID = td.traceContext.Trace + e.ParentID = td.traceContext.Span e.TransactionID = e.ParentID } // SetSpan sets TraceID, TransactionID, and ParentID to the span's IDs. If you call // this, it is not necessary to call SetTransaction. // -// This must be called before s.End(). After SetSpanreturns, e may be sent and e ended -// in either order. +// 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) + } + s.mu.RUnlock() +} + +func (e *Error) setSpanData(s *SpanData) { e.TraceID = s.traceContext.Trace e.ParentID = s.traceContext.Span e.TransactionID = s.transactionID } -func (e *Error) reset() { - *e = Error{ - tracer: e.tracer, - stacktrace: e.stacktrace[:0], - modelStacktrace: e.modelStacktrace[:0], - Context: e.Context, - } - e.Context.reset() - e.tracer.errorPool.Put(e) -} - // Send enqueues the error for sending to the Elastic APM server. -// The Error must not be used after this. +// +// Send will set e.ErrorData to nil, so the error must not be +// modified after Send returns. func (e *Error) Send() { - if e == nil { + if e == nil || e.sent() { return } + e.ErrorData.enqueue() + e.ErrorData = nil +} + +func (e *Error) sent() bool { + return e.ErrorData == nil +} + +func (e *ErrorData) enqueue() { select { case e.tracer.errors <- e: default: @@ -210,7 +232,18 @@ func (e *Error) Send() { } } -func (e *Error) setStacktrace() { +func (e *ErrorData) reset() { + *e = ErrorData{ + tracer: e.tracer, + stacktrace: e.stacktrace[:0], + modelStacktrace: e.modelStacktrace[:0], + Context: e.Context, + } + e.Context.reset() + e.tracer.errorDataPool.Put(e) +} + +func (e *ErrorData) setStacktrace() { if len(e.stacktrace) == 0 { return } @@ -219,7 +252,7 @@ func (e *Error) setStacktrace() { e.model.Exception.Stacktrace = e.modelStacktrace } -func (e *Error) setCulprit() { +func (e *ErrorData) setCulprit() { if e.Culprit != "" { e.model.Culprit = e.Culprit } else if e.modelStacktrace != nil { diff --git a/gocontext.go b/gocontext.go index 6691104c9..86dec19f1 100644 --- a/gocontext.go +++ b/gocontext.go @@ -59,23 +59,31 @@ func StartSpanOptions(ctx context.Context, name, spanType string, opts SpanOptio // from err. The Error.Handled field will be set to true, and a stacktrace // set either from err, or from the caller. // -// If there is no span or transaction in the context, or the transaction -// is not being sampled, CaptureError returns nil. As a convenience, if -// the provided error is nil, then CaptureError will also return nil. +// If there is no span or transaction in the context, CaptureError returns +// nil. As a convenience, if the provided error is nil, then CaptureError +// will also return nil. func CaptureError(ctx context.Context, err error) *Error { if err == nil { return nil } var e *Error if span := SpanFromContext(ctx); span != nil { - e = span.tracer.NewError(err) - e.SetSpan(span) - } else if tx := TransactionFromContext(ctx); tx != nil && tx.Sampled() { - e = tx.tracer.NewError(err) - e.SetTransaction(tx) - } else { - return nil + span.mu.RLock() + if !span.ended() { + e = span.tracer.NewError(err) + e.setSpanData(span.SpanData) + } + 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 != nil { + e.Handled = true } - e.Handled = true return e } diff --git a/gocontext_test.go b/gocontext_test.go new file mode 100644 index 000000000..413484f93 --- /dev/null +++ b/gocontext_test.go @@ -0,0 +1,67 @@ +package apm_test + +import ( + "context" + "sync" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + + "go.elastic.co/apm" + "go.elastic.co/apm/transport/transporttest" +) + +func TestContextStartSpanTransactionEnded(t *testing.T) { + tracer, err := apm.NewTracer("tracer_testing", "") + assert.NoError(t, err) + defer tracer.Close() + tracer.Transport = transporttest.Discard + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 1000; i++ { + tx := tracer.StartTransaction("name", "type") + ctx := apm.ContextWithTransaction(context.Background(), tx) + tx.End() + apm.CaptureError(ctx, errors.New("boom")).Send() + span, _ := apm.StartSpan(ctx, "name", "type") + assert.True(t, span.Dropped()) + span.End() + } + }() + } + tracer.Flush(nil) + wg.Wait() +} + +func TestContextStartSpanSpanEnded(t *testing.T) { + tracer, err := apm.NewTracer("tracer_testing", "") + assert.NoError(t, err) + defer tracer.Close() + tracer.Transport = transporttest.Discard + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 1000; i++ { + tx := tracer.StartTransaction("name", "type") + ctx := apm.ContextWithTransaction(context.Background(), tx) + span1, ctx := apm.StartSpan(ctx, "name", "type") + span1.End() + apm.CaptureError(ctx, errors.New("boom")).Send() + span2, _ := apm.StartSpan(ctx, "name", "type") + assert.True(t, span2.Dropped()) + span2.End() + tx.End() + } + }() + } + tracer.Flush(nil) + wg.Wait() +} diff --git a/modelwriter.go b/modelwriter.go index feb078645..e5453c576 100644 --- a/modelwriter.go +++ b/modelwriter.go @@ -27,7 +27,7 @@ type modelWriter struct { } // writeTransaction encodes tx as JSON to the buffer, and then resets tx. -func (w *modelWriter) writeTransaction(tx *Transaction) { +func (w *modelWriter) writeTransaction(tx *TransactionData) { var modelTx model.Transaction w.buildModelTransaction(&modelTx, tx) w.json.RawString(`{"transaction":`) @@ -39,7 +39,7 @@ func (w *modelWriter) writeTransaction(tx *Transaction) { } // writeSpan encodes s as JSON to the buffer, and then resets s. -func (w *modelWriter) writeSpan(s *Span) { +func (w *modelWriter) writeSpan(s *SpanData) { var modelSpan model.Span w.buildModelSpan(&modelSpan, s) w.json.RawString(`{"span":`) @@ -51,7 +51,7 @@ func (w *modelWriter) writeSpan(s *Span) { } // writeError encodes e as JSON to the buffer, and then resets e. -func (w *modelWriter) writeError(e *Error) { +func (w *modelWriter) writeError(e *ErrorData) { w.buildModelError(e) w.json.RawString(`{"error":`) e.model.MarshalFastJSON(&w.json) @@ -73,7 +73,7 @@ func (w *modelWriter) writeMetrics(m *Metrics) { m.reset() } -func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transaction) { +func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *TransactionData) { out.ID = model.SpanID(tx.traceContext.Span) out.TraceID = model.TraceID(tx.traceContext.Trace) out.ParentID = model.SpanID(tx.parentSpan) @@ -86,7 +86,7 @@ func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transact out.SpanCount.Started = tx.spansCreated out.SpanCount.Dropped = tx.spansDropped - if !tx.Sampled() { + if !tx.traceContext.Options.Recorded() { out.Sampled = ¬Sampled } @@ -101,7 +101,7 @@ func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transact } } -func (w *modelWriter) buildModelSpan(out *model.Span, span *Span) { +func (w *modelWriter) buildModelSpan(out *model.Span, span *SpanData) { w.modelStacktrace = w.modelStacktrace[:0] out.ID = model.SpanID(span.traceContext.Span) out.TraceID = model.TraceID(span.traceContext.Trace) @@ -119,7 +119,7 @@ func (w *modelWriter) buildModelSpan(out *model.Span, span *Span) { w.setStacktraceContext(out.Stacktrace) } -func (w *modelWriter) buildModelError(e *Error) { +func (w *modelWriter) buildModelError(e *ErrorData) { // TODO(axw) move the model type outside of Error e.model.ID = model.TraceID(e.ID) e.model.TraceID = model.TraceID(e.TraceID) diff --git a/module/apmgocql/observer.go b/module/apmgocql/observer.go index ab37d3665..47cc08b1d 100644 --- a/module/apmgocql/observer.go +++ b/module/apmgocql/observer.go @@ -79,12 +79,11 @@ func (o *Observer) ObserveQuery(ctx context.Context, query gocql.ObservedQuery) Instance: query.Keyspace, Statement: query.Statement, }) - span.End() - if e := apm.CaptureError(ctx, query.Err); e != nil { e.Timestamp = query.End e.Send() } + span.End() } type options struct { diff --git a/module/apmhttp/client.go b/module/apmhttp/client.go index d22478476..64bf4f2a8 100644 --- a/module/apmhttp/client.go +++ b/module/apmhttp/client.go @@ -74,7 +74,7 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { req = &reqCopy traceContext := tx.TraceContext() - if !tx.Sampled() { + if !traceContext.Options.Recorded() { req.Header.Set(TraceparentHeader, FormatTraceparentHeader(traceContext)) return r.r.RoundTrip(req) } diff --git a/module/apmsql/conn.go b/module/apmsql/conn.go index da8a7b6c5..c568869f7 100644 --- a/module/apmsql/conn.go +++ b/module/apmsql/conn.go @@ -68,10 +68,10 @@ func (c *conn) finishSpan(ctx context.Context, span *apm.Span, resultError *erro // in check. return } - span.End() if e := apm.CaptureError(ctx, *resultError); e != nil { e.Send() } + span.End() } func (c *conn) Ping(ctx context.Context) (resultError error) { diff --git a/span.go b/span.go index cfd4b2d0e..e9ed0d087 100644 --- a/span.go +++ b/span.go @@ -9,17 +9,17 @@ import ( "go.elastic.co/apm/stacktrace" ) -// droppedSpanPool holds *Spans which are used when the span -// is created for a nil or non-sampled Transaction, or one -// whose max spans limit has been reached. -var droppedSpanPool sync.Pool +// droppedSpanDataPool holds *SpanData which are used when the span +// is created for a nil or non-sampled Transaction, or one whose max +// spans limit has been reached. +var droppedSpanDataPool sync.Pool // StartSpan starts and returns a new Span within the transaction, // with the specified name, type, and optional parent span, and // with the start time set to the current time. // -// StartSpan always returns a non-nil Span. Its End method must -// be called when the span completes. +// StartSpan always returns a non-nil Span, with a non-nil SpanData +// field. Its End method must be called when the span completes. // // StartSpan is equivalent to calling StartSpanOptions with // SpanOptions.Parent set to the trace context of parent if @@ -27,7 +27,13 @@ var droppedSpanPool sync.Pool func (tx *Transaction) StartSpan(name, spanType string, parent *Span) *Span { var parentTraceContext TraceContext if parent != nil { + parent.mu.RLock() + if parent.ended() { + parent.mu.RUnlock() + return newDroppedSpan() + } parentTraceContext = parent.TraceContext() + parent.mu.RUnlock() } return tx.StartSpanOptions(name, spanType, SpanOptions{ Parent: parentTraceContext, @@ -40,13 +46,24 @@ func (tx *Transaction) StartSpan(name, spanType string, parent *Span) *Span { // StartSpan always returns a non-nil Span. Its End method must // be called when the span completes. func (tx *Transaction) StartSpanOptions(name, spanType string, opts SpanOptions) *Span { - if tx == nil || !tx.Sampled() { + if tx == nil { + return newDroppedSpan() + } + + // Prevent tx from being ended while we're starting a span. + tx.mu.RLock() + defer tx.mu.RUnlock() + + if tx.ended() || !tx.traceContext.Options.Recorded() { return newDroppedSpan() } - tx.mu.Lock() + + // Guard access to spansCreated, spansDropped, and rand. + tx.TransactionData.mu.Lock() + defer tx.TransactionData.mu.Unlock() + if tx.maxSpans > 0 && tx.spansCreated >= tx.maxSpans { tx.spansDropped++ - tx.mu.Unlock() return newDroppedSpan() } transactionID := tx.traceContext.Span @@ -65,7 +82,6 @@ func (tx *Transaction) StartSpanOptions(name, spanType string, opts SpanOptions) binary.LittleEndian.PutUint64(span.traceContext.Span[:], tx.rand.Uint64()) span.stackFramesMinDuration = tx.spanFramesMinDuration tx.spansCreated++ - tx.mu.Unlock() return span } @@ -100,14 +116,35 @@ func (t *Tracer) StartSpan(name, spanType string, transactionID SpanID, opts Spa return span } +// SpanOptions holds options for Transaction.StartSpanOptions and Tracer.StartSpan. +type SpanOptions struct { + // Parent, if non-zero, holds the trace context of the parent span. + Parent TraceContext + + // Start is the start time of the span. If this has the zero value, + // time.Now() will be used instead. + // + // When a span is created using Transaction.StartSpanOptions, the + // span timestamp is internally calculated relative to the transaction + // timestamp. + // + // When Tracer.StartSpan is used, this timestamp should be pre-calculated + // as relative from the transaction start time, i.e. by calculating the + // time elapsed since the transaction started, and adding that to the + // transaction timestamp. Calculating the timstamp in this way will ensure + // monotonicity of events within a transaction. + Start time.Time +} + func (t *Tracer) startSpan(name, spanType string, transactionID SpanID, opts SpanOptions) *Span { - span, _ := t.spanPool.Get().(*Span) - if span == nil { - span = &Span{ + sd, _ := t.spanDataPool.Get().(*SpanData) + if sd == nil { + sd = &SpanData{ tracer: t, Duration: -1, } } + span := &Span{SpanData: sd} span.Name = name span.Type = spanType span.traceContext = opts.Parent @@ -117,44 +154,33 @@ func (t *Tracer) startSpan(name, spanType string, transactionID SpanID, opts Spa return span } -// Span describes an operation within a transaction. -type Span struct { - tracer *Tracer // nil if span is dropped - traceContext TraceContext - parentID SpanID - transactionID SpanID - stackFramesMinDuration time.Duration - timestamp time.Time - - Name string - Type string - Duration time.Duration - Context SpanContext - - stacktrace []stacktrace.Frame -} - func newDroppedSpan() *Span { - span, _ := droppedSpanPool.Get().(*Span) + span, _ := droppedSpanDataPool.Get().(*Span) if span == nil { - span = &Span{} + span = &Span{SpanData: &SpanData{}} } return span } -func (s *Span) reset() { - *s = Span{ - tracer: s.tracer, - Context: s.Context, - Duration: -1, - stacktrace: s.stacktrace[:0], - } - s.Context.reset() - s.tracer.spanPool.Put(s) +// Span describes an operation within a transaction. +type Span struct { + mu sync.RWMutex + + // SpanData holds the span data. This field is set to nil when + // the span's End method is called. + *SpanData } // TraceContext returns the span's TraceContext. func (s *Span) TraceContext() TraceContext { + if s == nil { + return TraceContext{} + } + s.mu.RLock() + defer s.mu.RUnlock() + if s.ended() { + return TraceContext{} + } return s.traceContext } @@ -165,7 +191,7 @@ func (s *Span) SetStacktrace(skip int) { if s.Dropped() { return } - s.stacktrace = stacktrace.AppendStacktrace(s.stacktrace[:0], skip+1, -1) + s.SpanData.setStacktrace(skip + 1) } // Dropped indicates whether or not the span is dropped, meaning it will not @@ -176,7 +202,13 @@ func (s *Span) SetStacktrace(skip int) { // Dropped may be used to avoid any expensive computation required to set // the span's context. func (s *Span) Dropped() bool { - return s.tracer == nil + if s == nil { + return true + } + s.mu.RLock() + dropped := s.ended() || s.dropped() + s.mu.RUnlock() + return dropped } // End marks the s as being complete; s must not be used after this. @@ -184,20 +216,64 @@ func (s *Span) Dropped() bool { // If s.Duration has not been set, End will set it to the elapsed time // since the span's start time. func (s *Span) End() { - if s.Dropped() { - droppedSpanPool.Put(s) + s.mu.Lock() + defer s.mu.Unlock() + if s.ended() || s.dropped() { + droppedSpanDataPool.Put(s.SpanData) return } if s.Duration < 0 { s.Duration = time.Since(s.timestamp) } if len(s.stacktrace) == 0 && s.Duration >= s.stackFramesMinDuration { - s.SetStacktrace(1) + s.setStacktrace(1) } - s.enqueue() + s.SpanData.enqueue() + s.SpanData = nil +} + +func (s *Span) ended() bool { + return s.SpanData == nil } -func (s *Span) enqueue() { +// SpanData holds the details for a span, and is embedded inside Span. +// When a span is ended or discarded, its SpanData field will be set +// to nil. +type SpanData struct { + tracer *Tracer // nil if span is dropped + traceContext TraceContext + parentID SpanID + transactionID SpanID + stackFramesMinDuration time.Duration + timestamp time.Time + + // Name holds the span name, initialized with the value passed to StartSpan. + Name string + + // Type holds the span type, initialized with the value passed to StartSpan. + Type string + + // Duration holds the span duration, initialized to -1. + // + // If you do not update Duration, calling Span.End will calculate the + // duration based on the elapsed time since the span's start time. + Duration time.Duration + + // Context describes the context in which span occurs. + Context SpanContext + + stacktrace []stacktrace.Frame +} + +func (s *SpanData) setStacktrace(skip int) { + s.stacktrace = stacktrace.AppendStacktrace(s.stacktrace[:0], skip+1, -1) +} + +func (s *SpanData) dropped() bool { + return s.tracer == nil +} + +func (s *SpanData) enqueue() { select { case s.tracer.spans <- s: default: @@ -209,22 +285,13 @@ func (s *Span) enqueue() { } } -// SpanOptions holds options for Transaction.StartSpanOptions and Tracer.StartSpan. -type SpanOptions struct { - // Parent, if non-zero, holds the trace context of the parent span. - Parent TraceContext - - // Start is the start time of the span. If this has the zero value, - // time.Now() will be used instead. - // - // When a span is created using Transaction.StartSpanOptions, the - // span timestamp is internally calculated relative to the transaction - // timestamp. - // - // When Tracer.StartSpan is used, this timestamp should be pre-calculated - // as relative from the transaction start time, i.e. by calculating the - // time elapsed since the transaction started, and adding that to the - // transaction timestamp. Calculating the timstamp in this way will ensure - // monotonicity of events within a transaction. - Start time.Time +func (s *SpanData) reset() { + *s = SpanData{ + tracer: s.tracer, + Context: s.Context, + Duration: -1, + stacktrace: s.stacktrace[:0], + } + s.Context.reset() + s.tracer.spanDataPool.Put(s) } diff --git a/tracer.go b/tracer.go index 12779aa1b..acb1b4705 100644 --- a/tracer.go +++ b/tracer.go @@ -179,9 +179,9 @@ type Tracer struct { forceFlush chan chan<- struct{} forceSendMetrics chan chan<- struct{} configCommands chan tracerConfigCommand - transactions chan *Transaction - spans chan *Span - errors chan *Error + transactions chan *TransactionData + spans chan *SpanData + errors chan *ErrorData statsMu sync.Mutex stats TracerStats @@ -198,9 +198,9 @@ type Tracer struct { captureBodyMu sync.RWMutex captureBody CaptureBodyMode - errorPool sync.Pool - spanPool sync.Pool - transactionPool sync.Pool + errorDataPool sync.Pool + spanDataPool sync.Pool + transactionDataPool sync.Pool } // NewTracer returns a new Tracer, using the default transport, @@ -235,9 +235,9 @@ func newTracer(opts options) *Tracer { forceFlush: make(chan chan<- struct{}), forceSendMetrics: make(chan chan<- struct{}), configCommands: make(chan tracerConfigCommand), - transactions: make(chan *Transaction, transactionsChannelCap), - spans: make(chan *Span, spansChannelCap), - errors: make(chan *Error, errorsChannelCap), + transactions: make(chan *TransactionData, transactionsChannelCap), + spans: make(chan *SpanData, spansChannelCap), + errors: make(chan *ErrorData, errorsChannelCap), maxSpans: opts.maxSpans, sampler: opts.sampler, captureBody: opts.captureBody, diff --git a/transaction.go b/transaction.go index ab0e9da26..5c92610fd 100644 --- a/transaction.go +++ b/transaction.go @@ -19,9 +19,9 @@ func (t *Tracer) StartTransaction(name, transactionType string) *Transaction { // StartTransactionOptions returns a new Transaction with the // specified name, type, and options. func (t *Tracer) StartTransactionOptions(name, transactionType string, opts TransactionOptions) *Transaction { - tx, _ := t.transactionPool.Get().(*Transaction) - if tx == nil { - tx = &Transaction{ + td, _ := t.transactionDataPool.Get().(*TransactionData) + if td == nil { + td = &TransactionData{ tracer: t, Duration: -1, Context: Context{ @@ -32,8 +32,10 @@ func (t *Tracer) StartTransactionOptions(name, transactionType string, opts Tran if err := binary.Read(cryptorand.Reader, binary.LittleEndian, &seed); err != nil { seed = time.Now().UnixNano() } - tx.rand = rand.New(rand.NewSource(seed)) + td.rand = rand.New(rand.NewSource(seed)) } + tx := &Transaction{TransactionData: td} + tx.Name = name tx.Type = transactionType @@ -84,44 +86,24 @@ func (t *Tracer) StartTransactionOptions(name, transactionType string, opts Tran return tx } -// Transaction describes an event occurring in the monitored service. -type Transaction struct { - Name string - Type string - Duration time.Duration - Context Context - Result string - traceContext TraceContext - timestamp time.Time - - tracer *Tracer - maxSpans int - spanFramesMinDuration time.Duration +// TransactionOptions holds options for Tracer.StartTransactionOptions. +type TransactionOptions struct { + // TraceContext holds the TraceContext for a new transaction. If this is + // zero, a new trace will be started. + TraceContext TraceContext - mu sync.Mutex - parentSpan SpanID - spansCreated int - spansDropped int - rand *rand.Rand // for ID generation + // Start is the start time of the transaction. If this has the + // zero value, time.Now() will be used instead. + Start time.Time } -// reset resets the Transaction back to its zero state and places it back -// into the transaction pool. -func (tx *Transaction) reset() { - *tx = Transaction{ - tracer: tx.tracer, - Context: tx.Context, - Duration: -1, - rand: tx.rand, - } - tx.Context.reset() - tx.tracer.transactionPool.Put(tx) -} +// Transaction describes an event occurring in the monitored service. +type Transaction struct { + mu sync.RWMutex -// Discard discards a previously started transaction. The Transaction -// must not be used after this. -func (tx *Transaction) Discard() { - tx.reset() + // TransactionData holds the transaction data. This field is set to + // nil when either of the transaction's End or Discard methods are called. + *TransactionData } // Sampled reports whether or not the transaction is sampled. @@ -129,17 +111,28 @@ func (tx *Transaction) Sampled() bool { if tx == nil { return false } + tx.mu.RLock() + defer tx.mu.RUnlock() + if tx.ended() { + return false + } return tx.traceContext.Options.Recorded() } // TraceContext returns the transaction's TraceContext. // // The resulting TraceContext's Span field holds the transaction's ID. -// If tx is nil, a zero (invalid) TraceContext is returned. +// If tx is nil, or has already been ended, a zero (invalid) TraceContext +// is returned. func (tx *Transaction) TraceContext() TraceContext { if tx == nil { return TraceContext{} } + tx.mu.RLock() + defer tx.mu.RUnlock() + if tx.ended() { + return TraceContext{} + } return tx.traceContext } @@ -154,7 +147,7 @@ func (tx *Transaction) EnsureParent() SpanID { if tx == nil { return SpanID{} } - tx.mu.Lock() + tx.TransactionData.mu.Lock() if tx.parentSpan.isZero() { // parentSpan can only be zero if tx is a root transaction // for which GenerateParentTraceContext() has not previously @@ -163,41 +156,110 @@ func (tx *Transaction) EnsureParent() SpanID { // transaction ID. copy(tx.parentSpan[:], tx.traceContext.Trace[8:]) } - tx.mu.Unlock() + tx.TransactionData.mu.Unlock() return tx.parentSpan } -// End enqueues tx for sending to the Elastic APM server; tx must not -// be used after this. +// Discard discards a previously started transaction. +// +// Calling Discard will set tx's TransactionData field to nil, so callers must +// ensure tx is not updated after Discard returns. +func (tx *Transaction) Discard() { + tx.mu.Lock() + defer tx.mu.Unlock() + if tx.ended() { + return + } + tx.TransactionData.reset() + tx.TransactionData = nil +} + +// End enqueues tx for sending to the Elastic APM server. +// +// Calling End will set tx's TransactionData field to nil, so callers +// must ensure tx is not updated after End returns. // // If tx.Duration has not been set, End will set it to the elapsed time // since the transaction's start time. func (tx *Transaction) End() { + tx.mu.Lock() + defer tx.mu.Unlock() + if tx.ended() { + return + } if tx.Duration < 0 { tx.Duration = time.Since(tx.timestamp) } - tx.enqueue() + tx.TransactionData.enqueue() + tx.TransactionData = nil } -func (tx *Transaction) enqueue() { +// ended reports whether or not End or Discard has been called. +// +// This must be called with tx.mu held for reading. +func (tx *Transaction) ended() bool { + return tx.TransactionData == nil +} + +// TransactionData holds the details for a transaction, and is embedded +// inside Transaction. When a transaction is ended, its TransactionData +// field will be set to nil. +type TransactionData struct { + // Name holds the transaction name, initialized with the value + // passed to StartTransaction. + Name string + + // Type holds the transaction type, initialized with the value + // passed to StartTransaction. + Type string + + // Duration holds the transaction duration, initialized to -1. + // + // If you do not update Duration, calling Transaction.End will + // calculate the duration based on the elapsed time since the + // transaction's start time. + Duration time.Duration + + // Context describes the context in which the transaction occurs. + Context Context + + // Result holds the transaction result. + Result string + + tracer *Tracer + maxSpans int + spanFramesMinDuration time.Duration + timestamp time.Time + traceContext TraceContext + + mu sync.Mutex + parentSpan SpanID + spansCreated int + spansDropped int + rand *rand.Rand // for ID generation +} + +func (td *TransactionData) enqueue() { select { - case tx.tracer.transactions <- tx: + case td.tracer.transactions <- td: default: // Enqueuing a transaction should never block. - tx.tracer.statsMu.Lock() - tx.tracer.stats.TransactionsDropped++ - tx.tracer.statsMu.Unlock() - tx.reset() + td.tracer.statsMu.Lock() + td.tracer.stats.TransactionsDropped++ + td.tracer.statsMu.Unlock() + td.reset() } } -// TransactionOptions holds options for Tracer.StartTransactionOptions. -type TransactionOptions struct { - // TraceContext holds the TraceContext for a new transaction. If this is - // zero, a new trace will be started. - TraceContext TraceContext - - // Start is the start time of the transaction. If this has the - // zero value, time.Now() will be used instead. - Start time.Time +// reset resets the Transaction back to its zero state and places it back +// into the transaction pool. +func (td *TransactionData) reset() { + *td = TransactionData{ + tracer: td.tracer, + Context: td.Context, + Duration: -1, + rand: td.rand, + } + td.Context.reset() + td.tracer.transactionDataPool.Put(td) }