Unexport all of Tracer's fields

All options should be set at construction
time, and should be immutable there on.

CHANGELOG.asciidoc

@@ -37,6 +37,7 @@ https://github.com/elastic/apm-agent-go/compare/v1.15.0...main[View commits]
 - Remove WarningLogger, add Warningf methe to Logger {pull}1205[#(1205)]
 - Replace Sampler with ExtendedSampler {pull}1206[#(1206)]
 - Drop unsampled txs when connected to an APM Server >= 8.0 {pull}1208[#(1208)]
+- Unexport Tracer's fields -- TracerOptions must be used instead {pull}1219[#(1219)]
 
 [[release-notes-1.x]]
 === Go Agent version 1.x

metrics_test.go

@@ -190,19 +190,24 @@ func TestTracerMetricsBusyTracer(t *testing.T) {
 	os.Setenv("ELASTIC_APM_API_BUFFER_SIZE", "10KB")
 	defer os.Unsetenv("ELASTIC_APM_API_BUFFER_SIZE")
 
-	tracer, transport := transporttest.NewRecorderTracer()
-	defer tracer.Close()
-
+	var recorder transporttest.RecorderTransport
 	firstRequestDone := make(chan struct{})
-	tracer.Transport = sendStreamFunc(func(ctx context.Context, r io.Reader) error {
+	transport := sendStreamFunc(func(ctx context.Context, r io.Reader) error {
 		if firstRequestDone != nil {
 			firstRequestDone <- struct{}{}
 			firstRequestDone = nil
 			return nil
 		}
-		return transport.SendStream(ctx, r)
+		return recorder.SendStream(ctx, r)
 	})
 
+	tracer, err := apm.NewTracerOptions(apm.TracerOptions{
+		ServiceName: "transporttest",
+		Transport:   transport,
+	})
+	require.NoError(t, err)
+	defer tracer.Close()
+
 	// Force a complete request to be flushed, preventing metrics from
 	// being added to the request buffer until we unblock the transport.
 	nonblocking := make(chan struct{})
@@ -228,7 +233,7 @@ func TestTracerMetricsBusyTracer(t *testing.T) {
 	tracer.Flush(nil) // wait for possibly-latent flush
 	tracer.Flush(nil) // wait for buffered events to be flushed
 
-	assert.NotZero(t, transport.Payloads().Metrics)
+	assert.NotZero(t, recorder.Payloads().Metrics)
 }
 
 func TestTracerMetricsBuffered(t *testing.T) {

tracer.go

@@ -371,20 +371,11 @@ type compressionOptions struct {
 // a limit to the number of errors that will be buffered, and
 // once that limit has been reached, new errors will be dropped
 // until the queue is drained.
-//
-// The exported fields be altered or replaced any time up until
-// any Tracer methods have been invoked.
 type Tracer struct {
-	Transport transport.Transport
-	Service   struct {
-		Name        string
-		Version     string
-		Environment string
-	}
-
-	process *model.Process
-	system  *model.System
-
+	transport         transport.Transport
+	service           model.Service
+	process           *model.Process
+	system            *model.System
 	active            int32
 	bufferSize        int
 	metricsBufferSize int
@@ -398,6 +389,7 @@ type Tracer struct {
 	breakdownMetrics  *breakdownMetrics
 	profileSender     profileSender
 	versionGetter     majorVersionGetter
+
 	// stats is heap-allocated to ensure correct alignment for atomic access.
 	stats *TracerStats
 
@@ -434,7 +426,12 @@ func NewTracerOptions(opts TracerOptions) (*Tracer, error) {
 
 func newTracer(opts TracerOptions) *Tracer {
 	t := &Tracer{
-		Transport:         opts.Transport,
+		transport: opts.Transport,
+		service: makeService(
+			opts.ServiceName,
+			opts.ServiceVersion,
+			opts.ServiceEnvironment,
+		),
 		process:           &currentProcess,
 		system:            &localSystem,
 		closing:           make(chan struct{}),
@@ -455,9 +452,6 @@ func newTracer(opts TracerOptions) *Tracer {
 			local: make(map[string]func(*instrumentationConfigValues)),
 		},
 	}
-	t.Service.Name = opts.ServiceName
-	t.Service.Version = opts.ServiceVersion
-	t.Service.Environment = opts.ServiceEnvironment
 	t.breakdownMetrics.enabled = opts.breakdownMetrics
 	// Initialise local transaction config.
 	t.setLocalInstrumentationConfig(envRecording, func(cfg *instrumentationConfigValues) {
@@ -889,7 +883,7 @@ func (t *Tracer) loop() {
 				case <-ctx.Done():
 				}
 			}
-			requestResult <- t.Transport.SendStream(ctx, iochanReader)
+			requestResult <- t.transport.SendStream(ctx, iochanReader)
 		}
 	}()
 
@@ -1009,8 +1003,8 @@ func (t *Tracer) loop() {
 			}
 			var configWatcherContext context.Context
 			var watchParams apmconfig.WatchParams
-			watchParams.Service.Name = t.Service.Name
-			watchParams.Service.Environment = t.Service.Environment
+			watchParams.Service.Name = t.service.Name
+			watchParams.Service.Environment = t.service.Environment
 			configWatcherContext, stopConfigWatcher = context.WithCancel(ctx)
 			configChanges = cw.WatchConfig(configWatcherContext, watchParams)
 			// Silence go vet's "possible context leak" false positive.
@@ -1310,13 +1304,12 @@ func (t *Tracer) metadataReader() io.Reader {
 }
 
 func (t *Tracer) encodeRequestMetadata(json *fastjson.Writer) {
-	service := makeService(t.Service.Name, t.Service.Version, t.Service.Environment)
 	json.RawString(`{"system":`)
 	t.system.MarshalFastJSON(json)
 	json.RawString(`,"process":`)
 	t.process.MarshalFastJSON(json)
 	json.RawString(`,"service":`)
-	service.MarshalFastJSON(json)
+	t.service.MarshalFastJSON(json)
 	if cloud := getCloudMetadata(); cloud != nil {
 		json.RawString(`,"cloud":`)
 		cloud.MarshalFastJSON(json)

tracer_test.go

@@ -346,13 +346,17 @@ func TestTracerBufferSize(t *testing.T) {
 	defer os.Unsetenv("ELASTIC_APM_API_REQUEST_SIZE")
 	defer os.Unsetenv("ELASTIC_APM_API_BUFFER_SIZE")
 
-	tracer, recorder := transporttest.NewRecorderTracer()
-	defer tracer.Close()
+	var recorder transporttest.RecorderTransport
 	unblock := make(chan struct{})
-	tracer.Transport = blockedTransport{
-		Transport: tracer.Transport,
-		unblocked: unblock,
-	}
+	tracer, err := apm.NewTracerOptions(apm.TracerOptions{
+		ServiceName: "transporttest",
+		Transport: blockedTransport{
+			Transport: &recorder,
+			unblocked: unblock,
+		},
+	})
+	require.NoError(t, err)
+	defer tracer.Close()
 
 	// Send a bunch of transactions, which will be buffered. Because the
 	// buffer cannot hold all of them we should expect to see some of the
@@ -658,7 +662,7 @@ func TestTracerUnsampledTransactions(t *testing.T) {
 }
 
 func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
-	newTracer := func(srvURL string) *apm.Tracer {
+	newTracer := func(srvURL string) (*apm.Tracer, *transport.HTTPTransport) {
 		os.Setenv("ELASTIC_APM_SERVER_URL", srvURL)
 		defer os.Unsetenv("ELASTIC_APM_SERVER_URL")
 		transport, err := transport.NewHTTPTransport(transport.HTTPTransportOptions{})
@@ -668,7 +672,7 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 			Transport:   transport,
 		})
 		require.NoError(t, err)
-		return tracer
+		return tracer, transport
 	}
 
 	type event struct {
@@ -752,7 +756,7 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 		srv := httptest.NewServer(mux)
 		defer srv.Close()
 
-		tracer := newTracer(srv.URL)
+		tracer, _ := newTracer(srv.URL)
 		generateTx(tracer)
 
 		assert.Equal(t, uint32(200), atomic.LoadUint32(&tCounter))
@@ -766,7 +770,7 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 		srv := httptest.NewServer(mux)
 		defer srv.Close()
 
-		tracer := newTracer(srv.URL)
+		tracer, _ := newTracer(srv.URL)
 		generateTx(tracer)
 
 		assert.Equal(t, uint32(100), atomic.LoadUint32(&tCounter))
@@ -786,7 +790,7 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 		srv := httptest.NewServer(mux)
 		defer srv.Close()
 
-		tracer := newTracer(srv.URL)
+		tracer, transport := newTracer(srv.URL)
 		for i := 0; i < 3; i++ {
 			generateTx(tracer)
 		}
@@ -797,10 +801,9 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 		type majorVersionGetter interface {
 			MajorServerVersion(ctx context.Context, refreshStale bool) uint32
 		}
-		vg := tracer.Transport.(majorVersionGetter)
 		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 		defer cancel()
-		vg.MajorServerVersion(ctx, true)
+		transport.MajorServerVersion(ctx, true)
 		assert.Equal(t, uint32(2), atomic.LoadUint32(&rootCounter))
 
 		// Send 100 sampled and 100 unsampled txs.
@@ -815,7 +818,7 @@ func TestTracerUnsampledTransactionsHTTPTransport(t *testing.T) {
 		srv := httptest.NewServer(mux)
 		defer srv.Close()
 
-		tracer := newTracer(srv.URL)
+		tracer, _ := newTracer(srv.URL)
 		generateTx(tracer)
 
 		assert.Equal(t, uint32(200), atomic.LoadUint32(&tCounter))

transaction_test.go

@@ -33,7 +33,6 @@ import (
 	"go.elastic.co/apm/v2"
 	"go.elastic.co/apm/v2/apmtest"
 	"go.elastic.co/apm/v2/model"
-	"go.elastic.co/apm/v2/transport"
 	"go.elastic.co/apm/v2/transport/transporttest"
 )
 
@@ -551,11 +550,7 @@ func TestTransactionOutcome(t *testing.T) {
 }
 
 func BenchmarkTransaction(b *testing.B) {
-	tracer, err := apm.NewTracer("service", "")
-	require.NoError(b, err)
-
-	tracer.Transport = transport.Discard
-	defer tracer.Close()
+	tracer := apmtest.DiscardTracer
 
 	names := []string{}
 	for i := 0; i < 1000; i++ {

validation_test.go

@@ -41,20 +41,20 @@ import (
 )
 
 func TestValidateServiceName(t *testing.T) {
-	validatePayloadMetadata(t, func(tracer *apm.Tracer) {
-		tracer.Service.Name = strings.Repeat("x", 1025)
+	validatePayloadMetadata(t, func(opts *apm.TracerOptions) {
+		opts.ServiceName = strings.Repeat("x", 1025)
 	})
 }
 
 func TestValidateServiceVersion(t *testing.T) {
-	validatePayloadMetadata(t, func(tracer *apm.Tracer) {
-		tracer.Service.Version = strings.Repeat("x", 1025)
+	validatePayloadMetadata(t, func(opts *apm.TracerOptions) {
+		opts.ServiceVersion = strings.Repeat("x", 1025)
 	})
 }
 
 func TestValidateServiceEnvironment(t *testing.T) {
-	validatePayloadMetadata(t, func(tracer *apm.Tracer) {
-		tracer.Service.Environment = strings.Repeat("x", 1025)
+	validatePayloadMetadata(t, func(opts *apm.TracerOptions) {
+		opts.ServiceEnvironment = strings.Repeat("x", 1025)
 	})
 }
 
@@ -364,20 +364,32 @@ func validateTransaction(t *testing.T, f func(tx *apm.Transaction)) {
 	})
 }
 
-func validatePayloadMetadata(t *testing.T, f func(tracer *apm.Tracer)) {
-	validatePayloads(t, func(tracer *apm.Tracer) {
-		f(tracer)
+func validatePayloadMetadata(t *testing.T, f func(opts *apm.TracerOptions)) {
+	var opts apm.TracerOptions
+	f(&opts)
+	validatePayloadsTracerOptions(t, opts, func(tracer *apm.Tracer) {
 		tracer.StartTransaction("name", "type").End()
 	})
 }
 
 func validatePayloads(t *testing.T, f func(tracer *apm.Tracer)) {
-	tracer, _ := apm.NewTracerOptions(apm.TracerOptions{
-		ServiceName:        "x",
-		ServiceVersion:     "y",
-		ServiceEnvironment: "z",
-		Transport:          &validatingTransport{t: t},
-	})
+	validatePayloadsTracerOptions(t, apm.TracerOptions{}, f)
+}
+
+func validatePayloadsTracerOptions(t *testing.T, opts apm.TracerOptions, f func(tracer *apm.Tracer)) {
+	if opts.ServiceName == "" {
+		opts.ServiceName = "x"
+	}
+	if opts.ServiceVersion == "" {
+		opts.ServiceVersion = "y"
+	}
+	if opts.ServiceEnvironment == "" {
+		opts.ServiceEnvironment = "z"
+	}
+	opts.Transport = &validatingTransport{t: t}
+
+	tracer, err := apm.NewTracerOptions(opts)
+	require.NoError(t, err)
 	defer tracer.Close()
 
 	f(tracer)