From 98fa14ce44809d3c6d91eb72e43ca36798c5bc04 Mon Sep 17 00:00:00 2001 From: shubham sharma Date: Sun, 28 Apr 2024 02:03:50 +0530 Subject: [PATCH 1/7] incase of invalid tracer config disabling it instead of returning discard transport --- tracer.go | 8 +++----- tracer_test.go | 14 +------------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/tracer.go b/tracer.go index 1b6271559..f1af2c774 100644 --- a/tracer.go +++ b/tracer.go @@ -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 { @@ -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) + opts.active = false } else { opts.Transport = initialTransport } diff --git a/tracer_test.go b/tracer_test.go index 4537f6348..b30707f58 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -593,21 +593,9 @@ func TestTracerDefaultTransport(t *testing.T) { defer os.Unsetenv("ELASTIC_APM_SERVER_TIMEOUT") // NewTracer returns errors. - tracer, err := apm.NewTracer("", "") + _, err := apm.NewTracer("", "") 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. - 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()) }) } From f24346a14046109fa682cd24601ac4b29ee517ed Mon Sep 17 00:00:00 2001 From: shubham sharma Date: Sun, 28 Apr 2024 12:36:37 +0530 Subject: [PATCH 2/7] The default value of 'active' is true, setting it to 'false' to support the case where invalid tracer config --- tracer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracer.go b/tracer.go index f1af2c774..126d5bfb5 100644 --- a/tracer.go +++ b/tracer.go @@ -312,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.active = false + active = false } else { opts.Transport = initialTransport } From a777f8006f9b9c5abeb667b49480b5d59292a480 Mon Sep 17 00:00:00 2001 From: shubham sharma Date: Wed, 1 May 2024 23:20:45 +0530 Subject: [PATCH 3/7] added a test case to check if tracer is being set to inactive incase of invalid configuration --- tracer_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tracer_test.go b/tracer_test.go index b30707f58..3cf43437a 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -593,9 +593,16 @@ func TestTracerDefaultTransport(t *testing.T) { defer os.Unsetenv("ELASTIC_APM_SERVER_TIMEOUT") // NewTracer returns errors. - _, err := apm.NewTracer("", "") + tracer, err := apm.NewTracer("", "") require.Error(t, err) assert.EqualError(t, err, "failed to parse ELASTIC_APM_SERVER_TIMEOUT: invalid duration never") + + + // Implicitly created Tracers will have Inactive tracer incase of invalid configuration + apm.SetDefaultTracer(nil) + tracer = apm.DefaultTracer() + isActiveTracer :=tracer.Active() + assert.Equal(t,false,isActiveTracer) }) } From 3a4110684f952f733aa95c3b1d317dfd82ebcac5 Mon Sep 17 00:00:00 2001 From: shubham sharma Date: Fri, 3 May 2024 00:15:45 +0530 Subject: [PATCH 4/7] removed unwanted variable and linter fix --- tracer_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tracer_test.go b/tracer_test.go index 3cf43437a..dfade141b 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -598,11 +598,10 @@ func TestTracerDefaultTransport(t *testing.T) { assert.EqualError(t, err, "failed to parse ELASTIC_APM_SERVER_TIMEOUT: invalid duration never") - // Implicitly created Tracers will have Inactive tracer incase of invalid configuration + // Implicitly created Tracers will have Inactive tracer in case of invalid configuration apm.SetDefaultTracer(nil) tracer = apm.DefaultTracer() - isActiveTracer :=tracer.Active() - assert.Equal(t,false,isActiveTracer) + assert.Equal(t, false, tracer.Active()) }) } From da25349d3ff8f56c75b71ca92348dbca65d3a9cf Mon Sep 17 00:00:00 2001 From: dmathieu Date: Fri, 3 May 2024 09:51:56 +0200 Subject: [PATCH 5/7] fix precheck --- tracer_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tracer_test.go b/tracer_test.go index dfade141b..0a32716d6 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -597,7 +597,6 @@ 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 Inactive tracer in case of invalid configuration apm.SetDefaultTracer(nil) tracer = apm.DefaultTracer() From a6e22273b074180981643bee02fa3e438389276a Mon Sep 17 00:00:00 2001 From: shubham sharma Date: Sat, 4 May 2024 23:44:32 +0530 Subject: [PATCH 6/7] ran gofmt and golangci-lint to format tracer.go and tracer_test.go --- tracer_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tracer_test.go b/tracer_test.go index dfade141b..0a32716d6 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -597,7 +597,6 @@ 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 Inactive tracer in case of invalid configuration apm.SetDefaultTracer(nil) tracer = apm.DefaultTracer() From f60debb9a61e7c7829da859e62bf25db5fc37f8c Mon Sep 17 00:00:00 2001 From: dmathieu Date: Mon, 6 May 2024 09:52:32 +0200 Subject: [PATCH 7/7] ensure tracecontext example has a discard tracer --- example_tracecontext_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/example_tracecontext_test.go b/example_tracecontext_test.go index 3554b4e3e..965935b67 100644 --- a/example_tracecontext_test.go +++ b/example_tracecontext_test.go @@ -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},