Skip to content

Commit

Permalink
[exporter/datadog]: updated remove sublayer stats calc and mutex (3rd…
Browse files Browse the repository at this point in the history
… times a charm) (#3531)

* [exporter/datadog]: remove sublayer metrics calc

* [exporter/datadog]: formatting and linting

* Update exporter/datadogexporter/stats.go

Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>

Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
  • Loading branch information
ericmustin and mx-psi authored May 26, 2021
1 parent f02e8a0 commit 2696783
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 95 deletions.
17 changes: 0 additions & 17 deletions exporter/datadogexporter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
"github.com/DataDog/datadog-agent/pkg/trace/exportable/obfuscate"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/pb"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/sampler"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/stats"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/traceutil"
)

// obfuscatePayload applies obfuscator rules to the trace payloads
Expand Down Expand Up @@ -74,18 +72,3 @@ func getAnalyzedSpans(sps []*pb.Span) []*pb.Span {
}
return top
}

// Compute Sublayers updates a spans metrics with relevant metadata so that it's duration and breakdown between different services can
// be accurately displayed in the Datadog UI
func computeSublayerMetrics(calculator *sublayerCalculator, t pb.Trace) {
root := traceutil.GetRoot(t)
traceutil.ComputeTopLevel(t)

subtraces := stats.ExtractSubtraces(t, root)
sublayers := make(map[*pb.Span][]stats.SublayerValue)
for _, subtrace := range subtraces {
subtraceSublayers := calculator.computeSublayers(subtrace.Trace)
sublayers[subtrace.Root] = subtraceSublayers
stats.SetSublayersOnSpan(subtrace.Root, subtraceSublayers)
}
}
9 changes: 6 additions & 3 deletions exporter/datadogexporter/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ const (
)

