Skip to content

Commit

Permalink
feat: improve log and error messages (#404)
Browse files Browse the repository at this point in the history
* feat: improve log and error messages

remove debug logs
improve message

* lint: remove unused files
  • Loading branch information
kruskall authored Dec 5, 2024
1 parent 87be983 commit 95a8b60
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 120 deletions.
4 changes: 2 additions & 2 deletions input/elasticapm/internal/modeldecoder/nullable/nullable.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ func init() {
case string:
h.Add(key, entry)
default:
iter.Error = fmt.Errorf("invalid input for HTTPHeader: %v", v)
iter.Error = fmt.Errorf("invalid input for HTTPHeader slice value type: %T", entry)
return
}
}
default:
iter.Error = fmt.Errorf("invalid input for HTTPHeader: %v", v)
iter.Error = fmt.Errorf("invalid input for HTTPHeader value type: %T", v)
return
}
}
Expand Down
26 changes: 12 additions & 14 deletions input/otlp/exceptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func setJavaExceptionStacktrace(s string, out *modelpb.Exception) error {
current := Exception{out, nil, 0}
stack := []Exception{}
scanner := bufio.NewScanner(strings.NewReader(s))
for scanner.Scan() {
for j := 0; scanner.Scan(); j++ {
if first {
// Ignore the first line, we only care about the locations.
first = false
Expand All @@ -140,25 +140,28 @@ func setJavaExceptionStacktrace(s string, out *modelpb.Exception) error {
}
switch {
case strings.HasPrefix(line, "at "):
if err := parseJavaStacktraceFrame(line, current.Exception); err != nil {
return err
submatch := javaStacktraceAtRegexp.FindStringSubmatch(line)
if submatch == nil {
return fmt.Errorf("failed to parse stacktrace at line %d", j)
}

parseJavaStacktraceFrame(submatch, current.Exception)
case strings.HasPrefix(line, "..."):
// "... N more" lines indicate that the last N frames from the enclosing
// exception's stacktrace are common to this exception.
if current.enclosing == nil {
return fmt.Errorf("no enclosing exception preceding line %q", line)
return fmt.Errorf("no enclosing exception preceding at line %d", j)
}
submatch := javaStacktraceMoreRegexp.FindStringSubmatch(line)
if submatch == nil {
return fmt.Errorf("failed to parse stacktrace line %q", line)
return fmt.Errorf("failed to parse stacktrace at line %d", j)
}
if n, err := strconv.Atoi(submatch[1]); err == nil {
enclosing := current.enclosing
if len(enclosing.Stacktrace) < n {
return fmt.Errorf(
"enclosing exception stacktrace has %d frames, cannot satisfy %q",
len(enclosing.Stacktrace), line,
"enclosing exception stacktrace has %d frames, cannot satisfy line %d",
len(enclosing.Stacktrace), j,
)
}
m := len(enclosing.Stacktrace)
Expand All @@ -184,17 +187,13 @@ func setJavaExceptionStacktrace(s string, out *modelpb.Exception) error {
current.Exception = &modelpb.Exception{}
current.indent = indent
default:
return fmt.Errorf("unexpected line %q", line)
return fmt.Errorf("unexpected value at line %d", j)
}
}
return scanner.Err()
}

func parseJavaStacktraceFrame(s string, out *modelpb.Exception) error {
submatch := javaStacktraceAtRegexp.FindStringSubmatch(s)
if submatch == nil {
return fmt.Errorf("failed to parse stacktrace line %q", s)
}
func parseJavaStacktraceFrame(submatch []string, out *modelpb.Exception) {
var module string
function := submatch[1]
if slash := strings.IndexRune(function, '/'); slash >= 0 {
Expand Down Expand Up @@ -226,7 +225,6 @@ func parseJavaStacktraceFrame(s string, out *modelpb.Exception) error {
sf.Filename = file
sf.Lineno = lineno
out.Stacktrace = append(out.Stacktrace, &sf)
return nil
}

func isNotTab(r rune) bool {
Expand Down
60 changes: 0 additions & 60 deletions input/otlp/logging.go

This file was deleted.

2 changes: 0 additions & 2 deletions input/otlp/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/plog"
semconv "go.opentelemetry.io/collector/semconv/v1.5.0"
"go.uber.org/zap"

"github.com/elastic/apm-data/model/modelpb"
)
Expand All @@ -70,7 +69,6 @@ func (c *Consumer) ConsumeLogsWithResult(ctx context.Context, logs plog.Logs) (C
defer c.sem.Release(1)

receiveTimestamp := time.Now()
c.config.Logger.Debug("consuming logs", zap.Stringer("logs", logsStringer(logs)))
resourceLogs := logs.ResourceLogs()
batch := make(modelpb.Batch, 0, resourceLogs.Len())
for i := 0; i < resourceLogs.Len(); i++ {
Expand Down
2 changes: 0 additions & 2 deletions input/otlp/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.uber.org/zap"

"github.com/elastic/apm-data/model/modelpb"
)
Expand Down Expand Up @@ -74,7 +73,6 @@ func (c *Consumer) ConsumeMetricsWithResult(ctx context.Context, metrics pmetric
remainingDataPoints := totalDataPoints
remainingMetrics := totalMetrics
receiveTimestamp := time.Now()
c.config.Logger.Debug("consuming metrics", zap.Stringer("metrics", metricsStringer(metrics)))
batch := c.handleMetrics(metrics, receiveTimestamp, &remainingDataPoints, &remainingMetrics)
if remainingMetrics > 0 {
// Some metrics remained after conversion, meaning that they were dropped.
Expand Down
17 changes: 0 additions & 17 deletions input/otlp/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1121,23 +1121,6 @@ func TestConsumeMetricsWithOTelRemapper(t *testing.T) {
}
}

/* TODO
func TestMetricsLogging(t *testing.T) {
for _, level := range []logp.Level{logp.InfoLevel, logp.DebugLevel} {
t.Run(level.String(), func(t *testing.T) {
logp.DevelopmentSetup(logp.ToObserverOutput(), logp.WithLevel(level))
transformMetrics(t, pmetric.NewMetrics())
logs := logp.ObserverLogs().TakeAll()
if level == logp.InfoLevel {
assert.Empty(t, logs)
} else {
assert.NotEmpty(t, logs)
}
})
}
}
*/

func transformMetrics(t *testing.T, metrics pmetric.Metrics) ([]*modelpb.APMEvent, otlp.ConsumerStats, otlp.ConsumeMetricsResult, error) {
var batches []*modelpb.Batch
recorder := batchRecorderBatchProcessor(&batches)
Expand Down
6 changes: 0 additions & 6 deletions input/otlp/traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import (
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"
semconv "go.opentelemetry.io/collector/semconv/v1.5.0"
"go.uber.org/zap"
"google.golang.org/grpc/codes"

"github.com/elastic/apm-data/model/modelpb"
Expand Down Expand Up @@ -107,7 +106,6 @@ func (c *Consumer) ConsumeTracesWithResult(ctx context.Context, traces ptrace.Tr
defer c.sem.Release(1)

receiveTimestamp := time.Now()
c.config.Logger.Debug("consuming traces", zap.Stringer("traces", tracesStringer(traces)))

resourceSpans := traces.ResourceSpans()
batch := make(modelpb.Batch, 0, resourceSpans.Len())
Expand Down Expand Up @@ -1226,10 +1224,6 @@ func (c *Consumer) convertJaegerErrorSpanEvent(event ptrace.SpanEvent, apmEvent
return nil
}
if logMessage == "" && exMessage == "" && exType == "" {
c.config.Logger.Debug(
"cannot convert span event into Elastic APM error",
zap.String("name", event.Name()),
)
return nil
}
e := modelpb.Error{}
Expand Down
17 changes: 0 additions & 17 deletions input/otlp/traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,23 +1200,6 @@ func TestConsumeTracesSemaphore(t *testing.T) {
assert.NoError(t, err)
}

/* TODO
func TestTracesLogging(t *testing.T) {
for _, level := range []logp.Level{logp.InfoLevel, logp.DebugLevel} {
t.Run(level.String(), func(t *testing.T) {
logp.DevelopmentSetup(logp.ToObserverOutput(), logp.WithLevel(level))
transformTraces(t, ptrace.NewTraces())
logs := logp.ObserverLogs().TakeAll()
if level == logp.InfoLevel {
assert.Empty(t, logs)
} else {
assert.NotEmpty(t, logs)
}
})
}
}
*/

func TestServiceTarget(t *testing.T) {
test := func(t *testing.T, expected *modelpb.ServiceTarget, input map[string]interface{}) {
t.Helper()
Expand Down

0 comments on commit 95a8b60

Please sign in to comment.