Add non-empty string check for attribute keys (open-telemetry#1659)

* Fixes open-telemetry#1657 Add non-empty string check for attribute keys * Update PR number in changelog * Update CHANGELOG.md Co-authored-by: Tyler Yahn <[email protected]> * Update sdk/trace/span.go Co-authored-by: Tyler Yahn <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
atlassian-forks · Mar 8, 2021 · 238e7c6 · 238e7c6
1 parent e9b9aca
commit 238e7c6
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Added
 
 - Added `Marshler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586)
+- Added non-empty string check for trace `Attribute` keys. (#1659)
+
 ### Removed
 
 - Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs.

diff --git a/sdk/trace/span.go b/sdk/trace/span.go
@@ -491,7 +491,9 @@ func (s *span) copyToCappedAttributes(attributes ...attribute.KeyValue) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	for _, a := range attributes {
-		if a.Value.Type() != attribute.INVALID {
+		// Ensure attributes conform to the specification:
+		// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.0.1/specification/common/common.md#attributes
+		if a.Value.Type() != attribute.INVALID && a.Key != "" {
 			s.attributes.add(a)
 		}
 	}

diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
@@ -520,6 +520,41 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
 	}
 }
 
+func TestSetSpanAttributesWithInvalidKey(t *testing.T) {
+	te := NewTestExporter()
+	cfg := Config{SpanLimits: SpanLimits{}}
+	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+
+	span := startSpan(tp, "SpanToSetInvalidKeyOrValue")
+	span.SetAttributes(
+		attribute.Bool("", true),
+		attribute.Bool("key1", false),
+	)
+	got, err := endSpan(te, span)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := &export.SpanSnapshot{
+		SpanContext: trace.SpanContext{
+			TraceID:    tid,
+			TraceFlags: 0x1,
+		},
+		ParentSpanID: sid,
+		Name:         "span0",
+		Attributes: []attribute.KeyValue{
+			attribute.Bool("key1", false),
+		},
+		SpanKind:               trace.SpanKindInternal,
+		HasRemoteParent:        true,
+		DroppedAttributeCount:  0,
+		InstrumentationLibrary: instrumentation.Library{Name: "SpanToSetInvalidKeyOrValue"},
+	}
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SetSpanAttributesWithInvalidKey: -got +want %s", diff)
+	}
+}
+
 func TestEvents(t *testing.T) {
 	te := NewTestExporter()
 	tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty()))