From f4a273c2157678847de38e654022c02adfda69d0 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Wed, 2 Dec 2020 13:45:55 +0800 Subject: [PATCH 1/2] Make sanitize_field_names centrally configurable --- config.go | 6 ++++++ config_test.go | 14 ++++++++++++++ modelwriter.go | 6 +++--- tracer.go | 11 ++++++++--- transaction.go | 4 ++++ 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/config.go b/config.go index 7cd0f9c41..5e5411b50 100644 --- a/config.go +++ b/config.go @@ -358,6 +358,11 @@ func (t *Tracer) updateRemoteConfig(logger WarningLogger, old, attrs map[string] cfg.recording = recording }) } + case envSanitizeFieldNames: + matchers := configutil.ParseWildcardPatterns(v) + updates = append(updates, func(cfg *instrumentationConfig) { + cfg.sanitizedFieldNames = matchers + }) case envSpanFramesMinDuration: duration, err := configutil.ParseDuration(v) if err != nil { @@ -487,4 +492,5 @@ type instrumentationConfigValues struct { spanFramesMinDuration time.Duration stackTraceLimit int propagateLegacyHeader bool + sanitizedFieldNames wildcard.Matchers } diff --git a/config_test.go b/config_test.go index d5033a07a..6c91bebe0 100644 --- a/config_test.go +++ b/config_test.go @@ -83,6 +83,20 @@ func TestTracerCentralConfigUpdate(t *testing.T) { assert.Len(t, payloads.Errors, 1) return len(payloads.Errors[0].Exception.Stacktrace) == 1 }) + run("sanitize_field_names", "secret", func(tracer *apmtest.RecordingTracer) bool { + tracer.ResetPayloads() + tracer.SetSanitizedFieldNames("not_secret") + req, _ := http.NewRequest("GET", "http://server.testing/", nil) + req.AddCookie(&http.Cookie{Name: "secret", Value: "top"}) + tx := tracer.StartTransaction("name", "type") + tx.Context.SetHTTPRequest(req) + tx.End() + tracer.Flush(nil) + payloads := tracer.Payloads() + assert.Len(t, payloads.Transactions, 1) + assert.Len(t, payloads.Transactions[0].Context.Request.Cookies, 1) + return payloads.Transactions[0].Context.Request.Cookies[0].Value == "[REDACTED]" + }) } func testTracerCentralConfigUpdate(t *testing.T, serverResponse string, isRemote func(*apmtest.RecordingTracer) bool) { diff --git a/modelwriter.go b/modelwriter.go index d21c8d258..be0a37e3e 100644 --- a/modelwriter.go +++ b/modelwriter.go @@ -126,12 +126,12 @@ func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transact out.Context = td.Context.build() } - if len(w.cfg.sanitizedFieldNames) != 0 && out.Context != nil { + if len(td.sanitizedFieldNames) != 0 && out.Context != nil { if out.Context.Request != nil { - sanitizeRequest(out.Context.Request, w.cfg.sanitizedFieldNames) + sanitizeRequest(out.Context.Request, td.sanitizedFieldNames) } if out.Context.Response != nil { - sanitizeResponse(out.Context.Response, w.cfg.sanitizedFieldNames) + sanitizeResponse(out.Context.Response, td.sanitizedFieldNames) } } } diff --git a/tracer.go b/tracer.go index 36dc70ecc..66dc57484 100644 --- a/tracer.go +++ b/tracer.go @@ -423,6 +423,9 @@ func newTracer(opts TracerOptions) *Tracer { t.setLocalInstrumentationConfig(envUseElasticTraceparentHeader, func(cfg *instrumentationConfigValues) { cfg.propagateLegacyHeader = opts.propagateLegacyHeader }) + t.setLocalInstrumentationConfig(envSanitizeFieldNames, func(cfg *instrumentationConfigValues) { + cfg.sanitizedFieldNames = opts.sanitizedFieldNames + }) if !opts.active { t.active = 0 @@ -439,7 +442,6 @@ func newTracer(opts TracerOptions) *Tracer { cfg.metricsInterval = opts.metricsInterval cfg.requestDuration = opts.requestDuration cfg.requestSize = opts.requestSize - cfg.sanitizedFieldNames = opts.sanitizedFieldNames cfg.disabledMetrics = opts.disabledMetrics cfg.preContext = defaultPreContext cfg.postContext = defaultPostContext @@ -465,7 +467,6 @@ type tracerConfig struct { metricsGatherers []MetricsGatherer contextSetter stacktrace.ContextSetter preContext, postContext int - sanitizedFieldNames wildcard.Matchers disabledMetrics wildcard.Matchers cpuProfileDuration time.Duration cpuProfileInterval time.Duration @@ -572,6 +573,10 @@ func (t *Tracer) SetLogger(logger Logger) { // of the the supplied patterns will have their values redacted. If // SetSanitizedFieldNames is called with no arguments, then no fields // will be redacted. +// +// Configuration via Kibana takes precedence over local configuration, so +// if sanitized_field_names has been configured via Kibana, this call will +// not have any effect until/unless that configuration has been removed. func (t *Tracer) SetSanitizedFieldNames(patterns ...string) error { var matchers wildcard.Matchers if len(patterns) != 0 { @@ -580,7 +585,7 @@ func (t *Tracer) SetSanitizedFieldNames(patterns ...string) error { matchers[i] = configutil.ParseWildcardPattern(p) } } - t.sendConfigCommand(func(cfg *tracerConfig) { + t.setLocalInstrumentationConfig(envSanitizeFieldNames, func(cfg *instrumentationConfigValues) { cfg.sanitizedFieldNames = matchers }) return nil diff --git a/transaction.go b/transaction.go index b5ac7cb33..1ab427995 100644 --- a/transaction.go +++ b/transaction.go @@ -23,6 +23,8 @@ import ( "math/rand" "sync" "time" + + "go.elastic.co/apm/internal/wildcard" ) // StartTransaction returns a new Transaction with the specified @@ -66,6 +68,7 @@ func (t *Tracer) StartTransactionOptions(name, transactionType string, opts Tran tx.stackTraceLimit = instrumentationConfig.stackTraceLimit tx.Context.captureHeaders = instrumentationConfig.captureHeaders tx.propagateLegacyHeader = instrumentationConfig.propagateLegacyHeader + tx.sanitizedFieldNames = instrumentationConfig.sanitizedFieldNames tx.breakdownMetricsEnabled = t.breakdownMetrics.enabled var root bool @@ -343,6 +346,7 @@ type TransactionData struct { stackTraceLimit int breakdownMetricsEnabled bool propagateLegacyHeader bool + sanitizedFieldNames wildcard.Matchers timestamp time.Time mu sync.Mutex From e942c5e0c7d8c8751367f4372e73a460bdafb9b7 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Tue, 8 Dec 2020 17:14:01 +0800 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index cedd466e1..9064cf416 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -24,6 +24,8 @@ endif::[] https://github.com/elastic/apm-agent-go/compare/v1.9.0...master[View commits] - module/apmsql: add tracingDriver.Unwrap method to get underlying driver {pull}#849[#(849)] +- module/apmgopgv10: add support for github.com/go-pg/pg/v10 {pull}857[(#857)] +- Enable central configuration of "sanitize_field_names" {pull}856[(#856)] [[release-notes-1.x]] === Go Agent version 1.x