Trace Filtering #309

p-lambert · 2017-08-15T21:44:49Z

This PR enables filtering out certain traces based on their root resource name. The underlying motivation for it is to prevent traces such as those originated from heartbeat requests to be sent to the backend services.

Usage

A blacklist of regular expressions can be set through the agent configuration file or through the environment variable DD_IGNORE_RESOURCE.

Entries must surrounded by double quotes and separated by comas.

INI File

[trace.ignore]
resource: "(GET|POST) /heartbeat", "another_entry", "SELECT COUNT\\(\\*\\) FROM BAR"

ENV variable

DD_IGNORE_RESOURCE='"(GET|POST) /heartbeat", "another_entry"'

The regular expressions are evaluated using the RE2 engine. Accepted syntax is described here.

Performance impact

The performance impact of this feature was measured against two scenarios:

running the agent with a set of filters that match some traces – rather than degrading the performance this turned out to improve it since we avoid computation when we drop a trace;
running the agent with a set of filters that don't match any traces – this is the worst case scenario since we have the overhead of filtering without dropping any trace;

In both cases the impact seems to be negligibe:

BenchmarkAgentTraceProcessing                         	   10000	    120873 ns/op	   50970 B/op	     706 allocs/op
BenchmarkAgentTraceProcessingWithFiltering            	   10000	    114600 ns/op	   45879 B/op	     633 allocs/op
BenchmarkAgentTraceProcessingWithWorstCaseFiltering   	   10000	    125369 ns/op	   51124 B/op	     706 allocs/op

talwai · 2017-08-16T19:23:22Z

agent/agent.go

+func (a *Agent) countAsDropped(t model.Trace) {
+	// We get the address of the struct holding the stats associated to the tags
+	ts := a.Receiver.stats.getTagStats(Tags{})
+	atomic.AddInt64(&ts.TracesDropped, 1)


I think traces discarded to comply with a user request should be counted separately from those discarded for being badly formatted. i.e. let's count TracesFiltered as a distinct thing from TracesDropped.

it also bothers me the amount of coupling between the Agent and the Receiver. This seems to be a classic example of feature envy. Maybe we should abstract this away? @talwai

talwai · 2017-08-16T19:24:14Z

agent/agent.go

@@ -35,6 +35,7 @@ func (pt *processedTrace) weight() float64 {
 type Agent struct {
 	Receiver     *HTTPReceiver
 	Concentrator *Concentrator
+	Filter       Filter


maybe take a []Filter here so it can accommodate several blacklists that check against different fields (name service etc.)

talwai · 2017-08-16T19:25:18Z

agent/agent.go

@@ -56,6 +57,7 @@ func NewAgent(conf *config.AgentConfig) *Agent {
 		conf.ExtraAggregators,
 		conf.BucketInterval.Nanoseconds(),
 	)
+	f := NewResourceFilter(conf)


as per above, call this NewFilters so that it is more generalized. implementing a name- and service- blacklist can be out of the scope PR, but let's make it easy for us to add this in the future

talwai · 2017-08-16T19:26:45Z

config/agent.go

@@ -65,6 +65,8 @@ type AgentConfig struct {

 	// http/s proxying
 	Proxy *ProxySettings
+
+	ResourceBlacklist []string


maybe Blacklist map[string][]string and have the current implementation only populate blacklist["resource"]

talwai · 2017-08-16T19:27:42Z

agent/agent.go


+	if !a.Filter.Keep(root) {


in the new "general" scheme, loop over a.Filters and check against each. As of now there'll be only one

talwai · 2017-08-16T21:35:33Z

filters/base.go

+)
+
+// Interface handles Span filtering
+type Interface interface {


just call it Filter

talwai · 2017-08-16T21:38:13Z

filters/resource.go

+	f.blacklist = blacklist
+
+	return f
+}


just do func NewResource() *Resource

then in filters/base you can do return []Filter{NewResource(conf)}

talwai · 2017-08-16T21:38:54Z

filters/resource.go

+)
+
+// Resource implements a resource-based filter
+type Resource struct {


call this ResourceFilter

maybe it shouldn't be exported?

p-lambert assigned talwai Aug 15, 2017

p-lambert force-pushed the trace-filtering branch 4 times, most recently from a040b7c to 41348fc Compare August 16, 2017 14:25

talwai reviewed Aug 16, 2017

View reviewed changes

filters/base.go Outdated

)

// Interface handles Span filtering

type Interface interface {

Copy link

talwai Aug 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just call it Filter

talwai reviewed Aug 16, 2017

View reviewed changes

p-lambert force-pushed the trace-filtering branch 7 times, most recently from 5bd9879 to 219c31b Compare August 17, 2017 18:43

p-lambert added 3 commits August 17, 2017 14:46

Add trace filtering based on resource name

e2d38bb

Allow GetStrArray to handle values with quoted commas

2cf2a43

Allow ResourceBlacklist to be specified through ENV vars

09ac151

p-lambert force-pushed the trace-filtering branch 2 times, most recently from 32d7386 to 1b78231 Compare August 21, 2017 15:00

Allow multiple filters to be specified

221d4bb

p-lambert force-pushed the trace-filtering branch from 1b78231 to 221d4bb Compare August 21, 2017 16:03

talwai merged commit 9772170 into master Aug 22, 2017

dtilghman deleted the trace-filtering branch August 23, 2017 05:14

DataDog deleted a comment from p-lambert Aug 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trace Filtering #309

Trace Filtering #309

p-lambert commented Aug 15, 2017 •

edited

Loading

talwai Aug 16, 2017

p-lambert Aug 16, 2017

talwai Aug 16, 2017

talwai Aug 16, 2017

talwai Aug 16, 2017

talwai Aug 16, 2017

talwai Aug 16, 2017

talwai Aug 16, 2017 •

edited

Loading

talwai Aug 16, 2017

talwai Aug 16, 2017

Trace Filtering #309

Trace Filtering #309

Conversation

p-lambert commented Aug 15, 2017 • edited Loading

Usage

INI File

ENV variable

Performance impact

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talwai Aug 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-lambert commented Aug 15, 2017 •

edited

Loading

talwai Aug 16, 2017 •

edited

Loading