From f853a99d3be25aa6436e9c89f5453169c62efc82 Mon Sep 17 00:00:00 2001 From: Eric Mustin Date: Fri, 30 Apr 2021 16:55:45 -0400 Subject: [PATCH] [exporter/datadog]: add ignore_resources configuration option (#3245) * [exporter/datadog]: wip add blocklist for resources. wip test and documentation * [exporter/datadog]: clean up ignore resources defaults * [exporter/datadog]: remove unused helpers * [exporter/datadog]: add yaml config details * [exporter/datadog]: add links to datadog agent funcs * [exporter/datadog]: seperate blocklist into its own file and tests * [exporter/datadog]: fix copyrights * [exporter/datadog]: pr feedback update code comments and update naming conventions, clean up code comments * [exporter/datadog]: add validatioon block to config * [exporter/datadog]: update config * [exporter/datadog]: linting * [exporter/datadog]: wip add helper tests * [exporter/datadog]: add helper tests * [exporter/datadog]: linting * [exporter/datadog]: use mustcompile for denylist * [exporter/datadog]: fix tests for new config/denylist behavior * [exporter/datadog]: add ignore resources validation test for config --- exporter/datadogexporter/config/config.go | 19 +++++ .../datadogexporter/config/config_test.go | 10 +++ exporter/datadogexporter/denylister.go | 57 +++++++++++++ exporter/datadogexporter/denylister_test.go | 83 +++++++++++++++++++ exporter/datadogexporter/example/config.yaml | 6 ++ exporter/datadogexporter/factory.go | 1 + exporter/datadogexporter/factory_test.go | 5 ++ exporter/datadogexporter/traces_exporter.go | 7 +- .../datadogexporter/traces_exporter_test.go | 1 + exporter/datadogexporter/translate_traces.go | 31 ++++++- .../datadogexporter/translate_traces_test.go | 78 +++++++++++++---- .../datadogexporter/utils/trace_helpers.go | 43 ++++++++++ .../utils/trace_helpers_test.go | 45 ++++++++++ 13 files changed, 363 insertions(+), 23 deletions(-) create mode 100644 exporter/datadogexporter/denylister.go create mode 100644 exporter/datadogexporter/denylister_test.go diff --git a/exporter/datadogexporter/config/config.go b/exporter/datadogexporter/config/config.go index 8fc66f54b2fc..4d6cf86f295f 100644 --- a/exporter/datadogexporter/config/config.go +++ b/exporter/datadogexporter/config/config.go @@ -17,6 +17,7 @@ package config import ( "errors" "fmt" + "regexp" "strings" "sync" @@ -97,6 +98,12 @@ type TracesConfig struct { // meaning no sampling. If you want to send one event out of every 250 // times Send() is called, you would specify 250 here. SampleRate uint `mapstructure:"sample_rate"` + + // ignored resources + // A blacklist of regular expressions can be provided to disable certain traces based on their resource name + // all entries must be surrounded by double quotes and separated by commas. + // ignore_resources: ["(GET|POST) /healthcheck"] + IgnoreResources []string `mapstructure:"ignore_resources"` } // TagsConfig defines the tag-related configuration @@ -226,3 +233,15 @@ func (c *Config) Sanitize() error { return nil } + +func (c *Config) Validate() error { + if c.Traces.IgnoreResources != nil { + for _, entry := range c.Traces.IgnoreResources { + _, err := regexp.Compile(entry) + if err != nil { + return fmt.Errorf("'%s' is not valid resource filter regular expression", entry) + } + } + } + return nil +} diff --git a/exporter/datadogexporter/config/config_test.go b/exporter/datadogexporter/config/config_test.go index fec388457c40..0cb62845e7fd 100644 --- a/exporter/datadogexporter/config/config_test.go +++ b/exporter/datadogexporter/config/config_test.go @@ -145,3 +145,13 @@ func TestCensorAPIKey(t *testing.T) { cfg.GetCensoredKey(), ) } + +func TestIgnoreResourcesValidation(t *testing.T) { + validCfg := Config{Traces: TracesConfig{IgnoreResources: []string{"[123]"}}} + invalidCfg := Config{Traces: TracesConfig{IgnoreResources: []string{"[123"}}} + + noErr := validCfg.Validate() + err := invalidCfg.Validate() + require.NoError(t, noErr) + require.Error(t, err) +} diff --git a/exporter/datadogexporter/denylister.go b/exporter/datadogexporter/denylister.go new file mode 100644 index 000000000000..7584f6bfae8b --- /dev/null +++ b/exporter/datadogexporter/denylister.go @@ -0,0 +1,57 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed 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 datadogexporter + +import ( + "regexp" + + "github.com/DataDog/datadog-agent/pkg/trace/exportable/pb" +) + +// Denylister holds a list of regular expressions which will match resources +// on spans that should be dropped. +// From: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/filters/blacklister.go#L15-L19 +type Denylister struct { + list []*regexp.Regexp +} + +// Allows returns true if the Denylister permits this span. +// From: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/filters/blacklister.go#L21-L29 +func (f *Denylister) Allows(span *pb.Span) bool { + for _, entry := range f.list { + if entry.MatchString(span.Resource) { + return false + } + } + return true +} + +// NewDenylister creates a new Denylister based on the given list of +// regular expressions. +// From: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/filters/blacklister.go#L41-L45 +func NewDenylister(exprs []string) *Denylister { + return &Denylister{list: compileRules(exprs)} +} + +// compileRules compiles as many rules as possible from the list of expressions. +// From: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/filters/blacklister.go#L47-L59 +func compileRules(exprs []string) []*regexp.Regexp { + list := make([]*regexp.Regexp, 0, len(exprs)) + for _, entry := range exprs { + rule := regexp.MustCompile(entry) + list = append(list, rule) + } + return list +} diff --git a/exporter/datadogexporter/denylister_test.go b/exporter/datadogexporter/denylister_test.go new file mode 100644 index 000000000000..f6bcc42dcc71 --- /dev/null +++ b/exporter/datadogexporter/denylister_test.go @@ -0,0 +1,83 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed 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 datadogexporter + +import ( + "testing" + + "github.com/DataDog/datadog-agent/pkg/trace/exportable/pb" + "github.com/stretchr/testify/assert" +) + +// TestSpan returns a fix span with hardcoded info, useful for reproducible tests +// https://github.com/DataDog/datadog-agent/blob/d3a4cd66314d70162e2c76d2481f4b93455cf717/pkg/trace/test/testutil/span.go#L372 +func testSpan() *pb.Span { + return &pb.Span{ + Duration: 10000000, + Error: 0, + Resource: "GET /some/raclette", + Service: "django", + Name: "django.controller", + SpanID: 42, + Start: 1472732573337575936, + TraceID: 424242, + Meta: map[string]string{ + "user": "eric", + "pool": "fondue", + }, + Metrics: map[string]float64{ + "cheese_weight": 100000.0, + }, + ParentID: 1111, + Type: "http", + } +} + +func TestDenylister(t *testing.T) { + tests := []struct { + filter []string + resource string + expectation bool + }{ + {[]string{"/foo/bar"}, "/foo/bar", false}, + {[]string{"/foo/b.r"}, "/foo/bar", false}, + {[]string{"[0-9]+"}, "/abcde", true}, + {[]string{"[0-9]+"}, "/abcde123", false}, + {[]string{"\\(foobar\\)"}, "(foobar)", false}, + {[]string{"\\(foobar\\)"}, "(bar)", true}, + {[]string{"(GET|POST) /healthcheck"}, "GET /foobar", true}, + {[]string{"(GET|POST) /healthcheck"}, "GET /healthcheck", false}, + {[]string{"(GET|POST) /healthcheck"}, "POST /healthcheck", false}, + {[]string{"SELECT COUNT\\(\\*\\) FROM BAR"}, "SELECT COUNT(*) FROM BAR", false}, + {[]string{"ABC+", "W+"}, "ABCCCC", false}, + {[]string{"ABC+", "W+"}, "WWW", false}, + } + + for _, test := range tests { + span := testSpan() + span.Resource = test.resource + filter := NewDenylister(test.filter) + + assert.Equal(t, test.expectation, filter.Allows(span)) + } +} + +func TestCompileRules(t *testing.T) { + filter := NewDenylister([]string{"\n{6}"}) + for i := 0; i < 100; i++ { + span := testSpan() + assert.True(t, filter.Allows(span)) + } +} diff --git a/exporter/datadogexporter/example/config.yaml b/exporter/datadogexporter/example/config.yaml index d0837088dff6..e4865b713be3 100644 --- a/exporter/datadogexporter/example/config.yaml +++ b/exporter/datadogexporter/example/config.yaml @@ -116,6 +116,12 @@ exporters: # # endpoint: https://api.datadoghq.com + ## @param ignore_resources - list of strings - optional + ## A blacklist of regular expressions can be provided to disable certain traces based on their resource name + ## all entries must be surrounded by double quotes and separated by commas. + # + # ignore_resources: ["(GET|POST) /healthcheck"] + service: pipelines: traces: diff --git a/exporter/datadogexporter/factory.go b/exporter/datadogexporter/factory.go index 5fb103227429..9a50679eb879 100644 --- a/exporter/datadogexporter/factory.go +++ b/exporter/datadogexporter/factory.go @@ -75,6 +75,7 @@ func createDefaultConfig() config.Exporter { TCPAddr: confignet.TCPAddr{ Endpoint: "$DD_APM_URL", // If not provided, set during config sanitization }, + IgnoreResources: []string{}, }, SendMetadata: true, diff --git a/exporter/datadogexporter/factory_test.go b/exporter/datadogexporter/factory_test.go index 66a70b08e6ae..aff998f0c9e9 100644 --- a/exporter/datadogexporter/factory_test.go +++ b/exporter/datadogexporter/factory_test.go @@ -64,6 +64,7 @@ func TestCreateDefaultConfig(t *testing.T) { TCPAddr: confignet.TCPAddr{ Endpoint: "$DD_APM_URL", }, + IgnoreResources: []string{}, }, TagsConfig: ddconfig.TagsConfig{ @@ -131,6 +132,7 @@ func TestLoadConfig(t *testing.T) { TCPAddr: confignet.TCPAddr{ Endpoint: "https://trace.agent.datadoghq.eu", }, + IgnoreResources: []string{}, }, SendMetadata: true, OnlyMetadata: false, @@ -173,6 +175,7 @@ func TestLoadConfig(t *testing.T) { TCPAddr: confignet.TCPAddr{ Endpoint: "https://trace.agent.datadoghq.com", }, + IgnoreResources: []string{}, }, SendMetadata: true, OnlyMetadata: false, @@ -257,6 +260,7 @@ func TestLoadConfigEnvVariables(t *testing.T) { TCPAddr: confignet.TCPAddr{ Endpoint: "https://trace.agent.datadoghq.test", }, + IgnoreResources: []string{}, }, SendMetadata: true, OnlyMetadata: false, @@ -302,6 +306,7 @@ func TestLoadConfigEnvVariables(t *testing.T) { TCPAddr: confignet.TCPAddr{ Endpoint: "https://trace.agent.datadoghq.com", }, + IgnoreResources: []string{}, }, SendMetadata: true, OnlyMetadata: false, diff --git a/exporter/datadogexporter/traces_exporter.go b/exporter/datadogexporter/traces_exporter.go index 6602a2776d27..5d8bf02b5bd0 100644 --- a/exporter/datadogexporter/traces_exporter.go +++ b/exporter/datadogexporter/traces_exporter.go @@ -56,6 +56,7 @@ type traceExporter struct { obfuscator *obfuscate.Obfuscator calculator *sublayerCalculator client *datadog.Client + denylister *Denylister } var ( @@ -85,6 +86,9 @@ func newTracesExporter(ctx context.Context, params component.ExporterCreateParam // https://github.com/DataDog/datadog-serverless-functions/blob/11f170eac105d66be30f18eda09eca791bc0d31b/aws/logs_monitoring/trace_forwarder/cmd/trace/main.go#L43 obfuscator := obfuscate.NewObfuscator(obfuscatorConfig) + // a denylist for dropping ignored resources + denylister := NewDenylister(cfg.Traces.IgnoreResources) + calculator := &sublayerCalculator{sc: stats.NewSublayerCalculator()} exporter := &traceExporter{ params: params, @@ -94,6 +98,7 @@ func newTracesExporter(ctx context.Context, params component.ExporterCreateParam obfuscator: obfuscator, calculator: calculator, client: client, + denylister: denylister, } return exporter @@ -131,7 +136,7 @@ func (exp *traceExporter) pushTraceData( // convert traces to datadog traces and group trace payloads by env // 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 - ddTraces, ms := convertToDatadogTd(td, exp.calculator, exp.cfg) + ddTraces, ms := convertToDatadogTd(td, exp.calculator, exp.cfg, exp.denylister) // group the traces by env to reduce the number of flushes aggregatedTraces := aggregateTracePayloadsByEnv(ddTraces) diff --git a/exporter/datadogexporter/traces_exporter_test.go b/exporter/datadogexporter/traces_exporter_test.go index a3298e5f0dc6..7c63e3d472a3 100644 --- a/exporter/datadogexporter/traces_exporter_test.go +++ b/exporter/datadogexporter/traces_exporter_test.go @@ -81,6 +81,7 @@ func testTracesExporterHelper(td pdata.Traces, t *testing.T) []string { TCPAddr: confignet.TCPAddr{ Endpoint: server.URL, }, + IgnoreResources: []string{}, }, } diff --git a/exporter/datadogexporter/translate_traces.go b/exporter/datadogexporter/translate_traces.go index bff2883f7d8d..31e0157b7491 100644 --- a/exporter/datadogexporter/translate_traces.go +++ b/exporter/datadogexporter/translate_traces.go @@ -60,7 +60,7 @@ const ( ) // converts Traces into an array of datadog trace payloads grouped by env -func convertToDatadogTd(td pdata.Traces, calculator *sublayerCalculator, cfg *config.Config) ([]*pb.TracePayload, []datadog.Metric) { +func convertToDatadogTd(td pdata.Traces, calculator *sublayerCalculator, cfg *config.Config, blk *Denylister) ([]*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 @@ -80,7 +80,7 @@ func convertToDatadogTd(td pdata.Traces, calculator *sublayerCalculator, cfg *co hostname = resHostname } - payload := resourceSpansToDatadogSpans(rs, calculator, hostname, cfg) + payload := resourceSpansToDatadogSpans(rs, calculator, hostname, cfg, blk) traces = append(traces, &payload) ms := metrics.DefaultMetrics("traces", uint64(pushTime)) @@ -118,7 +118,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) pb.TracePayload { +func resourceSpansToDatadogSpans(rs pdata.ResourceSpans, calculator *sublayerCalculator, hostname string, cfg *config.Config, blk *Denylister) pb.TracePayload { // get env tag env := utils.NormalizeTag(cfg.Env) @@ -172,6 +172,27 @@ func resourceSpansToDatadogSpans(rs pdata.ResourceSpans, calculator *sublayerCal } for _, apiTrace := range apiTraces { + // first drop trace if root span exists in trace chunk that is on denylist (drop trace no stats). + // In the dd-agent, the denylist/blacklist behavior can be performed before most of the expensive + // operations on the span. + // See: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/agent/agent.go#L200 + // However, in our case, the span must be converted from otlp span into a dd span first. This is for 2 reasons. + // First, DD trace chunks rec'd by datadog-agent v0.4+ endpoint are expected as arrays of spans grouped by trace id. + // But, since OTLP groups by arrays of spans from the same instrumentation library, not trace-id, + // (contrib-instrumention-redis, contrib-instrumention-rails, etc), we have to iterate + // over batch and group all spans by trace id. + // Second, otlp->dd conversion is what creates the resource name that we are checking in the denylist. + // Note: OTLP also groups by ResourceSpans but practically speaking a payload will usually only + // contain 1 ResourceSpan array. + + // Root span is used to carry some trace-level metadata, such as sampling rate and priority. + rootSpan := utils.GetRoot(apiTrace) + + if !blk.Allows(rootSpan) { + // drop trace by not adding to payload if it's root span matches denylist + continue + } + // calculates analyzed spans for use in trace search and app analytics // appends a specific piece of metadata to these spans marking them as analyzed // TODO: allow users to configure specific spans to be marked as an analyzed spans for app analytics @@ -249,11 +270,13 @@ func spanToDatadogSpan(s pdata.Span, // we can then set Error field when creating and predefine a max meta capacity isSpanError := getSpanErrorAndSetTags(s, tags) + resourceName := getDatadogResourceName(s, tags) + span := &pb.Span{ TraceID: decodeAPMTraceID(s.TraceID().Bytes()), SpanID: decodeAPMSpanID(s.SpanID().Bytes()), Name: getDatadogSpanName(s, tags), - Resource: getDatadogResourceName(s, tags), + Resource: resourceName, Service: normalizedServiceName, Start: int64(startTime), Duration: duration, diff --git a/exporter/datadogexporter/translate_traces_test.go b/exporter/datadogexporter/translate_traces_test.go index c6b60c6dd343..324e5fd8c9e9 100644 --- a/exporter/datadogexporter/translate_traces_test.go +++ b/exporter/datadogexporter/translate_traces_test.go @@ -137,8 +137,9 @@ func TestConvertToDatadogTd(t *testing.T) { traces := pdata.NewTraces() traces.ResourceSpans().AppendEmpty() calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) - outputTraces, runningMetrics := convertToDatadogTd(traces, calculator, &config.Config{}) + outputTraces, runningMetrics := convertToDatadogTd(traces, calculator, &config.Config{}, denylister) assert.Equal(t, 1, len(outputTraces)) assert.Equal(t, 1, len(runningMetrics)) } @@ -146,8 +147,9 @@ func TestConvertToDatadogTd(t *testing.T) { func TestConvertToDatadogTdNoResourceSpans(t *testing.T) { traces := pdata.NewTraces() calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) - outputTraces, runningMetrics := convertToDatadogTd(traces, calculator, &config.Config{}) + outputTraces, runningMetrics := convertToDatadogTd(traces, calculator, &config.Config{}, denylister) assert.Equal(t, 0, len(outputTraces)) assert.Equal(t, 0, len(runningMetrics)) } @@ -171,7 +173,7 @@ func TestRunningTraces(t *testing.T) { rts.AppendEmpty() - _, runningMetrics := convertToDatadogTd(td, newSublayerCalculator(), &config.Config{}) + _, runningMetrics := convertToDatadogTd(td, newSublayerCalculator(), &config.Config{}, NewDenylister([]string{})) runningHostnames := []string{} for _, metric := range runningMetrics { @@ -190,6 +192,7 @@ func TestRunningTraces(t *testing.T) { func TestObfuscation(t *testing.T) { calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) traces := pdata.NewTraces() rs := traces.ResourceSpans().AppendEmpty() @@ -208,7 +211,7 @@ func TestObfuscation(t *testing.T) { // of them is currently not supported. span.Attributes().InsertString("testinfo?=123", "http.route") - outputTraces, _ := convertToDatadogTd(traces, calculator, &config.Config{}) + outputTraces, _ := convertToDatadogTd(traces, calculator, &config.Config{}, denylister) aggregatedTraces := aggregateTracePayloadsByEnv(outputTraces) obfuscator := obfuscate.NewObfuscator(obfuscatorConfig) @@ -221,6 +224,7 @@ func TestObfuscation(t *testing.T) { func TestBasicTracesTranslation(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -233,7 +237,7 @@ func TestBasicTracesTranslation(t *testing.T) { rs := NewResourceSpansData(mockTraceID, mockSpanID, mockParentSpanID, pdata.StatusCodeUnset, false, mockEndTime) // translate mocks to datadog traces - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -288,9 +292,37 @@ func TestBasicTracesTranslation(t *testing.T) { assert.Equal(t, mockEventsString, datadogPayload.Traces[0].Spans[0].Meta["events"]) } +func TestBasicTracesDenylist(t *testing.T) { + hostname := "testhostname" + calculator := newSublayerCalculator() + + // adding some regex bits to the resource name, but this should drop the trace + denylister := NewDenylister([]string{".nd-To-E.d H.re"}) + + // generate mock trace, span and parent span ids + mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} + mockSpanID := [8]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8} + mockParentSpanID := [8]byte{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8} + mockEndTime := time.Now().Round(time.Second) + + // create mock resource span data + // set shouldError and resourceServiceandEnv to false to test defaut behavior + rs := NewResourceSpansData(mockTraceID, mockSpanID, mockParentSpanID, pdata.StatusCodeUnset, false, mockEndTime) + + // translate mocks to datadog traces + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}, denylister) + // ensure we return the correct type + assert.IsType(t, pb.TracePayload{}, datadogPayload) + + // ensure hostname arg is respected + assert.Equal(t, hostname, datadogPayload.HostName) + assert.Equal(t, 0, len(datadogPayload.Traces)) +} + func TestTracesTranslationErrorsAndResource(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -310,7 +342,7 @@ func TestTracesTranslationErrorsAndResource(t *testing.T) { }, } - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -343,6 +375,7 @@ func TestTracesTranslationErrorsAndResource(t *testing.T) { func TestTracesTranslationErrorsFromEventsUsesLast(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -390,7 +423,7 @@ func TestTracesTranslationErrorsFromEventsUsesLast(t *testing.T) { }, } - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // Ensure the error type is copied over from the last error event logged assert.Equal(t, attribs[conventions.AttributeExceptionType].StringVal(), datadogPayload.Traces[0].Spans[0].Meta[ext.ErrorType]) @@ -406,6 +439,7 @@ func TestTracesTranslationErrorsFromEventsUsesLast(t *testing.T) { func TestTracesTranslationErrorsFromEventsBounds(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -444,7 +478,7 @@ func TestTracesTranslationErrorsFromEventsBounds(t *testing.T) { }, } - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // Ensure the error type is copied over assert.Equal(t, attribs[conventions.AttributeExceptionType].StringVal(), datadogPayload.Traces[0].Spans[0].Meta[ext.ErrorType]) @@ -481,6 +515,7 @@ func TestTracesTranslationErrorsFromEventsBounds(t *testing.T) { func TestTracesTranslationOkStatus(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -500,7 +535,7 @@ func TestTracesTranslationOkStatus(t *testing.T) { }, } - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -530,6 +565,7 @@ func TestTracesTranslationOkStatus(t *testing.T) { func TestTracesTranslationConfig(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -550,7 +586,7 @@ func TestTracesTranslationConfig(t *testing.T) { } // translate mocks to datadog traces - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -577,6 +613,7 @@ func TestTracesTranslationConfig(t *testing.T) { func TestTracesTranslationNoIls(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) rs := pdata.NewResourceSpans() @@ -588,7 +625,7 @@ func TestTracesTranslationNoIls(t *testing.T) { } // translate mocks to datadog traces - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -601,6 +638,7 @@ func TestTracesTranslationNoIls(t *testing.T) { func TestTracesTranslationInvalidService(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -638,9 +676,9 @@ func TestTracesTranslationInvalidService(t *testing.T) { } // translate mocks to datadog traces - datadogPayloadInvalidService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgInvalidService) - datadogPayloadEmptyService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgEmptyService) - datadogPayloadStartWithInvalidService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgStartWithInvalidService) + datadogPayloadInvalidService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgInvalidService, denylister) + datadogPayloadEmptyService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgEmptyService, denylister) + datadogPayloadStartWithInvalidService := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfgStartWithInvalidService, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayloadInvalidService) @@ -659,6 +697,7 @@ func TestTracesTranslationInvalidService(t *testing.T) { func TestTracesTranslationServicePeerName(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -674,7 +713,7 @@ func TestTracesTranslationServicePeerName(t *testing.T) { span.Attributes().InsertString(conventions.AttributePeerService, "my_peer_service_name") // translate mocks to datadog traces - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -733,6 +772,7 @@ func TestTracesTranslationServicePeerName(t *testing.T) { func TestTracesTranslationTruncatetag(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -749,7 +789,7 @@ func TestTracesTranslationTruncatetag(t *testing.T) { span.Attributes().InsertString(conventions.AttributeExceptionStacktrace, RandStringBytes(5500)) // translate mocks to datadog traces - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{}, denylister) // ensure we return the correct type assert.IsType(t, pb.TracePayload{}, datadogPayload) @@ -1111,6 +1151,7 @@ func TestTracePayloadAggr(t *testing.T) { func TestStatsAggregations(t *testing.T) { hostname := "testhostname" calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) // generate mock trace, span and parent span ids mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F} @@ -1126,7 +1167,7 @@ func TestStatsAggregations(t *testing.T) { // translate mocks to datadog traces cfg := config.Config{} - datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg) + datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &cfg, denylister) statsOutput := computeAPMStats(&datadogPayload, calculator, time.Now().UTC().UnixNano()) @@ -1147,6 +1188,7 @@ func TestStatsAggregations(t *testing.T) { // ensure that sanitization of trace payloads occurs func TestSanitization(t *testing.T) { calculator := newSublayerCalculator() + denylister := NewDenylister([]string{}) traces := pdata.NewTraces() traces.ResourceSpans().Resize(1) @@ -1162,7 +1204,7 @@ func TestSanitization(t *testing.T) { instrumentationLibrary.SetVersion("v1") ilss.Spans().Resize(1) - outputTraces, _ := convertToDatadogTd(traces, calculator, &config.Config{}) + outputTraces, _ := convertToDatadogTd(traces, calculator, &config.Config{}, denylister) aggregatedTraces := aggregateTracePayloadsByEnv(outputTraces) obfuscator := obfuscate.NewObfuscator(obfuscatorConfig) diff --git a/exporter/datadogexporter/utils/trace_helpers.go b/exporter/datadogexporter/utils/trace_helpers.go index 964b582bb635..3383077616a6 100644 --- a/exporter/datadogexporter/utils/trace_helpers.go +++ b/exporter/datadogexporter/utils/trace_helpers.go @@ -19,6 +19,7 @@ import ( "unicode" "unicode/utf8" + "github.com/DataDog/datadog-agent/pkg/trace/exportable/pb" "go.opentelemetry.io/collector/consumer/pdata" ) @@ -134,6 +135,48 @@ func NormalizeServiceName(service string) string { return s } +// GetRoot extracts the root span from a trace +// From: https://github.com/DataDog/datadog-agent/blob/a6872e436681ea2136cf8a67465e99fdb4450519/pkg/trace/traceutil/trace.go#L27 +func GetRoot(t *pb.APITrace) *pb.Span { + // That should be caught beforehand + spans := t.GetSpans() + if len(spans) == 0 { + return nil + } + // General case: go over all spans and check for one which matching parent + parentIDToChild := map[uint64]*pb.Span{} + + for i := range spans { + // Common case optimization: check for span with ParentID == 0, starting from the end, + // since some clients report the root last + j := len(spans) - 1 - i + if spans[j].ParentID == 0 { + return spans[j] + } + parentIDToChild[spans[j].ParentID] = spans[j] + } + + for i := range spans { + _, ok := parentIDToChild[spans[i].SpanID] + + if ok { + delete(parentIDToChild, spans[i].SpanID) + } + } + + // TODO: pass logger into convertToDatadogTd, so that we can improve debug logging in translation. + // For now it would require some rewrite, but ideally should log if we cant locate a root span + + // Have a safe behavior if there is multiple spans without a parent + // Pick the first span without its parent + for parentID := range parentIDToChild { + return parentIDToChild[parentID] + } + + // Gracefully fail with the last span of the trace + return spans[len(spans)-1] +} + // TruncateUTF8 truncates the given string to make sure it uses less than limit bytes. // If the last character is an utf8 character that would be splitten, it removes it // entirely to make sure the resulting string is not broken. diff --git a/exporter/datadogexporter/utils/trace_helpers_test.go b/exporter/datadogexporter/utils/trace_helpers_test.go index c5c7c9c5bc3e..e6b2c145d162 100644 --- a/exporter/datadogexporter/utils/trace_helpers_test.go +++ b/exporter/datadogexporter/utils/trace_helpers_test.go @@ -20,9 +20,54 @@ import ( "testing" "unicode" + "github.com/DataDog/datadog-agent/pkg/trace/exportable/pb" "github.com/stretchr/testify/assert" ) +// TestGetRootFromCompleteTrace ensures that GetRoot returns a root span or fails gracefully from a complete trace chunk +// From: https://github.com/DataDog/datadog-agent/blob/eb819b117fba4d57ccc35f1a74d4618b57daf8aa/pkg/trace/traceutil/trace_test.go#L15 +func TestGetRootFromCompleteTrace(t *testing.T) { + assert := assert.New(t) + + trace := &pb.APITrace{ + TraceID: uint64(1234), + Spans: []*pb.Span{}, + StartTime: 0, + EndTime: 0, + } + + trace.Spans = []*pb.Span{ + {TraceID: uint64(1234), SpanID: uint64(12341), Service: "s1", Name: "n1", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12342), ParentID: uint64(12341), Service: "s1", Name: "n1", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12343), ParentID: uint64(12341), Service: "s1", Name: "n1", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12344), ParentID: uint64(12342), Service: "s2", Name: "n2", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12345), ParentID: uint64(12344), Service: "s2", Name: "n2", Resource: ""}, + } + + assert.Equal(GetRoot(trace).SpanID, uint64(12341)) +} + +// TestGetRootFromPartialTrace ensures that GetRoot returns a root span or fails gracefully from a partial trace chunk +// From: https://github.com/DataDog/datadog-agent/blob/eb819b117fba4d57ccc35f1a74d4618b57daf8aa/pkg/trace/traceutil/trace_test.go#L29 +func TestGetRootFromPartialTrace(t *testing.T) { + assert := assert.New(t) + + trace := &pb.APITrace{ + TraceID: uint64(1234), + Spans: []*pb.Span{}, + StartTime: 0, + EndTime: 0, + } + + trace.Spans = []*pb.Span{ + {TraceID: uint64(1234), SpanID: uint64(12341), ParentID: uint64(12340), Service: "s1", Name: "n1", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12342), ParentID: uint64(12341), Service: "s1", Name: "n1", Resource: ""}, + {TraceID: uint64(1234), SpanID: uint64(12343), ParentID: uint64(12342), Service: "s2", Name: "n2", Resource: ""}, + } + + assert.Equal(GetRoot(trace).SpanID, uint64(12341)) +} + // ensure that truncation helper function truncates strings as expected // and accounts for the limit and multi byte ending characters // from https://github.com/DataDog/datadog-agent/blob/140a4ee164261ef2245340c50371ba989fbeb038/pkg/trace/traceutil/truncate_test.go#L15