diff --git a/modelwriter.go b/modelwriter.go index 103652c7d..e9e0e81bd 100644 --- a/modelwriter.go +++ b/modelwriter.go @@ -22,7 +22,6 @@ import ( "go.elastic.co/apm/internal/ringbuffer" "go.elastic.co/apm/model" - "go.elastic.co/apm/stacktrace" "go.elastic.co/fastjson" ) @@ -162,7 +161,6 @@ func (w *modelWriter) buildModelSpan(out *model.Span, span *Span, sd *SpanData) w.modelStacktrace = appendModelStacktraceFrames(w.modelStacktrace, sd.stacktrace) out.Stacktrace = w.modelStacktrace - w.setStacktraceContext(out.Stacktrace) } func (w *modelWriter) buildModelError(out *model.Error, e *ErrorData) { @@ -197,7 +195,6 @@ func (w *modelWriter) buildModelError(out *model.Error, e *ErrorData) { if len(e.logStacktrace) != 0 { w.modelStacktrace = appendModelStacktraceFrames(w.modelStacktrace, e.logStacktrace) } - w.setStacktraceContext(w.modelStacktrace) var modelStacktraceOffset int if e.exception.message != "" { @@ -262,19 +259,6 @@ func stacktraceCulprit(frames []model.StacktraceFrame) string { return "" } -func (w *modelWriter) setStacktraceContext(stack []model.StacktraceFrame) { - if w.cfg.contextSetter == nil || len(stack) == 0 { - return - } - err := stacktrace.SetContext(w.cfg.contextSetter, stack, w.cfg.preContext, w.cfg.postContext) - if err != nil { - if w.cfg.logger != nil { - w.cfg.logger.Debugf("setting context failed: %v", err) - } - w.stats.Errors.SetContext++ - } -} - func normalizeOutcome(outcome string) string { switch outcome { case "success", "failure", "unknown": diff --git a/stacktrace/context.go b/stacktrace/context.go deleted file mode 100644 index e7e307a76..000000000 --- a/stacktrace/context.go +++ /dev/null @@ -1,100 +0,0 @@ -// Licensed to Elasticsearch B.V. under one or more contributor -// license agreements. See the NOTICE file distributed with -// this work for additional information regarding copyright -// ownership. Elasticsearch B.V. licenses this file to you under -// the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package stacktrace // import "go.elastic.co/apm/stacktrace" - -import ( - "bufio" - "net/http" - "os" - - "go.elastic.co/apm/model" -) - -// SetContext sets the source context for the given stack frames, -// with the specified number of pre- and post- lines. -func SetContext(setter ContextSetter, frames []model.StacktraceFrame, pre, post int) error { - for i := 0; i < len(frames); i++ { - if err := setter.SetContext(&frames[i], pre, post); err != nil { - return err - } - } - return nil -} - -// ContextSetter is an interface that can be used for setting the source -// context for a stack frame. -type ContextSetter interface { - // SetContext sets the source context for the given stack frame, - // with the specified number of pre- and post- lines. - SetContext(frame *model.StacktraceFrame, pre, post int) error -} - -// FileSystemContextSetter returns a ContextSetter that sets context -// by reading file contents from the provided http.FileSystem. -func FileSystemContextSetter(fs http.FileSystem) ContextSetter { - if fs == nil { - panic("fs is nil") - } - return &fileSystemContextSetter{fs} -} - -type fileSystemContextSetter struct { - http.FileSystem -} - -func (s *fileSystemContextSetter) SetContext(frame *model.StacktraceFrame, pre, post int) error { - if frame.Line <= 0 { - return nil - } - f, err := s.Open(frame.AbsolutePath) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - defer f.Close() - - var lineno int - var line string - preLines := make([]string, 0, pre) - postLines := make([]string, 0, post) - - scanner := bufio.NewScanner(f) - for scanner.Scan() { - lineno++ - if lineno > frame.Line+post { - break - } - switch { - case lineno == frame.Line: - line = scanner.Text() - case lineno < frame.Line && lineno >= frame.Line-pre: - preLines = append(preLines, scanner.Text()) - case lineno > frame.Line && lineno <= frame.Line+post: - postLines = append(postLines, scanner.Text()) - } - } - if err := scanner.Err(); err != nil { - return err - } - frame.ContextLine = line - frame.PreContext = preLines - frame.PostContext = postLines - return nil -} diff --git a/stacktrace/context_test.go b/stacktrace/context_test.go deleted file mode 100644 index 60c595e1c..000000000 --- a/stacktrace/context_test.go +++ /dev/null @@ -1,99 +0,0 @@ -// Licensed to Elasticsearch B.V. under one or more contributor -// license agreements. See the NOTICE file distributed with -// this work for additional information regarding copyright -// ownership. Elasticsearch B.V. licenses this file to you under -// the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package stacktrace_test - -import ( - "io/ioutil" - "net/http" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - - "go.elastic.co/apm/model" - "go.elastic.co/apm/stacktrace" -) - -func TestFilesystemContextSetter(t *testing.T) { - setter := stacktrace.FileSystemContextSetter(http.Dir("./testdata")) - frame := model.StacktraceFrame{ - AbsolutePath: "/foo.go", - Line: 5, - } - - data, err := ioutil.ReadFile("./testdata/foo.go") - if err != nil { - t.Fatal(err) - } - lines := strings.Split(strings.TrimSpace(string(data)), "\n") - - testSetContext(t, setter, frame, 2, 1, - lines[4], - lines[2:4], - lines[5:], - ) - testSetContext(t, setter, frame, 0, 0, lines[4], []string{}, []string{}) - testSetContext(t, setter, frame, 500, 0, lines[4], lines[:4], []string{}) - testSetContext(t, setter, frame, 0, 500, lines[4], []string{}, lines[5:]) -} - -func TestFilesystemContextSetterFileNotFound(t *testing.T) { - setter := stacktrace.FileSystemContextSetter(http.Dir("./testdata")) - frame := model.StacktraceFrame{ - AbsolutePath: "/foo.go", - Line: 5, - } - - data, err := ioutil.ReadFile("./testdata/foo.go") - if err != nil { - t.Fatal(err) - } - lines := strings.Split(strings.TrimSpace(string(data)), "\n") - - testSetContext(t, setter, frame, 2, 1, - lines[4], - lines[2:4], - lines[5:], - ) - testSetContext(t, setter, frame, 0, 0, lines[4], []string{}, []string{}) - testSetContext(t, setter, frame, 500, 0, lines[4], lines[:4], []string{}) - testSetContext(t, setter, frame, 0, 500, lines[4], []string{}, lines[5:]) -} - -func testSetContext( - t *testing.T, - setter stacktrace.ContextSetter, - frame model.StacktraceFrame, - nPre, nPost int, - expectedContext string, expectedPre, expectedPost []string, -) { - frames := []model.StacktraceFrame{frame} - err := stacktrace.SetContext(setter, frames, nPre, nPost) - if err != nil { - t.Fatalf("SetContext failed: %s", err) - } - if diff := cmp.Diff(frames[0].ContextLine, expectedContext); diff != "" { - t.Fatalf("ContextLine differs: %s", diff) - } - if diff := cmp.Diff(frames[0].PreContext, expectedPre); diff != "" { - t.Fatalf("PreContext differs: %s", diff) - } - if diff := cmp.Diff(frames[0].PostContext, expectedPost); diff != "" { - t.Fatalf("PostContext differs: %s", diff) - } -} diff --git a/tracer.go b/tracer.go index 6fed4fcc5..6b6e3d766 100644 --- a/tracer.go +++ b/tracer.go @@ -35,14 +35,11 @@ import ( "go.elastic.co/apm/internal/ringbuffer" "go.elastic.co/apm/internal/wildcard" "go.elastic.co/apm/model" - "go.elastic.co/apm/stacktrace" "go.elastic.co/apm/transport" "go.elastic.co/fastjson" ) const ( - defaultPreContext = 3 - defaultPostContext = 3 gracePeriodJitter = 0.1 // +/- 10% tracerEventChannelCap = 1000 ) @@ -503,8 +500,6 @@ func newTracer(opts TracerOptions) *Tracer { cfg.requestDuration = opts.requestDuration cfg.requestSize = opts.requestSize cfg.disabledMetrics = opts.disabledMetrics - cfg.preContext = defaultPreContext - cfg.postContext = defaultPostContext cfg.metricsGatherers = []MetricsGatherer{newBuiltinMetricsGatherer(t)} if apmlog.DefaultLogger != nil { cfg.logger = apmlog.DefaultLogger @@ -519,18 +514,16 @@ func newTracer(opts TracerOptions) *Tracer { // tracerConfig holds the tracer's runtime configuration, which may be modified // by sending a tracerConfigCommand to the tracer's configCommands channel. type tracerConfig struct { - recording bool - requestSize int - requestDuration time.Duration - metricsInterval time.Duration - logger WarningLogger - metricsGatherers []MetricsGatherer - contextSetter stacktrace.ContextSetter - preContext, postContext int - disabledMetrics wildcard.Matchers - cpuProfileDuration time.Duration - cpuProfileInterval time.Duration - heapProfileInterval time.Duration + recording bool + requestSize int + requestDuration time.Duration + metricsInterval time.Duration + logger WarningLogger + metricsGatherers []MetricsGatherer + disabledMetrics wildcard.Matchers + cpuProfileDuration time.Duration + cpuProfileInterval time.Duration + heapProfileInterval time.Duration } type tracerConfigCommand func(*tracerConfig) @@ -604,15 +597,6 @@ func (t *Tracer) SetMetricsInterval(d time.Duration) { }) } -// SetContextSetter sets the stacktrace.ContextSetter to be used for -// setting stacktrace source context. If nil (which is the initial -// value), no context will be set. -func (t *Tracer) SetContextSetter(setter stacktrace.ContextSetter) { - t.sendConfigCommand(func(cfg *tracerConfig) { - cfg.contextSetter = setter - }) -} - // SetLogger sets the Logger to be used for logging the operation of // the tracer. // diff --git a/tracer_stats.go b/tracer_stats.go index e4be95f41..9143c38d5 100644 --- a/tracer_stats.go +++ b/tracer_stats.go @@ -32,7 +32,6 @@ type TracerStats struct { // TracerStatsErrors holds error statistics for a Tracer. type TracerStatsErrors struct { - SetContext uint64 SendStream uint64 } @@ -43,7 +42,6 @@ func (s TracerStats) isZero() bool { // accumulate updates the stats by accumulating them with // the values in rhs. func (s *TracerStats) accumulate(rhs TracerStats) { - atomic.AddUint64(&s.Errors.SetContext, rhs.Errors.SetContext) atomic.AddUint64(&s.Errors.SendStream, rhs.Errors.SendStream) atomic.AddUint64(&s.ErrorsSent, rhs.ErrorsSent) atomic.AddUint64(&s.ErrorsDropped, rhs.ErrorsDropped) @@ -57,7 +55,6 @@ func (s *TracerStats) accumulate(rhs TracerStats) { func (s *TracerStats) copy() TracerStats { return TracerStats{ Errors: TracerStatsErrors{ - SetContext: atomic.LoadUint64(&s.Errors.SetContext), SendStream: atomic.LoadUint64(&s.Errors.SendStream), }, ErrorsSent: atomic.LoadUint64(&s.ErrorsSent),