// ComputeAPMStats calculates the stats that should be submitted to APM about a given trace
func computeAPMStats(tracePayload *pb.TracePayload, calculator *sublayerCalculator, pushTime int64) *stats.Payload {
func computeAPMStats(tracePayload *pb.TracePayload, pushTime int64) *stats.Payload {
statsRawBuckets := make(map[int64]*stats.RawBucket)

// removing sublayer calc as part of work to port
// https://github.com/DataDog/datadog-agent/pull/7450/files
var emptySublayer []stats.SublayerValue

bucketTS := pushTime - statsBucketDuration
for _, trace := range tracePayload.Traces {
spans := getAnalyzedSpans(trace.Spans)
sublayers := calculator.computeSublayers(trace.Spans)
for _, span := range spans {

// TODO: While this is hardcoded to assume a single 10s buckets for now,
Expand All @@ -58,7 +61,7 @@ func computeAPMStats(tracePayload *pb.TracePayload, calculator *sublayerCalculat
Weight: 1,
TopLevel: true,
}
statsRawBucket.HandleSpan(weightedSpan, tracePayload.Env, []string{versionAggregationTag}, sublayers)
statsRawBucket.HandleSpan(weightedSpan, tracePayload.Env, []string{versionAggregationTag}, emptySublayer)
}
}

Expand Down
24 changes: 2 additions & 22 deletions exporter/datadogexporter/traces_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@ package datadogexporter

import (
"context"
"sync"
"time"

"github.com/DataDog/datadog-agent/pkg/trace/exportable/config/configdefs"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/obfuscate"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/pb"
"github.com/DataDog/datadog-agent/pkg/trace/exportable/stats"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.uber.org/zap"
Expand All @@ -33,28 +31,12 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/utils"
)

// sublayerCalculator is thread safe wrapper of a sublayer
// calculator. Each trace exporter has a single sublayer
// calculator that is reused by each push
type sublayerCalculator struct {
sc *stats.SublayerCalculator
mutex sync.Mutex
}

// ComputeSublayers computes the sublayers of a trace
func (s *sublayerCalculator) computeSublayers(trace pb.Trace) []stats.SublayerValue {
s.mutex.Lock()
defer s.mutex.Unlock()
return s.sc.ComputeSublayers(trace)
}

type traceExporter struct {
params component.ExporterCreateParams
cfg *config.Config
ctx context.Context
edgeConnection TraceEdgeConnection
obfuscator *obfuscate.Obfuscator
calculator *sublayerCalculator
client *datadog.Client
denylister *Denylister
}
Expand Down Expand Up @@ -89,14 +71,12 @@ func newTracesExporter(ctx context.Context, params component.ExporterCreateParam
// a denylist for dropping ignored resources
denylister := NewDenylister(cfg.Traces.IgnoreResources)

calculator := &sublayerCalculator{sc: stats.NewSublayerCalculator()}
exporter := &traceExporter{
params: params,
cfg: cfg,
ctx: ctx,
edgeConnection: createTraceEdgeConnection(cfg.Traces.TCPAddr.Endpoint, cfg.API.Key, params.BuildInfo),
obfuscator: obfuscator,
calculator: calculator,
client: client,
denylister: denylister,
}
Expand Down Expand Up @@ -137,7 +117,7 @@ func (exp *traceExporter) pushTraceData(
// we largely apply the same logic as the serverless implementation, simplified a bit
// https://github.com/DataDog/datadog-serverless-functions/blob/f5c3aedfec5ba223b11b76a4239fcbf35ec7d045/aws/logs_monitoring/trace_forwarder/cmd/trace/main.go#L61-L83
fallbackHost := metadata.GetHost(exp.params.Logger, exp.cfg)
ddTraces, ms := convertToDatadogTd(td, fallbackHost, exp.calculator, exp.cfg, exp.denylister, exp.params.BuildInfo)
ddTraces, ms := convertToDatadogTd(td, fallbackHost, exp.cfg, exp.denylister, exp.params.BuildInfo)

// group the traces by env to reduce the number of flushes
aggregatedTraces := aggregateTracePayloadsByEnv(ddTraces)
Expand Down Expand Up @@ -169,7 +149,7 @@ func (exp *traceExporter) pushWithRetry(ctx context.Context, ddTracePayload *pb.
}

// this is for generating metrics like hits, errors, and latency, it uses a separate endpoint than Traces
stats := computeAPMStats(ddTracePayload, exp.calculator, pushTime)
stats := computeAPMStats(ddTracePayload, pushTime)
errStats := exp.edgeConnection.SendStats(context.Background(), stats, maxRetries)

if errStats != nil {
Expand Down
10 changes: 3 additions & 7 deletions exporter/datadogexporter/translate_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const (
)

// converts Traces into an array of datadog trace payloads grouped by env
func convertToDatadogTd(td pdata.Traces, fallbackHost string, calculator *sublayerCalculator, cfg *config.Config, blk *Denylister, buildInfo component.BuildInfo) ([]*pb.TracePayload, []datadog.Metric) {
func convertToDatadogTd(td pdata.Traces, fallbackHost string, cfg *config.Config, blk *Denylister, buildInfo component.BuildInfo) ([]*pb.TracePayload, []datadog.Metric) {
// TODO:
// do we apply other global tags, like version+service, to every span or only root spans of a service
// should globalTags['service'] take precedence over a trace's resource.service.name? I don't believe so, need to confirm
Expand All @@ -82,7 +82,7 @@ func convertToDatadogTd(td pdata.Traces, fallbackHost string, calculator *sublay
host = fallbackHost
}

payload := resourceSpansToDatadogSpans(rs, calculator, host, cfg, blk)
payload := resourceSpansToDatadogSpans(rs, host, cfg, blk)
traces = append(traces, &payload)

ms := metrics.DefaultMetrics("traces", host, uint64(pushTime), buildInfo)
Expand Down Expand Up @@ -119,7 +119,7 @@ func aggregateTracePayloadsByEnv(tracePayloads []*pb.TracePayload) []*pb.TracePa
}

// converts a Trace's resource spans into a trace payload
func resourceSpansToDatadogSpans(rs pdata.ResourceSpans, calculator *sublayerCalculator, hostname string, cfg *config.Config, blk *Denylister) pb.TracePayload {
func resourceSpansToDatadogSpans(rs pdata.ResourceSpans, hostname string, cfg *config.Config, blk *Denylister) pb.TracePayload {
// get env tag
env := utils.NormalizeTag(cfg.Env)

Expand Down Expand Up @@ -199,10 +199,6 @@ func resourceSpansToDatadogSpans(rs pdata.ResourceSpans, calculator *sublayerCal
// TODO: allow users to configure specific spans to be marked as an analyzed spans for app analytics
top := getAnalyzedSpans(apiTrace.Spans)

// calculates span metrics for representing direction and timing among it's different services for display in
// service overview graphs
// see: https://github.com/DataDog/datadog-agent/blob/f69a7d35330c563e9cad4c5b8865a357a87cd0dc/pkg/trace/stats/sublayers.go#L204
computeSublayerMetrics(calculator, apiTrace.Spans)
payload.Transactions = append(payload.Transactions, top...)
payload.Traces = append(payload.Traces, apiTrace)
}
Expand Down
Loading

0 comments on commit 2696783

Please sign in to comment.