From a0fa828a0454d4ee4f6d10c1e905675dd0deecfa Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Wed, 26 Feb 2020 16:47:30 +0100 Subject: [PATCH 1/3] Enables optional stacktrace tag in http and sql instrumentation. --- agent/agent.go | 3 +++ env/vars.go | 2 ++ errors/handler.go | 34 +++++++++++++++++++++++++++---- instrumentation/nethttp/client.go | 8 ++++++++ instrumentation/nethttp/patch.go | 8 ++++++++ instrumentation/sql/driver.go | 21 +++++++++++++++++-- instrumentation/tracer.go | 19 +++++++++++++++-- 7 files changed, 87 insertions(+), 8 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 3c85a824..08c1f23f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -292,10 +292,12 @@ func NewAgent(options ...Option) (*Agent, error) { agent.metadata[tags.Dependencies] = getDependencyMap() // Expand '~' in source root + var sourceRoot string if sRoot, ok := agent.metadata[tags.SourceRoot]; ok { cSRoot := sRoot.(string) cSRoot = filepath.Clean(cSRoot) if sRootEx, err := homedir.Expand(cSRoot); err == nil { + sourceRoot = sRootEx agent.metadata[tags.SourceRoot] = sRootEx } } @@ -339,6 +341,7 @@ func NewAgent(options ...Option) (*Agent, error) { }) instrumentation.SetTracer(agent.tracer) instrumentation.SetLogger(agent.logger) + instrumentation.SetSourceRoot(sourceRoot) if agent.setGlobalTracer || env.ScopeTracerGlobal.Value { opentracing.SetGlobalTracer(agent.Tracer()) } diff --git a/env/vars.go b/env/vars.go index 2b202cb6..bd726a20 100644 --- a/env/vars.go +++ b/env/vars.go @@ -20,5 +20,7 @@ var ( ScopeConfiguration = newSliceEnvVar([]string{tags.PlatformName, tags.PlatformArchitecture, tags.GoVersion}, "SCOPE_CONFIGURATION") ScopeMetadata = newMapEnvVar(nil, "SCOPE_METADATA") ScopeInstrumentationHttpPayloads = newBooleanEnvVar(false, "SCOPE_INSTRUMENTATION_HTTP_PAYLOADS") + ScopeInstrumentationHttpStacktrace = newBooleanEnvVar(false, "SCOPE_INSTRUMENTATION_HTTP_STACKTRACE") ScopeInstrumentationDbStatementValues = newBooleanEnvVar(false, "SCOPE_INSTRUMENTATION_DB_STATEMENT_VALUES") + ScopeInstrumentationDbStacktrace = newBooleanEnvVar(false, "SCOPE_INSTRUMENTATION_DB_STACKTRACE") ) diff --git a/errors/handler.go b/errors/handler.go index 6b20a8ce..c09836b2 100644 --- a/errors/handler.go +++ b/errors/handler.go @@ -2,6 +2,7 @@ package errors import ( "fmt" + "path" "strings" "time" @@ -9,6 +10,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/log" + "go.undefinedlabs.com/scopeagent/instrumentation" "go.undefinedlabs.com/scopeagent/tracer" ) @@ -57,7 +59,8 @@ func LogErrorInRawSpan(rawSpan *tracer.RawSpan, err **errors.Error) { } // Gets the current stack frames array -func GetCurrentStackFrames(skip int) []StackFrames { +func getCurrentStackFrames(skip int) []StackFrames { + sourceRoot := instrumentation.GetSourceRoot() skip = skip + 1 err := errors.New(nil) errStack := err.StackFrames() @@ -65,20 +68,43 @@ func GetCurrentStackFrames(skip int) []StackFrames { if nLength < 0 { return nil } - stackFrames := make([]StackFrames, nLength) + stackFrames := make([]StackFrames, 0) for idx, frame := range errStack { if idx >= skip { - stackFrames[idx-skip] = StackFrames{ + if len(stackFrames) == 0 { + // We ensure that the first frame of the stacktrace should be inside the source.root + dir := path.Dir(frame.File) + if strings.Index(dir, sourceRoot) == -1 { + continue + } + } + stackFrames = append(stackFrames, StackFrames{ File: frame.File, LineNumber: frame.LineNumber, Name: frame.Name, Package: frame.Package, - } + }) } } return stackFrames } +// Gets the current stacktrace +func GetCurrentStackTrace(skip int) map[string]interface{} { + var exFrames []map[string]interface{} + for _, frame := range getCurrentStackFrames(skip + 1) { + exFrames = append(exFrames, map[string]interface{}{ + "name": frame.Name, + "module": frame.Package, + "file": frame.File, + "line": frame.LineNumber, + }) + } + return map[string]interface{}{ + "frames": exFrames, + } +} + // Get the current error with the fixed stacktrace func GetCurrentError(recoverData interface{}) *errors.Error { return errors.Wrap(recoverData, 1) diff --git a/instrumentation/nethttp/client.go b/instrumentation/nethttp/client.go index 1f72ae9c..274a5b7a 100644 --- a/instrumentation/nethttp/client.go +++ b/instrumentation/nethttp/client.go @@ -14,6 +14,7 @@ import ( "github.com/opentracing/opentracing-go/ext" "github.com/opentracing/opentracing-go/log" + scopeerrors "go.undefinedlabs.com/scopeagent/errors" "go.undefinedlabs.com/scopeagent/instrumentation" ) @@ -35,6 +36,9 @@ type Transport struct { // Enable payload instrumentation PayloadInstrumentation bool + + // Enable stacktrace + Stacktrace bool } type clientOptions struct { @@ -178,6 +182,10 @@ func (t *Transport) doRoundTrip(req *http.Request) (*http.Response, error) { tracer.start(req) + if t.Stacktrace { + tracer.sp.SetTag("stacktrace", scopeerrors.GetCurrentStackTrace(2)) + } + ext.HTTPMethod.Set(tracer.sp, req.Method) ext.HTTPUrl.Set(tracer.sp, req.URL.String()) tracer.opts.spanObserver(tracer.sp, req) diff --git a/instrumentation/nethttp/patch.go b/instrumentation/nethttp/patch.go index ac0359f5..8b39fb37 100644 --- a/instrumentation/nethttp/patch.go +++ b/instrumentation/nethttp/patch.go @@ -17,6 +17,13 @@ func WithPayloadInstrumentation() Option { } } +// Enables stacktrace +func WithStacktrace() Option { + return func(t *Transport) { + t.Stacktrace = true + } +} + // Patches the default http client with the instrumented transport func PatchHttpDefaultClient(options ...Option) { once.Do(func() { @@ -25,6 +32,7 @@ func PatchHttpDefaultClient(options ...Option) { option(transport) } transport.PayloadInstrumentation = transport.PayloadInstrumentation || env.ScopeInstrumentationHttpPayloads.Value + transport.Stacktrace = transport.Stacktrace || env.ScopeInstrumentationHttpStacktrace.Value http.DefaultClient = &http.Client{Transport: transport} }) } diff --git a/instrumentation/sql/driver.go b/instrumentation/sql/driver.go index 2e3661e2..5b235f22 100644 --- a/instrumentation/sql/driver.go +++ b/instrumentation/sql/driver.go @@ -5,11 +5,14 @@ import ( "database/sql/driver" "errors" "fmt" + "reflect" + "strings" + "github.com/opentracing/opentracing-go" + "go.undefinedlabs.com/scopeagent/env" + scopeerrors "go.undefinedlabs.com/scopeagent/errors" "go.undefinedlabs.com/scopeagent/instrumentation" - "reflect" - "strings" ) type ( @@ -22,6 +25,7 @@ type ( driverConfiguration struct { t opentracing.Tracer statementValues bool + stacktrace bool connString string componentName string peerService string @@ -41,6 +45,13 @@ func WithStatementValues() Option { } } +// Enable span stacktrace +func WithStacktrace() Option { + return func(d *instrumentedDriver) { + d.configuration.stacktrace = true + } +} + // Wraps the current sql driver to add instrumentation func WrapDriver(d driver.Driver, options ...Option) driver.Driver { wrapper := &instrumentedDriver{ @@ -54,6 +65,7 @@ func WrapDriver(d driver.Driver, options ...Option) driver.Driver { option(wrapper) } wrapper.configuration.statementValues = wrapper.configuration.statementValues || env.ScopeInstrumentationDbStatementValues.Value + wrapper.configuration.stacktrace = wrapper.configuration.stacktrace || env.ScopeInstrumentationDbStacktrace.Value return wrapper } @@ -106,6 +118,11 @@ func (t *driverConfiguration) newSpan(operationName string, query string, args [ "db.instance": c.instance, "peer.hostname": c.host, }) + if t.stacktrace { + opts = append(opts, opentracing.Tags{ + "stacktrace": scopeerrors.GetCurrentStackTrace(2), + }) + } if query != "" { stIndex := strings.IndexRune(query, ' ') var method string diff --git a/instrumentation/tracer.go b/instrumentation/tracer.go index e31da740..e5be8d8b 100644 --- a/instrumentation/tracer.go +++ b/instrumentation/tracer.go @@ -8,8 +8,9 @@ import ( ) var ( - tracer opentracing.Tracer = opentracing.NoopTracer{} - logger = log.New(ioutil.Discard, "", 0) + tracer opentracing.Tracer = opentracing.NoopTracer{} + logger = log.New(ioutil.Discard, "", 0) + sourceRoot = "" m sync.RWMutex ) @@ -41,3 +42,17 @@ func Logger() *log.Logger { return logger } + +func SetSourceRoot(root string) { + m.Lock() + defer m.Unlock() + + sourceRoot = root +} + +func GetSourceRoot() string { + m.RLock() + defer m.RUnlock() + + return sourceRoot +} From 1c269472e74cb53b777f21929654633dc1a3896d Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 13 Mar 2020 23:18:32 +0100 Subject: [PATCH 2/3] Removes the algorithm to ensure the first frame to be inside the source root. --- errors/handler.go | 10 ---------- instrumentation/nethttp/nethttp_test.go | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/errors/handler.go b/errors/handler.go index c09836b2..be44817e 100644 --- a/errors/handler.go +++ b/errors/handler.go @@ -2,7 +2,6 @@ package errors import ( "fmt" - "path" "strings" "time" @@ -10,7 +9,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/log" - "go.undefinedlabs.com/scopeagent/instrumentation" "go.undefinedlabs.com/scopeagent/tracer" ) @@ -60,7 +58,6 @@ func LogErrorInRawSpan(rawSpan *tracer.RawSpan, err **errors.Error) { // Gets the current stack frames array func getCurrentStackFrames(skip int) []StackFrames { - sourceRoot := instrumentation.GetSourceRoot() skip = skip + 1 err := errors.New(nil) errStack := err.StackFrames() @@ -71,13 +68,6 @@ func getCurrentStackFrames(skip int) []StackFrames { stackFrames := make([]StackFrames, 0) for idx, frame := range errStack { if idx >= skip { - if len(stackFrames) == 0 { - // We ensure that the first frame of the stacktrace should be inside the source.root - dir := path.Dir(frame.File) - if strings.Index(dir, sourceRoot) == -1 { - continue - } - } stackFrames = append(stackFrames, StackFrames{ File: frame.File, LineNumber: frame.LineNumber, diff --git a/instrumentation/nethttp/nethttp_test.go b/instrumentation/nethttp/nethttp_test.go index ffbedc01..1efab075 100644 --- a/instrumentation/nethttp/nethttp_test.go +++ b/instrumentation/nethttp/nethttp_test.go @@ -16,7 +16,7 @@ import ( var r *tracer.InMemorySpanRecorder func TestMain(m *testing.M) { - PatchHttpDefaultClient(WithPayloadInstrumentation()) + PatchHttpDefaultClient(WithPayloadInstrumentation(), WithStacktrace()) // Test tracer r = tracer.NewInMemoryRecorder() From b593d21013a4878eba246e54bf3bfcf43c0de209 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 13 Mar 2020 23:25:28 +0100 Subject: [PATCH 3/3] Remove unused code and clean filepath --- agent/agent.go | 3 --- errors/handler.go | 5 +++-- instrumentation/tracer.go | 19 ++----------------- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 08c1f23f..3c85a824 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -292,12 +292,10 @@ func NewAgent(options ...Option) (*Agent, error) { agent.metadata[tags.Dependencies] = getDependencyMap() // Expand '~' in source root - var sourceRoot string if sRoot, ok := agent.metadata[tags.SourceRoot]; ok { cSRoot := sRoot.(string) cSRoot = filepath.Clean(cSRoot) if sRootEx, err := homedir.Expand(cSRoot); err == nil { - sourceRoot = sRootEx agent.metadata[tags.SourceRoot] = sRootEx } } @@ -341,7 +339,6 @@ func NewAgent(options ...Option) (*Agent, error) { }) instrumentation.SetTracer(agent.tracer) instrumentation.SetLogger(agent.logger) - instrumentation.SetSourceRoot(sourceRoot) if agent.setGlobalTracer || env.ScopeTracerGlobal.Value { opentracing.SetGlobalTracer(agent.Tracer()) } diff --git a/errors/handler.go b/errors/handler.go index be44817e..f3ec2020 100644 --- a/errors/handler.go +++ b/errors/handler.go @@ -2,6 +2,7 @@ package errors import ( "fmt" + "path/filepath" "strings" "time" @@ -69,7 +70,7 @@ func getCurrentStackFrames(skip int) []StackFrames { for idx, frame := range errStack { if idx >= skip { stackFrames = append(stackFrames, StackFrames{ - File: frame.File, + File: filepath.Clean(frame.File), LineNumber: frame.LineNumber, Name: frame.Name, Package: frame.Package, @@ -154,7 +155,7 @@ func getExceptionFrameData(errMessage string, errStack []errors.StackFrame) map[ exFrames = append(exFrames, map[string]interface{}{ "name": frame.Name, "module": frame.Package, - "file": frame.File, + "file": filepath.Clean(frame.File), "line": frame.LineNumber, }) } diff --git a/instrumentation/tracer.go b/instrumentation/tracer.go index e5be8d8b..e31da740 100644 --- a/instrumentation/tracer.go +++ b/instrumentation/tracer.go @@ -8,9 +8,8 @@ import ( ) var ( - tracer opentracing.Tracer = opentracing.NoopTracer{} - logger = log.New(ioutil.Discard, "", 0) - sourceRoot = "" + tracer opentracing.Tracer = opentracing.NoopTracer{} + logger = log.New(ioutil.Discard, "", 0) m sync.RWMutex ) @@ -42,17 +41,3 @@ func Logger() *log.Logger { return logger } - -func SetSourceRoot(root string) { - m.Lock() - defer m.Unlock() - - sourceRoot = root -} - -func GetSourceRoot() string { - m.RLock() - defer m.RUnlock() - - return sourceRoot -}