Add SetLabel, deprecate SetTag

Add Context.SetLabel and SpanContext.SetLabel,
which accept interface{} values. The value may
be a nil, string, bool, or numerical type. Any
other type will be sent as a string using
fmt.Sprint.

The SetTag methods remain, but marked as being
deprecated, with a notice that they will be
removed in a future major version.

CHANGELOG.md

@@ -9,6 +9,7 @@
  - Report error cause(s), add support for errors.Unwrap (#638)
  - Setting `ELASTIC_APM_TRANSACTION_MAX_SPANS` to 0 now disables all spans (#640)
  - module/apmzerolog: add Writer.MinLevel (#641)
+ - Introduce SetLabel and deprecate SetTag (#642)
 
 ## [v1.5.0](https://github.com/elastic/apm-agent-go/releases/tag/v1.5.0)
 

context.go

@@ -71,16 +71,29 @@ func (c *Context) reset() {
 	}
 }
 
-// SetTag sets a tag in the context. Invalid characters
-// ('.', '*', and '"') in the key will be replaced with
-// an underscore.
+// SetTag calls SetLabel(key, value).
+//
+// SetTag is deprecated, and will be removed in a future major version.
 func (c *Context) SetTag(key, value string) {
+	c.SetLabel(key, value)
+}
+
+// SetLabel sets a label in the context.
+//
+// Invalid characters ('.', '*', and '"') in the key will be replaced with
+// underscores.
+//
+// If the value is numerical or boolean, then it will be sent to the server
+// as a JSON number or boolean; otherwise it will converted to a string, using
+// `fmt.Sprint` if necessary. String values longer than 1024 characters will
+// be truncated.
+func (c *Context) SetLabel(key string, value interface{}) {
 	// Note that we do not attempt to de-duplicate the keys.
 	// This is OK, since json.Unmarshal will always take the
 	// final instance.
-	c.model.Tags = append(c.model.Tags, model.StringMapItem{
-		Key:   cleanTagKey(key),
-		Value: truncateString(value),
+	c.model.Tags = append(c.model.Tags, model.IfaceMapItem{
+		Key:   cleanLabelKey(key),
+		Value: makeLabelValue(value),
 	})
 }
 
@@ -94,7 +107,7 @@ func (c *Context) SetCustom(key string, value interface{}) {
 	// This is OK, since json.Unmarshal will always take the
 	// final instance.
 	c.model.Custom = append(c.model.Custom, model.IfaceMapItem{
-		Key:   cleanTagKey(key),
+		Key:   cleanLabelKey(key),
 		Value: value,
 	})
 }

context_test.go

@@ -29,15 +29,22 @@ import (
 	"go.elastic.co/apm/model"
 )
 
-func TestContextTags(t *testing.T) {
+func TestContextLabels(t *testing.T) {
+	type customInt int
 	tx := testSendTransaction(t, func(tx *apm.Transaction) {
-		tx.Context.SetTag("foo", "bar")
-		tx.Context.SetTag("foo", "bar!") // Last instance wins
-		tx.Context.SetTag("bar", "baz")
+		tx.Context.SetTag("foo", "bar")    // deprecated
+		tx.Context.SetLabel("foo", "bar!") // Last instance wins
+		tx.Context.SetLabel("bar", "baz")
+		tx.Context.SetLabel("baz", 123.456)
+		tx.Context.SetLabel("qux", true)
+		tx.Context.SetLabel("quux", customInt(123))
 	})
-	assert.Equal(t, model.StringMap{
+	assert.Equal(t, model.IfaceMap{
 		{Key: "bar", Value: "baz"},
+		{Key: "baz", Value: 123.456},
 		{Key: "foo", Value: "bar!"},
+		{Key: "quux", Value: 123.0},
+		{Key: "qux", Value: true},
 	}, tx.Context.Tags)
 }
 

docs/api.asciidoc

@@ -63,7 +63,7 @@ further describe the transaction.
 [source,go]
 ----
 transaction.Result = "Success"
-transaction.Context.SetTag("region", "us-east-1")
+transaction.Context.SetLabel("region", "us-east-1")
 ----
 
 See <<context-api>> for more details on setting transaction context.
@@ -328,15 +328,31 @@ custom context and tags.
 [[context-set-tag]]
 ==== `func (*Context) SetTag(key, value string)`
 
-SetTag tags the transaction or error with the given key and value. If the
-key contains any special characters (`.`, `*`, `"`), they will be replaced
-with underscores. Values longer than 1024 characters will be truncated.
-Tags will be indexed in Elasticsearch as keyword fields.
+SetTag is equivalent to calling SetLabel with a string value.
 
-TIP: Before using custom tags, ensure you understand the different types of
+NOTE: This function is deprecated, and will be removed in a future major
+version of the agent.
+
+[float]
+[[context-set-label]]
+==== `func (*Context) SetLabel(key string, value interface{})`
+
+SetLabel labels the transaction or error with the given key and value.
+If the key contains any special characters (`.`, `*`, `"`), they will be
+replaced with underscores.
+
+If the value is numerical or boolean, then it will be sent to the server
+as a JSON number or boolean; otherwise it will converted to a string, using
+`fmt.Sprint` if necessary. Numerical and boolean values are supported by
+the server from version 6.7 onwards.
+
+String values longer than 1024 characters will be truncated. Labels are
+indexed in Elasticsearch as keyword fields.
+
+TIP: Before using labels, ensure you understand the different types of
 {apm-overview-ref-v}/metadata.html[metadata] that are available.
 
-WARNING: Avoid defining too many user-specified tags.
+WARNING: Avoid defining too many user-specified labels.
 Defining too many unique fields in an index is a condition that can lead to a
 {ref}/mapping.html#mapping-limit-settings[mapping explosion].
 

docs/instrumenting.asciidoc

@@ -754,7 +754,7 @@ tx := apm.DefaultTracer.StartTransaction("GET /api/v1", "request")
 defer tx.End()
 ...
 tx.Result = "HTTP 2xx"
-tx.Context.SetTag("region", "us-east-1")
+tx.Context.SetLabel("region", "us-east-1")
 ----
 
 The agent supports sampling transactions: non-sampled transactions will be still be

env.go

@@ -95,7 +95,7 @@ var (
 			if i > 0 {
 				k, v := strings.TrimSpace(kv[:i]), strings.TrimSpace(kv[i+1:])
 				labels = append(labels, model.StringMapItem{
-					Key:   cleanTagKey(k),
+					Key:   cleanLabelKey(k),
 					Value: truncateString(v),
 				})
 			}

example_test.go

@@ -144,7 +144,7 @@ func (api *api) handleOrder(ctx context.Context, product string) {
 	defer tx.End()
 	ctx = apm.ContextWithTransaction(ctx, tx)
 
-	tx.Context.SetTag("product", product)
+	tx.Context.SetLabel("product", product)
 
 	time.Sleep(10 * time.Millisecond)
 	storeOrder(ctx, product)

gofuzz.go

@@ -62,7 +62,7 @@ func Fuzz(data []byte) int {
 			return nil
 		}
 		for k, v := range in.Tags {
-			out.SetTag(k, v)
+			out.SetLabel(k, v)
 		}
 		if in.Request != nil {
 			var body io.Reader

metrics_test.go

@@ -219,7 +219,7 @@ func TestTracerMetricsBusyTracer(t *testing.T) {
 			strings.Repeat("x", 1024),
 			strings.Repeat("y", 1024),
 		)
-		tx.Context.SetTag(strings.Repeat("a", 7000), "v")
+		tx.Context.SetLabel(strings.Repeat("a", 7000), "v")
 		tx.End()
 	}
 

model/marshal_test.go

@@ -509,7 +509,7 @@ func fakeTransaction() model.Transaction {
 			User: &model.User{
 				Username: "wanda",
 			},
-			Tags: model.StringMap{{
+			Tags: model.IfaceMap{{
 				Key: "tag", Value: "urit",
 			}},
 			Service: &model.Service{

model/model.go

@@ -262,7 +262,7 @@ type SpanContext struct {
 	HTTP *HTTPSpanContext `json:"http,omitempty"`
 
 	// Tags holds user-defined key/value pairs.
-	Tags StringMap `json:"tags,omitempty"`
+	Tags IfaceMap `json:"tags,omitempty"`
 }
 
 // DatabaseSpanContext holds contextual information for database
@@ -310,7 +310,7 @@ type Context struct {
 	User *User `json:"user,omitempty"`
 
 	// Tags holds user-defined key/value pairs.
-	Tags StringMap `json:"tags,omitempty"`
+	Tags IfaceMap `json:"tags,omitempty"`
 
 	// Service holds values to overrides service-level metadata.
 	Service *Service `json:"service,omitempty"`

module/apmot/span.go

@@ -179,7 +179,7 @@ func (s *otSpan) setSpanContext() {
 			s.span.Type = stringify(v)
 
 		default:
-			s.span.Context.SetTag(k, stringify(v))
+			s.span.Context.SetLabel(k, stringify(v))
 		}
 	}
 	switch {
@@ -246,7 +246,7 @@ func (s *otSpan) setTransactionContext() {
 			s.ctx.tx.Context.SetUsername(stringify(v))
 
 		default:
-			s.ctx.tx.Context.SetTag(k, stringify(v))
+			s.ctx.tx.Context.SetLabel(k, stringify(v))
 		}
 	}
 	if s.ctx.tx.Type == "" {

module/apmot/tracer_test.go

@@ -240,8 +240,8 @@ func TestCustomTags(t *testing.T) {
 	payloads := recorder.Payloads()
 	require.Len(t, payloads.Transactions, 1)
 	require.Len(t, payloads.Spans, 1)
-	assert.Equal(t, model.StringMap{{Key: "foo", Value: "bar"}}, payloads.Transactions[0].Context.Tags)
-	assert.Equal(t, model.StringMap{{Key: "baz", Value: "qux"}}, payloads.Spans[0].Context.Tags)
+	assert.Equal(t, model.IfaceMap{{Key: "foo", Value: "bar"}}, payloads.Transactions[0].Context.Tags)
+	assert.Equal(t, model.IfaceMap{{Key: "baz", Value: "qux"}}, payloads.Spans[0].Context.Tags)
 }
 
 func TestStartSpanFromContextMixed(t *testing.T) {

spancontext.go

@@ -65,16 +65,29 @@ func (c *SpanContext) reset() {
 	}
 }
 
-// SetTag sets a tag in the context. Invalid characters
-// ('.', '*', and '"') in the key will be replaced with
-// an underscore.
+// SetTag calls SetLabel(key, value).
+//
+// SetTag is deprecated, and will be removed in a future major version.
 func (c *SpanContext) SetTag(key, value string) {
+	c.SetLabel(key, value)
+}
+
+// SetLabel sets a label in the context.
+//
+// Invalid characters ('.', '*', and '"') in the key will be replaced with
+// underscores.
+//
+// If the value is numerical or boolean, then it will be sent to the server
+// as a JSON number or boolean; otherwise it will converted to a string, using
+// `fmt.Sprint` if necessary. String values longer than 1024 characters will
+// be truncated.
+func (c *SpanContext) SetLabel(key string, value interface{}) {
 	// Note that we do not attempt to de-duplicate the keys.
 	// This is OK, since json.Unmarshal will always take the
 	// final instance.
-	c.model.Tags = append(c.model.Tags, model.StringMapItem{
-		Key:   cleanTagKey(key),
-		Value: truncateString(value),
+	c.model.Tags = append(c.model.Tags, model.IfaceMapItem{
+		Key:   cleanLabelKey(key),
+		Value: makeLabelValue(value),
 	})
 }
 

spancontext_test.go

@@ -29,17 +29,21 @@ import (
 	"go.elastic.co/apm/model"
 )
 
-func TestSpanContextSetTag(t *testing.T) {
+func TestSpanContextSetLabel(t *testing.T) {
 	_, spans, _ := apmtest.WithTransaction(func(ctx context.Context) {
 		span, _ := apm.StartSpan(ctx, "name", "type")
-		span.Context.SetTag("foo", "bar")
-		span.Context.SetTag("foo", "bar!") // Last instance wins
-		span.Context.SetTag("bar", "baz")
+		span.Context.SetTag("foo", "bar")    // deprecated
+		span.Context.SetLabel("foo", "bar!") // Last instance wins
+		span.Context.SetLabel("bar", "baz")
+		span.Context.SetLabel("baz", 123.456)
+		span.Context.SetLabel("qux", true)
 		span.End()
 	})
 	require.Len(t, spans, 1)
-	assert.Equal(t, model.StringMap{
+	assert.Equal(t, model.IfaceMap{
 		{Key: "bar", Value: "baz"},
+		{Key: "baz", Value: 123.456},
 		{Key: "foo", Value: "bar!"},
+		{Key: "qux", Value: true},
 	}, spans[0].Context.Tags)
 }

transaction_test.go

@@ -147,7 +147,7 @@ func TestTransactionContextNotSampled(t *testing.T) {
 	tracer.SetSampler(samplerFunc(func(apm.TraceContext) bool { return false }))
 
 	tx := tracer.StartTransaction("name", "type")
-	tx.Context.SetTag("foo", "bar")
+	tx.Context.SetLabel("foo", "bar")
 	tx.End()
 	tracer.Flush(nil)
 

utils.go

@@ -18,9 +18,11 @@
 package apm
 
 import (
+	"fmt"
 	"math/rand"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"strings"
@@ -41,7 +43,10 @@ var (
 	localSystem    model.System
 
 	serviceNameInvalidRegexp = regexp.MustCompile("[^" + serviceNameValidClass + "]")
-	tagKeyReplacer           = strings.NewReplacer(`.`, `_`, `*`, `_`, `"`, `_`)
+	labelKeyReplacer         = strings.NewReplacer(`.`, `_`, `*`, `_`, `"`, `_`)
+
+	rtypeBool    = reflect.TypeOf(false)
+	rtypeFloat64 = reflect.TypeOf(float64(0))
 )
 
 const (
@@ -147,8 +152,33 @@ func getKubernetesMetadata() *model.Kubernetes {
 	return kubernetes
 }
 
-func cleanTagKey(k string) string {
-	return tagKeyReplacer.Replace(k)
+func cleanLabelKey(k string) string {
+	return labelKeyReplacer.Replace(k)
+}
+
+// makeLabelValue returns v as a value suitable for including
+// in a label value. If v is numerical or boolean, then it will
+// be returned as-is; otherwise the value will be returned as a
+// string, using fmt.Sprint if necessary, and possibly truncated
+// using truncateString.
+func makeLabelValue(v interface{}) interface{} {
+	switch v.(type) {
+	case nil, bool, float32, float64,
+		uint, uint8, uint16, uint32, uint64,
+		int, int8, int16, int32, int64:
+		return v
+	case string:
+		return truncateString(v.(string))
+	}
+	// Slow path. If v has a non-basic type whose underlying
+	// type is convertible to bool or float64, return v as-is.
+	// Otherwise, stringify.
+	rtype := reflect.TypeOf(v)
+	if rtype.ConvertibleTo(rtypeBool) || rtype.ConvertibleTo(rtypeFloat64) {
+		// Custom type
+		return v
+	}
+	return truncateString(fmt.Sprint(v))
 }
 
 func validateServiceName(name string) error {

validation_test.go

@@ -118,22 +118,27 @@ func TestValidateContextUserBasicAuth(t *testing.T) {
 	})
 }
 
-func TestValidateContextTags(t *testing.T) {
+func TestValidateContextLabels(t *testing.T) {
 	t.Run("long_key", func(t *testing.T) {
 		// NOTE(axw) this should probably fail, but does not. See:
 		// https://github.com/elastic/apm-server/issues/910
 		validateTransaction(t, func(tx *apm.Transaction) {
-			tx.Context.SetTag(strings.Repeat("x", 1025), "x")
+			tx.Context.SetLabel(strings.Repeat("x", 1025), "x")
 		})
 	})
 	t.Run("long_value", func(t *testing.T) {
 		validateTransaction(t, func(tx *apm.Transaction) {
-			tx.Context.SetTag("x", strings.Repeat("x", 1025))
+			tx.Context.SetLabel("x", strings.Repeat("x", 1025))
 		})
 	})
 	t.Run("reserved_key_chars", func(t *testing.T) {
 		validateTransaction(t, func(tx *apm.Transaction) {
-			tx.Context.SetTag("x.y", "z")
+			tx.Context.SetLabel("x.y", "z")
+		})
+	})
+	t.Run("null_value", func(t *testing.T) {
+		validateTransaction(t, func(tx *apm.Transaction) {
+			tx.Context.SetLabel("null", nil)
 		})
 	})
 }