diff --git a/CHANGELOG.md b/CHANGELOG.md index 331ef8d93da..f31b049836f 100644 --- a/CHANGELOG.md +++ b/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.17.0] - 2021-02-15 ### Added diff --git a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/config.go b/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/config.go index d83677b4752..68c3ff2ede2 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/config.go +++ b/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/config.go @@ -19,7 +19,6 @@ import ( ) type config struct { - serviceName string tracerProvider oteltrace.TracerProvider } @@ -33,10 +32,3 @@ func WithTracerProvider(provider oteltrace.TracerProvider) Option { cfg.tracerProvider = provider } } - -// WithServiceName sets the service name. -func WithServiceName(serviceName string) Option { - return func(cfg *config) { - cfg.serviceName = serviceName - } -} diff --git a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache.go b/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache.go index f1bf5ee8b91..217dfc7ce14 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache.go +++ b/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 } @@ -54,19 +52,14 @@ func NewClientWithTracing(client *memcache.Client, opts ...Option) *Client { o(cfg) } - if cfg.serviceName == "" { - cfg.serviceName = defaultServiceName - } - if cfg.tracerProvider == nil { cfg.tracerProvider = otel.GetTracerProvider() } return &Client{ client, - cfg, cfg.tracerProvider.Tracer( - defaultTracerName, + 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, } diff --git a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache_test.go b/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache_test.go index 2166a13ad91..a9621ce62c2 100644 --- a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/gomemcache_test.go +++ b/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, diff --git a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/status.go b/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/status.go deleted file mode 100644 index 063f9a41ca2..00000000000 --- a/instrumentation/github.com/bradfitz/gomemcache/memcache/otelmemcache/status.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package otelmemcache - -import ( - "github.com/bradfitz/gomemcache/memcache" - - "go.opentelemetry.io/otel/codes" -) - -// maps memcache error to appropriate error code; otherwise returns status OK -func memcacheErrToStatusCode(err error) codes.Code { - if err == nil { - return codes.Ok - } - - switch err { - case memcache.ErrCacheMiss, memcache.ErrNotStored, memcache.ErrNoStats: - return codes.Error - case memcache.ErrCASConflict: - return codes.Error - case memcache.ErrServerError: - return codes.Error - case memcache.ErrMalformedKey: - return codes.Error - default: - return codes.Unset - } -}