Skip to content

Commit

Permalink
[Bugfix] apm.DefaultTracer misbehaves when transport configuration is…
Browse files Browse the repository at this point in the history
… invalid (#1618)

* incase of invalid tracer config disabling it instead of returning discard transport

* The default value of 'active'  is true, setting it to 'false' to support the case where invalid tracer config

* added a test case to check if tracer is being set to inactive incase of invalid configuration

* removed unwanted variable and linter fix

* fix precheck

* ran gofmt and golangci-lint to format tracer.go and tracer_test.go

* ensure tracecontext example has a discard tracer

---------

Co-authored-by: dmathieu <damien.mathieu@elastic.co>
  • Loading branch information
shubhamsharma7867 and dmathieu committed May 6, 2024
1 parent 73ae416 commit 4b23fe9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 14 deletions.
8 changes: 8 additions & 0 deletions example_tracecontext_test.go
Expand Up @@ -23,9 +23,17 @@ import (
"os"

"go.elastic.co/apm/v2"
"go.elastic.co/apm/v2/apmtest"
)

func ExampleTransaction_EnsureParent() {
// When environment variables aren't set, the default tracer is inactive.
// For the sake of this example, we want a discarded tracer instead, so we do
// have trace and span IDs generated.
//
// This setup is now something you should be using in your own code.
apm.SetDefaultTracer(apmtest.NewDiscardTracer())

tx := apm.DefaultTracer().StartTransactionOptions("name", "type", apm.TransactionOptions{
TraceContext: apm.TraceContext{
Trace: apm.TraceID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},
Expand Down
8 changes: 3 additions & 5 deletions tracer.go
Expand Up @@ -53,10 +53,8 @@ var (
// DefaultTracer returns the default global Tracer, set the first time the
// function is called, or after calling SetDefaultTracer(nil).
//
// The default tracer is configured via environment variables, and will always
// be non-nil. If any of the environment variables are invalid, the
// corresponding errors will be logged to stderr and the default values will be
// used instead.
// The default tracer is configured via environment variables, and if the those are invalid
// the tracer will be disabled.
func DefaultTracer() *Tracer {
tracerMu.RLock()
if defaultTracer != nil {
Expand Down Expand Up @@ -314,7 +312,7 @@ func (opts *TracerOptions) initDefaults(continueOnError bool) error {
if opts.Transport == nil {
initialTransport, err := initialTransport(opts.ServiceName, opts.ServiceVersion)
if failed(err) {
opts.Transport = transport.NewDiscardTransport(err)
active = false
} else {
opts.Transport = initialTransport
}
Expand Down
11 changes: 2 additions & 9 deletions tracer_test.go
Expand Up @@ -597,17 +597,10 @@ func TestTracerDefaultTransport(t *testing.T) {
require.Error(t, err)
assert.EqualError(t, err, "failed to parse ELASTIC_APM_SERVER_TIMEOUT: invalid duration never")

// Implicitly created Tracers will have a discard tracer.
// Implicitly created Tracers will have Inactive tracer in case of invalid configuration
apm.SetDefaultTracer(nil)
tracer = apm.DefaultTracer()

tracer.StartTransaction("name", "type").End()
tracer.Flush(nil)
assert.Equal(t, apm.TracerStats{
Errors: apm.TracerStatsErrors{
SendStream: 1,
},
}, tracer.Stats())
assert.Equal(t, false, tracer.Active())
})
}

Expand Down

0 comments on commit 4b23fe9

Please sign in to comment.