otelmemcache: Simplify config and span status setting

CHANGELOG.md

@@ -8,6 +8,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Fixed
+
+- `otelmemcache` no longer sets span status to OK instead of leaving it unset. (#477)
+
+### Removed
+
+- Remove service name from `otelmemcache` configuration and span attributes. (#477)
+
 ## [0.14.0] - 2020-11-20
 
 ### Added
@@ -16,7 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The AWS detector now adds the cloud zone, host image ID, host type, and host name to the returned `Resource`. (#410)
 - Add Amazon ECS Resource Detector for AWS X-Ray. (#466)
 - Add propagator for AWS X-Ray (#462)
-  
+
 ### Changed
 
 - Add semantic version to `Tracer` / `Meter` created by instrumentation packages `otelsaram`, `otelrestful`, `otelmongo`, `otelhttp` and `otelhttptrace`. (#412)

instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/config.go

@@ -18,25 +18,11 @@ import (
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
-type config struct {
-	serviceName    string
-	tracerProvider oteltrace.TracerProvider
-}
-
 // Option is used to configure the client.
-type Option func(*config)
+type Option oteltrace.TracerProvider
 
 // WithTracerProvider specifies a tracer provider to use for creating a tracer.
 // If none is specified, the global provider is used.
 func WithTracerProvider(provider oteltrace.TracerProvider) Option {
-	return func(cfg *config) {
-		cfg.tracerProvider = provider
-	}
-}
-
-// WithServiceName sets the service name.
-func WithServiceName(serviceName string) Option {
-	return func(cfg *config) {
-		cfg.serviceName = serviceName
-	}
+	return provider
 }

instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache.go

@@ -21,20 +21,18 @@ import (
 
 	"go.opentelemetry.io/contrib"
 	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/label"
-	"go.opentelemetry.io/otel/semconv"
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
 const (
-	defaultTracerName  = "go.opentelemetry.io/contrib/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache"
-	defaultServiceName = "memcached"
+	tracerName = "go.opentelemetry.io/contrib/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache"
 )
 
 // Client is a wrapper around *memcache.Client.
 type Client struct {
 	*memcache.Client
-	cfg    *config
 	tracer oteltrace.Tracer
 	ctx    context.Context
 }
@@ -49,24 +47,19 @@ type Client struct {
 // error code and message, if an error occurs). Optionally, client context can
 // be set before an operation with the WithContext method.
 func NewClientWithTracing(client *memcache.Client, opts ...Option) *Client {
-	cfg := &config{}
+	var tp oteltrace.TracerProvider
 	for _, o := range opts {
-		o(cfg)
+		tp = o
 	}
 
-	if cfg.serviceName == "" {
-		cfg.serviceName = defaultServiceName
-	}
-
-	if cfg.tracerProvider == nil {
-		cfg.tracerProvider = otel.GetTracerProvider()
+	if tp == nil {
+		tp = otel.GetTracerProvider()
 	}
 
 	return &Client{
 		client,
-		cfg,
-		cfg.tracerProvider.Tracer(
-			defaultTracerName,
+		tp.Tracer(
+			tracerName,
 			oteltrace.WithInstrumentationVersion(contrib.SemVersion()),
 		),
 		context.Background(),
@@ -77,7 +70,6 @@ func NewClientWithTracing(client *memcache.Client, opts ...Option) *Client {
 // of the operation name and item key(s) (if available)
 func (c *Client) attrsByOperationAndItemKey(operation operation, key ...string) []label.KeyValue {
 	labels := []label.KeyValue{
-		semconv.ServiceNameKey.String(c.cfg.serviceName),
 		memcacheDBSystem(),
 		memcacheDBOperation(operation),
 	}
@@ -111,7 +103,7 @@ func (c *Client) startSpan(operationName operation, itemKey ...string) oteltrace
 // Ends span and, if applicable, sets error status
 func endSpan(s oteltrace.Span, err error) {
 	if err != nil {
-		s.SetStatus(memcacheErrToStatusCode(err), err.Error())
+		s.SetStatus(codes.Error, err.Error())
 	}
 	s.End()
 }
@@ -121,7 +113,6 @@ func (c *Client) WithContext(ctx context.Context) *Client {
 	cc := c.Client
 	return &Client{
 		Client: cc,
-		cfg:    c.cfg,
 		tracer: c.tracer,
 		ctx:    ctx,
 	}

instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache_test.go

@@ -26,7 +26,6 @@ import (
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/label"
 	"go.opentelemetry.io/otel/oteltest"
-	"go.opentelemetry.io/otel/semconv"
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
@@ -41,10 +40,7 @@ func TestNewClientWithTracing(t *testing.T) {
 	)
 
 	assert.NotNil(t, c.Client)
-	assert.NotNil(t, c.cfg)
-	assert.NotNil(t, c.cfg.tracerProvider)
 	assert.NotNil(t, c.tracer)
-	assert.Equal(t, defaultServiceName, c.cfg.serviceName)
 }
 
 func TestOperation(t *testing.T) {
@@ -61,10 +57,9 @@ func TestOperation(t *testing.T) {
 	assert.Len(t, spans, 1)
 	assert.Equal(t, oteltrace.SpanKindClient, spans[0].SpanKind())
 	assert.Equal(t, string(operationAdd), spans[0].Name())
-	assert.Len(t, spans[0].Attributes(), 4)
+	assert.Len(t, spans[0].Attributes(), 3)
 
 	expectedLabelMap := map[label.Key]label.Value{
-		semconv.ServiceNameKey:                              semconv.ServiceNameKey.String(defaultServiceName).Value,
 		memcacheDBSystem().Key:                              memcacheDBSystem().Value,
 		memcacheDBOperation(operationAdd).Key:               memcacheDBOperation(operationAdd).Value,
 		label.Key(memcacheDBItemKeyName).String(mi.Key).Key: label.Key(memcacheDBItemKeyName).String(mi.Key).Value,
@@ -83,10 +78,9 @@ func TestOperationWithCacheMissError(t *testing.T) {
 	assert.Len(t, spans, 1)
 	assert.Equal(t, oteltrace.SpanKindClient, spans[0].SpanKind())
 	assert.Equal(t, string(operationGet), spans[0].Name())
-	assert.Len(t, spans[0].Attributes(), 4)
+	assert.Len(t, spans[0].Attributes(), 3)
 
 	expectedLabelMap := map[label.Key]label.Value{
-		semconv.ServiceNameKey:                           semconv.ServiceNameKey.String(defaultServiceName).Value,
 		memcacheDBSystem().Key:                           memcacheDBSystem().Value,
 		memcacheDBOperation(operationGet).Key:            memcacheDBOperation(operationGet).Value,
 		label.Key(memcacheDBItemKeyName).String(key).Key: label.Key(memcacheDBItemKeyName).String(key).Value,