Increase the visibility of the api/key package (#650)

* Point to the convenience functions in api/key package This is to increase the visibility of the api/key package through the api/core package, otherwise developers often tend to miss the api/key package altogether and write `core.Key(name).TYPE(value)` and complain at the verbosity of such a construction. The api/key package would allow them to write `key.TYPE(name, value)`. * Use the api/key package where applicable This transforms all the uses of `core.Key(name).TYPE(value)` to `key.TYPE(name, value)`. This also should help increasing the visibility of the api/key package for developers reading the otel-go code. Co-authored-by: Joshua MacDonald <[email protected]>
open-telemetry · Apr 22, 2020 · 927d915 · 927d915
1 parent 395440d
commit 927d915
Show file tree

Hide file tree

Showing 21 changed files with 150 additions and 88 deletions.
diff --git a/api/core/doc.go b/api/core/doc.go
@@ -14,4 +14,7 @@
 
 // This package provides basic types used in OpenTelemetry - keys,
 // values, numbers and span contexts.
+//
+// See the api/key package for convenience functions for creating keys
+// and key-value pairs.
 package core // import "go.opentelemetry.io/otel/api/core"
diff --git a/api/core/key.go b/api/core/key.go
@@ -139,6 +139,11 @@ func Uint(v uint) Value {
 }
 
 // Bool creates a KeyValue instance with a BOOL Value.
+//
+// If creating both key and a bool value at the same time, then
+// instead of calling core.Key(name).Bool(value) consider using a
+// convenience function provided by the api/key package -
+// key.Bool(name, value).
 func (k Key) Bool(v bool) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -147,6 +152,11 @@ func (k Key) Bool(v bool) KeyValue {
 }
 
 // Int64 creates a KeyValue instance with an INT64 Value.
+//
+// If creating both key and an int64 value at the same time, then
+// instead of calling core.Key(name).Int64(value) consider using a
+// convenience function provided by the api/key package -
+// key.Int64(name, value).
 func (k Key) Int64(v int64) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -155,6 +165,11 @@ func (k Key) Int64(v int64) KeyValue {
 }
 
 // Uint64 creates a KeyValue instance with a UINT64 Value.
+//
+// If creating both key and a uint64 value at the same time, then
+// instead of calling core.Key(name).Uint64(value) consider using a
+// convenience function provided by the api/key package -
+// key.Uint64(name, value).
 func (k Key) Uint64(v uint64) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -163,6 +178,11 @@ func (k Key) Uint64(v uint64) KeyValue {
 }
 
 // Float64 creates a KeyValue instance with a FLOAT64 Value.
+//
+// If creating both key and a float64 value at the same time, then
+// instead of calling core.Key(name).Float64(value) consider using a
+// convenience function provided by the api/key package -
+// key.Float64(name, value).
 func (k Key) Float64(v float64) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -171,6 +191,11 @@ func (k Key) Float64(v float64) KeyValue {
 }
 
 // Int32 creates a KeyValue instance with an INT32 Value.
+//
+// If creating both key and an int32 value at the same time, then
+// instead of calling core.Key(name).Int32(value) consider using a
+// convenience function provided by the api/key package -
+// key.Int32(name, value).
 func (k Key) Int32(v int32) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -179,6 +204,11 @@ func (k Key) Int32(v int32) KeyValue {
 }
 
 // Uint32 creates a KeyValue instance with a UINT32 Value.
+//
+// If creating both key and a uint32 value at the same time, then
+// instead of calling core.Key(name).Uint32(value) consider using a
+// convenience function provided by the api/key package -
+// key.Uint32(name, value).
 func (k Key) Uint32(v uint32) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -187,6 +217,11 @@ func (k Key) Uint32(v uint32) KeyValue {
 }
 
 // Float32 creates a KeyValue instance with a FLOAT32 Value.
+//
+// If creating both key and a float32 value at the same time, then
+// instead of calling core.Key(name).Float32(value) consider using a
+// convenience function provided by the api/key package -
+// key.Float32(name, value).
 func (k Key) Float32(v float32) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -195,6 +230,11 @@ func (k Key) Float32(v float32) KeyValue {
 }
 
 // String creates a KeyValue instance with a STRING Value.
+//
+// If creating both key and a string value at the same time, then
+// instead of calling core.Key(name).String(value) consider using a
+// convenience function provided by the api/key package -
+// key.String(name, value).
 func (k Key) String(v string) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -204,15 +244,25 @@ func (k Key) String(v string) KeyValue {
 
 // Int creates a KeyValue instance with either an INT32 or an INT64
 // Value, depending on whether the int type is 32 or 64 bits wide.
+//
+// If creating both key and an int value at the same time, then
+// instead of calling core.Key(name).Int(value) consider using a
+// convenience function provided by the api/key package -
+// key.Int(name, value).
 func (k Key) Int(v int) KeyValue {
 	return KeyValue{
 		Key:   k,
 		Value: Int(v),
 	}
 }
 
-// Uint creates a KeyValue instance with either an UINT32 or an UINT64
+// Uint creates a KeyValue instance with either a UINT32 or a UINT64
 // Value, depending on whether the uint type is 32 or 64 bits wide.
+//
+// If creating both key and a uint value at the same time, then
+// instead of calling core.Key(name).Uint(value) consider using a
+// convenience function provided by the api/key package -
+// key.Uint(name, value).
 func (k Key) Uint(v uint) KeyValue {
 	return KeyValue{
 		Key:   k,
@@ -230,7 +280,7 @@ func (v *Value) Type() ValueType {
 	return v.vtype
 }
 
-// Bool returns the bool value. Make sure that the Value's type is
+// AsBool returns the bool value. Make sure that the Value's type is
 // BOOL.
 func (v *Value) AsBool() bool {
 	return rawToBool(v.numeric)

diff --git a/api/testharness/harness.go b/api/testharness/harness.go
@@ -24,6 +24,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	"go.opentelemetry.io/otel/api/core"
+	"go.opentelemetry.io/otel/api/key"
 	"go.opentelemetry.io/otel/api/trace"
 	"go.opentelemetry.io/otel/internal/matchers"
 )
@@ -329,7 +330,7 @@ func (h *Harness) testSpan(tracerFactory func() trace.Tracer) {
 			span.SetName("new name")
 		},
 		"#SetAttributes": func(span trace.Span) {
-			span.SetAttributes(core.Key("key1").String("value"), core.Key("key2").Int(123))
+			span.SetAttributes(key.String("key1", "value"), key.Int("key2", 123))
 		},
 	}
 	var mechanisms = map[string]func() trace.Span{

diff --git a/api/trace/testtrace/span_test.go b/api/trace/testtrace/span_test.go
@@ -25,6 +25,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	"go.opentelemetry.io/otel/api/core"
+	"go.opentelemetry.io/otel/api/key"
 	"go.opentelemetry.io/otel/api/trace"
 	"go.opentelemetry.io/otel/api/trace/testtrace"
 	"go.opentelemetry.io/otel/internal/matchers"
@@ -339,9 +340,9 @@ func TestSpan(t *testing.T) {
 			subject, ok := span.(*testtrace.Span)
 			e.Expect(ok).ToBeTrue()
 
-			attr1 := core.Key("key1").String("value1")
-			attr2 := core.Key("key2").String("value2")
-			attr3 := core.Key("key3").String("value3")
+			attr1 := key.String("key1", "value1")
+			attr2 := key.String("key2", "value2")
+			attr3 := key.String("key3", "value3")
 			unexpectedAttr := attr2.Key.String("unexpected")
 
 			subject.SetAttributes(attr1, unexpectedAttr, attr3)
@@ -365,7 +366,7 @@ func TestSpan(t *testing.T) {
 			subject, ok := span.(*testtrace.Span)
 			e.Expect(ok).ToBeTrue()
 
-			expectedAttr := core.Key("key").String("value")
+			expectedAttr := key.String("key", "value")
 			subject.SetAttributes(expectedAttr)
 			subject.End()
 
@@ -395,7 +396,7 @@ func TestSpan(t *testing.T) {
 			go func() {
 				defer wg.Done()
 
-				subject.SetAttributes(core.Key("key").String("value"))
+				subject.SetAttributes(key.String("key", "value"))
 			}()
 
 			go func() {
@@ -452,8 +453,8 @@ func TestSpan(t *testing.T) {
 
 			event1Name := "event1"
 			event1Attributes := []core.KeyValue{
-				core.Key("event1Attr1").String("foo"),
-				core.Key("event1Attr2").String("bar"),
+				key.String("event1Attr1", "foo"),
+				key.String("event1Attr2", "bar"),
 			}
 
 			event1Start := time.Now()
@@ -463,7 +464,7 @@ func TestSpan(t *testing.T) {
 			event2Timestamp := time.Now().AddDate(5, 0, 0)
 			event2Name := "event1"
 			event2Attributes := []core.KeyValue{
-				core.Key("event2Attr").String("abc"),
+				key.String("event2Attr", "abc"),
 			}
 
 			subject.AddEventWithTimestamp(context.Background(), event2Timestamp, event2Name, event2Attributes...)

diff --git a/api/trace/testtrace/tracer_test.go b/api/trace/testtrace/tracer_test.go
@@ -61,8 +61,8 @@ func TestTracer(t *testing.T) {
 
 			e := matchers.NewExpecter(t)
 
-			attr1 := core.Key("a").String("1")
-			attr2 := core.Key("b").String("2")
+			attr1 := key.String("a", "1")
+			attr2 := key.String("b", "2")
 
 			subject := testtrace.NewTracer()
 			_, span := subject.Start(context.Background(), "test", trace.WithAttributes(attr1, attr2))
@@ -227,15 +227,15 @@ func TestTracer(t *testing.T) {
 			link1 := trace.Link{
 				SpanContext: span.SpanContext(),
 				Attributes: []core.KeyValue{
-					core.Key("a").String("1"),
+					key.String("a", "1"),
 				},
 			}
 
 			_, span = subject.Start(context.Background(), "link2")
 			link2 := trace.Link{
 				SpanContext: span.SpanContext(),
 				Attributes: []core.KeyValue{
-					core.Key("b").String("2"),
+					key.String("b", "2"),
 				},
 			}
 
@@ -268,8 +268,8 @@ func TestTracer(t *testing.T) {
 
 			e := matchers.NewExpecter(t)
 
-			attr1 := core.Key("a").String("1")
-			attr2 := core.Key("b").String("2")
+			attr1 := key.String("a", "1")
+			attr2 := key.String("b", "2")
 
 			subject := testtrace.NewTracer()
 			var span trace.Span

diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go
@@ -285,8 +285,8 @@ func (s *MockSpan) RecordError(ctx context.Context, err error, opts ...oteltrace
 	}
 
 	s.AddEventWithTimestamp(ctx, cfg.Timestamp, "error",
-		otelcore.Key("error.type").String(reflect.TypeOf(err).String()),
-		otelcore.Key("error.message").String(err.Error()),
+		otelkey.String("error.type", reflect.TypeOf(err).String()),
+		otelkey.String("error.message", err.Error()),
 	)
 }
 

diff --git a/example/basic/main.go b/example/basic/main.go
@@ -47,7 +47,7 @@ func initTracer() {
 	}
 	tp, err := sdktrace.NewProvider(sdktrace.WithSyncer(exp),
 		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
-		sdktrace.WithResourceAttributes(core.Key("rk1").String("rv11"), core.Key("rk2").Int64(5)))
+		sdktrace.WithResourceAttributes(key.String("rk1", "rv11"), key.Int64("rk2", 5)))
 	if err != nil {
 		log.Panicf("failed to initialize trace provider %v", err)
 	}

diff --git a/exporters/otlp/internal/transform/attribute_test.go b/exporters/otlp/internal/transform/attribute_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/otel/api/core"
+	"go.opentelemetry.io/otel/api/key"
 )
 
 func TestAttributes(t *testing.T) {
@@ -31,16 +32,16 @@ func TestAttributes(t *testing.T) {
 		{nil, nil},
 		{
 			[]core.KeyValue{
-				core.Key("int to int").Int(123),
-				core.Key("uint to int").Uint(1234),
-				core.Key("int32 to int").Int32(12345),
-				core.Key("uint32 to int").Uint32(123456),
-				core.Key("int64 to int64").Int64(1234567),
-				core.Key("uint64 to int64").Uint64(12345678),
-				core.Key("float32 to double").Float32(3.14),
-				core.Key("float64 to double").Float32(1.61),
-				core.Key("string to string").String("string"),
-				core.Key("bool to bool").Bool(true),
+				key.Int("int to int", 123),
+				key.Uint("uint to int", 1234),
+				key.Int32("int32 to int", 12345),
+				key.Uint32("uint32 to int", 123456),
+				key.Int64("int64 to int64", 1234567),
+				key.Uint64("uint64 to int64", 12345678),
+				key.Float32("float32 to double", 3.14),
+				key.Float32("float64 to double", 1.61),
+				key.String("string to string", "string"),
+				key.Bool("bool to bool", true),
 			},
 			[]*commonpb.AttributeKeyValue{
 				{

diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/otel/api/core"
+	"go.opentelemetry.io/otel/api/key"
 	"go.opentelemetry.io/otel/api/metric"
 	"go.opentelemetry.io/otel/api/unit"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
@@ -47,16 +48,16 @@ func TestStringKeyValues(t *testing.T) {
 		},
 		{
 			[]core.KeyValue{
-				core.Key("true").Bool(true),
-				core.Key("one").Int64(1),
-				core.Key("two").Uint64(2),
-				core.Key("three").Float64(3),
-				core.Key("four").Int32(4),
-				core.Key("five").Uint32(5),
-				core.Key("six").Float32(6),
-				core.Key("seven").Int(7),
-				core.Key("eight").Uint(8),
-				core.Key("the").String("final word"),
+				key.Bool("true", true),
+				key.Int64("one", 1),
+				key.Uint64("two", 2),
+				key.Float64("three", 3),
+				key.Int32("four", 4),
+				key.Uint32("five", 5),
+				key.Float32("six", 6),
+				key.Int("seven", 7),
+				key.Uint("eight", 8),
+				key.String("the", "final word"),
 			},
 			[]*commonpb.StringKeyValue{
 				{Key: "true", Value: "true"},
@@ -130,7 +131,7 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) {
 			"test-b-description",
 			unit.Bytes,
 			core.Float64NumberKind, // This shouldn't change anything.
-			[]core.KeyValue{core.Key("A").String("1")},
+			[]core.KeyValue{key.String("A", "1")},
 			&metricpb.MetricDescriptor{
 				Name:        "mmsc-test-b",
 				Description: "test-b-description",
@@ -232,7 +233,7 @@ func TestSumMetricDescriptor(t *testing.T) {
 			"test-b-description",
 			unit.Milliseconds,
 			core.Float64NumberKind,
-			[]core.KeyValue{core.Key("A").String("1")},
+			[]core.KeyValue{key.String("A", "1")},
 			&metricpb.MetricDescriptor{
 				Name:        "sum-test-b",
 				Description: "test-b-description",

diff --git a/exporters/otlp/internal/transform/resource_test.go b/exporters/otlp/internal/transform/resource_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/otel/api/core"
+	"go.opentelemetry.io/otel/api/key"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
@@ -38,7 +39,7 @@ func TestEmptyResource(t *testing.T) {
  */
 
 func TestResourceAttributes(t *testing.T) {
-	attrs := []core.KeyValue{core.Key("one").Int(1), core.Key("two").Int(2)}
+	attrs := []core.KeyValue{key.Int("one", 1), key.Int("two", 2)}
 
 	got := Resource(resource.New(attrs...)).GetAttributes()
 	if !assert.Len(t, attrs, 2) {