Tidy up modelwriter (#888)

Some light refactoring to move "context" model building
logic out of modelwriter and into the Context.build method.
There remains some context model building in modelwriter
(related to setting destination service type) because it
requires additional information outside of "context", i.e.
the span type.

Author: axw

context.go

@@ -22,23 +22,25 @@ import (
 	"net/http"
 
 	"go.elastic.co/apm/internal/apmhttputil"
+	"go.elastic.co/apm/internal/wildcard"
 	"go.elastic.co/apm/model"
 )
 
 // Context provides methods for setting transaction and error context.
 //
 // NOTE this is entirely unrelated to the standard library's context.Context.
 type Context struct {
-	model            model.Context
-	request          model.Request
-	requestBody      model.RequestBody
-	requestSocket    model.RequestSocket
-	response         model.Response
-	user             model.User
-	service          model.Service
-	serviceFramework model.Framework
-	captureHeaders   bool
-	captureBodyMask  CaptureBodyMode
+	model               model.Context
+	request             model.Request
+	requestBody         model.RequestBody
+	requestSocket       model.RequestSocket
+	response            model.Response
+	user                model.User
+	service             model.Service
+	serviceFramework    model.Framework
+	captureHeaders      bool
+	captureBodyMask     CaptureBodyMode
+	sanitizedFieldNames wildcard.Matchers
 }
 
 func (c *Context) build() *model.Context {
@@ -52,6 +54,15 @@ func (c *Context) build() *model.Context {
 	default:
 		return nil
 	}
+	if len(c.sanitizedFieldNames) != 0 {
+		if c.model.Request != nil {
+			sanitizeRequest(c.model.Request, c.sanitizedFieldNames)
+		}
+		if c.model.Response != nil {
+			sanitizeResponse(c.model.Response, c.sanitizedFieldNames)
+		}
+
+	}
 	return &c.model
 }
 

error.go

@@ -148,6 +148,7 @@ func (t *Tracer) newError() *Error {
 	if e.recording {
 		e.Timestamp = time.Now()
 		e.Context.captureHeaders = instrumentationConfig.captureHeaders
+		e.Context.sanitizedFieldNames = instrumentationConfig.sanitizedFieldNames
 		e.stackTraceLimit = instrumentationConfig.stackTraceLimit
 	}
 

modelwriter.go

@@ -125,15 +125,6 @@ func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transact
 	if sampled {
 		out.Context = td.Context.build()
 	}
-
-	if len(td.sanitizedFieldNames) != 0 && out.Context != nil {
-		if out.Context.Request != nil {
-			sanitizeRequest(out.Context.Request, td.sanitizedFieldNames)
-		}
-		if out.Context.Response != nil {
-			sanitizeResponse(out.Context.Response, td.sanitizedFieldNames)
-		}
-	}
 }
 
 func (w *modelWriter) buildModelSpan(out *model.Span, span *Span, sd *SpanData) {

sanitizer_test.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -43,34 +44,46 @@ func TestSanitizeRequestResponse(t *testing.T) {
 		req.AddCookie(c)
 	}
 
-	tx, _, _ := apmtest.WithTransaction(func(ctx context.Context) {
+	tx, _, errors := apmtest.WithTransaction(func(ctx context.Context) {
+		e := apm.CaptureError(ctx, errors.New("boom!"))
+		defer e.Send()
+
 		tx := apm.TransactionFromContext(ctx)
 		tx.Context.SetHTTPRequest(req)
+		e.Context.SetHTTPRequest(req)
 
 		h := make(http.Header)
 		h.Add("Set-Cookie", (&http.Cookie{Name: "foo", Value: "bar"}).String())
 		h.Add("Set-Cookie", (&http.Cookie{Name: "baz", Value: "qux"}).String())
 		tx.Context.SetHTTPResponseHeaders(h)
 		tx.Context.SetHTTPStatusCode(http.StatusTeapot)
+		e.Context.SetHTTPResponseHeaders(h)
+		e.Context.SetHTTPStatusCode(http.StatusTeapot)
 	})
 
-	assert.Equal(t, tx.Context.Request.Cookies, model.Cookies{
-		{Name: "Custom-Credit-Card-Number", Value: "[REDACTED]"},
-		{Name: "secret", Value: "[REDACTED]"},
-		{Name: "sessionid", Value: "[REDACTED]"},
-		{Name: "user_id", Value: "456"},
-	})
-	assert.Equal(t, model.Headers{{
-		Key:    "Authorization",
-		Values: []string{"[REDACTED]"},
-	}}, tx.Context.Request.Headers)
-
-	// NOTE: the response includes multiple Set-Cookie headers,
-	// but we only report a single "[REDACTED]" value.
-	assert.Equal(t, model.Headers{{
-		Key:    "Set-Cookie",
-		Values: []string{"[REDACTED]"},
-	}}, tx.Context.Response.Headers)
+	checkContext := func(context *model.Context) {
+		assert.Equal(t, context.Request.Cookies, model.Cookies{
+			{Name: "Custom-Credit-Card-Number", Value: "[REDACTED]"},
+			{Name: "secret", Value: "[REDACTED]"},
+			{Name: "sessionid", Value: "[REDACTED]"},
+			{Name: "user_id", Value: "456"},
+		})
+		assert.Equal(t, model.Headers{{
+			Key:    "Authorization",
+			Values: []string{"[REDACTED]"},
+		}}, context.Request.Headers)
+
+		// NOTE: the response includes multiple Set-Cookie headers,
+		// but we only report a single "[REDACTED]" value.
+		assert.Equal(t, model.Headers{{
+			Key:    "Set-Cookie",
+			Values: []string{"[REDACTED]"},
+		}}, context.Response.Headers)
+	}
+	checkContext(tx.Context)
+	for _, e := range errors {
+		checkContext(e.Context)
+	}
 }
 
 func TestSetSanitizedFieldNamesNone(t *testing.T) {

transaction.go

@@ -23,8 +23,6 @@ import (
 	"math/rand"
 	"sync"
 	"time"
-
-	"go.elastic.co/apm/internal/wildcard"
 )
 
 // StartTransaction returns a new Transaction with the specified
@@ -68,7 +66,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.Context.sanitizedFieldNames = instrumentationConfig.sanitizedFieldNames
 	tx.breakdownMetricsEnabled = t.breakdownMetrics.enabled
 
 	var root bool
@@ -346,7 +344,6 @@ type TransactionData struct {
 	stackTraceLimit         int
 	breakdownMetricsEnabled bool
 	propagateLegacyHeader   bool
-	sanitizedFieldNames     wildcard.Matchers
 	timestamp               time.Time
 
 	mu            sync.Mutex