Remove process config for Jaeger exporter (#1776)

* remove process config for Jaeger exporter * remove process config for Jaeger exporter * Add CHANGELOG.md back * Add CHANGLOG.md changes back * fill in PR number in CHANGELOG * Update CHANGELOG.md Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
open-telemetry · Apr 6, 2021 · 23422c5 · 23422c5
1 parent 0d49b59
commit 23422c5
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 267 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -74,6 +74,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `HasRemoteParent` field of the `"go.opentelemetry.io/otel/sdk/trace".SamplingParameters` is removed.
   This field is redundant to the information returned from the `Remote` method of the `SpanContext` held in the `ParentContext` field. (#1749)
 - The `trace.FlagsDebug` and `trace.FlagsDeferred` constants have been removed and will be localized to the B3 propagator. (#1770)
+- Remove `Process` configuration, `WithProcessFromEnv` and `ProcessFromEnv`, from the Jaeger exporter package.
+  The information that could be configured in the `Process` struct should be configured in a `Resource` instead. (#1776)
 
 ## [0.19.0] - 2021-03-18
 

diff --git a/exporters/trace/jaeger/env.go b/exporters/trace/jaeger/env.go
@@ -20,7 +20,6 @@ import (
 	"strconv"
 	"strings"
 
-	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 )
 
@@ -71,38 +70,6 @@ func WithDisabledFromEnv() Option {
 	}
 }
 
-// ProcessFromEnv parse environment variables into jaeger exporter's Process.
-// It will return a nil tag slice if the environment variable JAEGER_TAGS is malformed.
-func ProcessFromEnv() Process {
-	var p Process
-	if e := os.Getenv(envServiceName); e != "" {
-		p.ServiceName = e
-	}
-	if e := os.Getenv(envTags); e != "" {
-		tags, err := parseTags(e)
-		if err != nil {
-			otel.Handle(err)
-		} else {
-			p.Tags = tags
-		}
-	}
-
-	return p
-}
-
-// WithProcessFromEnv uses environment variables and overrides jaeger exporter's Process.
-func WithProcessFromEnv() Option {
-	return func(o *options) {
-		p := ProcessFromEnv()
-		if p.ServiceName != "" {
-			o.Process.ServiceName = p.ServiceName
-		}
-		if len(p.Tags) != 0 {
-			o.Process.Tags = p.Tags
-		}
-	}
-}
-
 var errTagValueNotFound = errors.New("missing tag value")
 var errTagEnvironmentDefaultValueNotFound = errors.New("missing default value for tag environment value")
 

diff --git a/exporters/trace/jaeger/env_test.go b/exporters/trace/jaeger/env_test.go
@@ -229,13 +229,10 @@ func TestNewRawExporterWithEnv(t *testing.T) {
 		WithCollectorEndpoint(CollectorEndpointFromEnv(), WithCollectorEndpointOptionFromEnv()),
 		WithDisabled(true),
 		WithDisabledFromEnv(),
-		WithProcessFromEnv(),
 	)
 
 	assert.NoError(t, err)
 	assert.Equal(t, false, exp.o.Disabled)
-	assert.EqualValues(t, serviceName, exp.o.Process.ServiceName)
-	assert.Len(t, exp.o.Process.Tags, 1)
 
 	require.IsType(t, &collectorUploader{}, exp.uploader)
 	uploader := exp.uploader.(*collectorUploader)
@@ -276,8 +273,6 @@ func TestNewRawExporterWithEnvImplicitly(t *testing.T) {
 	assert.NoError(t, err)
 	// NewRawExporter will ignore Disabled env
 	assert.Equal(t, true, exp.o.Disabled)
-	assert.EqualValues(t, serviceName, exp.o.Process.ServiceName)
-	assert.Len(t, exp.o.Process.Tags, 1)
 
 	require.IsType(t, &collectorUploader{}, exp.uploader)
 	uploader := exp.uploader.(*collectorUploader)
@@ -394,119 +389,3 @@ func TestWithDisabledFromEnv(t *testing.T) {
 		})
 	}
 }
-
-func TestProcessFromEnv(t *testing.T) {
-	testCases := []struct {
-		name            string
-		serviceName     string
-		tags            string
-		expectedProcess Process
-	}{
-		{
-			name:        "set process",
-			serviceName: "test-service",
-			tags:        "key=value,key2=123",
-			expectedProcess: Process{
-				ServiceName: "test-service",
-				Tags: []attribute.KeyValue{
-					attribute.String("key", "value"),
-					attribute.Int64("key2", 123),
-				},
-			},
-		},
-		{
-			name:        "malformed tags",
-			serviceName: "test-service",
-			tags:        "key",
-			expectedProcess: Process{
-				ServiceName: "test-service",
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			envStore, err := ottest.SetEnvVariables(map[string]string{
-				envServiceName: tc.serviceName,
-				envTags:        tc.tags,
-			})
-			require.NoError(t, err)
-
-			p := ProcessFromEnv()
-			assert.Equal(t, tc.expectedProcess, p)
-
-			require.NoError(t, envStore.Restore())
-		})
-	}
-}
-
-func TestWithProcessFromEnv(t *testing.T) {
-	testCases := []struct {
-		name            string
-		envServiceName  string
-		envTags         string
-		options         options
-		expectedOptions options
-	}{
-		{
-			name:           "overwriting",
-			envServiceName: "service-name",
-			envTags:        "key=value",
-			options: options{
-				Process: Process{
-					ServiceName: "old-name",
-					Tags: []attribute.KeyValue{
-						attribute.String("old-key", "old-value"),
-					},
-				},
-			},
-			expectedOptions: options{
-				Process: Process{
-					ServiceName: "service-name",
-					Tags: []attribute.KeyValue{
-						attribute.String("key", "value"),
-					},
-				},
-			},
-		},
-		{
-			name:           "no overwriting",
-			envServiceName: "",
-			envTags:        "",
-			options: options{
-				Process: Process{
-					ServiceName: "old-name",
-					Tags: []attribute.KeyValue{
-						attribute.String("old-key", "old-value"),
-					},
-				},
-			},
-			expectedOptions: options{
-				Process: Process{
-					ServiceName: "old-name",
-					Tags: []attribute.KeyValue{
-						attribute.String("old-key", "old-value"),
-					},
-				},
-			},
-		},
-	}
-
-	envStore := ottest.NewEnvStore()
-	envStore.Record(envServiceName)
-	envStore.Record(envTags)
-	defer func() {
-		require.NoError(t, envStore.Restore())
-	}()
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			require.NoError(t, os.Setenv(envServiceName, tc.envServiceName))
-			require.NoError(t, os.Setenv(envTags, tc.envTags))
-
-			f := WithProcessFromEnv()
-			f(&tc.options)
-
-			assert.Equal(t, tc.expectedOptions, tc.options)
-		})
-	}
-}
diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go
@@ -49,8 +49,6 @@ type Option func(*options)
 
 // options are the options to be used when initializing a Jaeger export.
 type options struct {
-	// Process contains the information about the exporting process.
-	Process Process
 
 	// BufferMaxCount defines the total number of traces that can be buffered in memory
 	BufferMaxCount int
@@ -104,7 +102,6 @@ func NewRawExporter(endpointOption EndpointOption, opts ...Option) (*Exporter, e
 	}
 
 	o := options{}
-	opts = append(opts, WithProcessFromEnv())
 	for _, opt := range opts {
 		opt(&o)
 	}
@@ -120,10 +117,9 @@ func NewRawExporter(endpointOption EndpointOption, opts ...Option) (*Exporter, e
 	}
 
 	e := &Exporter{
-		uploader:            uploader,
-		o:                   o,
-		defaultServiceName:  defaultServiceName,
-		resourceFromProcess: processToResource(o.Process),
+		uploader:           uploader,
+		o:                  o,
+		defaultServiceName: defaultServiceName,
 	}
 	bundler := bundler.NewBundler((*export.SpanSnapshot)(nil), func(bundle interface{}) {
 		if err := e.upload(bundle.([]*export.SpanSnapshot)); err != nil {
@@ -201,8 +197,7 @@ type Exporter struct {
 	stoppedMu sync.RWMutex
 	stopped   bool
 
-	defaultServiceName  string
-	resourceFromProcess *resource.Resource
+	defaultServiceName string
 }
 
 var _ export.SpanExporter = (*Exporter)(nil)
@@ -431,7 +426,7 @@ func (e *Exporter) Flush() {
 }
 
 func (e *Exporter) upload(spans []*export.SpanSnapshot) error {
-	batchList := jaegerBatchList(spans, e.defaultServiceName, e.resourceFromProcess)
+	batchList := jaegerBatchList(spans, e.defaultServiceName)
 	for _, batch := range batchList {
 		err := e.uploader.upload(batch)
 		if err != nil {
@@ -444,7 +439,7 @@ func (e *Exporter) upload(spans []*export.SpanSnapshot) error {
 
 // jaegerBatchList transforms a slice of SpanSnapshot into a slice of jaeger
 // Batch.
-func jaegerBatchList(ssl []*export.SpanSnapshot, defaultServiceName string, resourceFromProcess *resource.Resource) []*gen.Batch {
+func jaegerBatchList(ssl []*export.SpanSnapshot, defaultServiceName string) []*gen.Batch {
 	if len(ssl) == 0 {
 		return nil
 	}
@@ -456,16 +451,11 @@ func jaegerBatchList(ssl []*export.SpanSnapshot, defaultServiceName string, reso
 			continue
 		}
 
-		newResource := ss.Resource
-		if resourceFromProcess != nil {
-			// The value from process will overwrite the value from span's resources
-			newResource = resource.Merge(ss.Resource, resourceFromProcess)
-		}
-		resourceKey := newResource.Equivalent()
+		resourceKey := ss.Resource.Equivalent()
 		batch, bOK := batchDict[resourceKey]
 		if !bOK {
 			batch = &gen.Batch{
-				Process: process(newResource, defaultServiceName),
+				Process: process(ss.Resource, defaultServiceName),
 				Spans:   []*gen.Span{},
 			}
 		}
@@ -508,16 +498,3 @@ func process(res *resource.Resource, defaultServiceName string) *gen.Process {
 
 	return &process
 }
-
-func processToResource(process Process) *resource.Resource {
-	var attrs []attribute.KeyValue
-	if process.ServiceName != "" {
-		attrs = append(attrs, semconv.ServiceNameKey.String(process.ServiceName))
-	}
-	attrs = append(attrs, process.Tags...)
-
-	if len(attrs) == 0 {
-		return nil
-	}
-	return resource.NewWithAttributes(attrs...)
-}