From 148c9ce5baf8195cd6d07cb018ced34ae25895a5 Mon Sep 17 00:00:00 2001
From: Joshua MacDonald <jmacd@users.noreply.github.com>
Date: Wed, 4 Mar 2020 14:19:25 -0800
Subject: [PATCH] Make the default label encoding unique (#508)

* Make the default label encoding unique

* More tests

* Cleanup
---
 sdk/metric/correct_test.go | 23 +++++++++++++++++++++++
 sdk/metric/labelencoder.go | 27 +++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/sdk/metric/correct_test.go b/sdk/metric/correct_test.go
index 5fdf86e7fe3..0b7b7ee533c 100644
--- a/sdk/metric/correct_test.go
+++ b/sdk/metric/correct_test.go
@@ -189,6 +189,29 @@ func TestSDKLabelEncoder(t *testing.T) {
 
 func TestDefaultLabelEncoder(t *testing.T) {
 	encoder := sdk.NewDefaultLabelEncoder()
+
 	encoded := encoder.Encode([]core.KeyValue{key.String("A", "B"), key.String("C", "D")})
 	require.Equal(t, `A=B,C=D`, encoded)
+
+	encoded = encoder.Encode([]core.KeyValue{key.String("A", "B,c=d"), key.String(`C\`, "D")})
+	require.Equal(t, `A=B\,c\=d,C\\=D`, encoded)
+
+	encoded = encoder.Encode([]core.KeyValue{key.String(`\`, `=`), key.String(`,`, `\`)})
+	require.Equal(t, `\\=\=,\,=\\`, encoded)
+
+	// Note: the label encoder does not sort or de-dup values,
+	// that is done in Labels(...).
+	encoded = encoder.Encode([]core.KeyValue{
+		key.Int("I", 1),
+		key.Uint("U", 1),
+		key.Int32("I32", 1),
+		key.Uint32("U32", 1),
+		key.Int64("I64", 1),
+		key.Uint64("U64", 1),
+		key.Float64("F64", 1),
+		key.Float64("F64", 1),
+		key.String("S", "1"),
+		key.Bool("B", true),
+	})
+	require.Equal(t, "I=1,U=1,I32=1,U32=1,I64=1,U64=1,F64=1,F64=1,S=1,B=true", encoded)
 }
diff --git a/sdk/metric/labelencoder.go b/sdk/metric/labelencoder.go
index a2bb0798d54..1c7477d89cc 100644
--- a/sdk/metric/labelencoder.go
+++ b/sdk/metric/labelencoder.go
@@ -22,6 +22,13 @@ import (
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 )
 
+// escapeChar is used to ensure uniqueness of the label encoding where
+// keys or values contain either '=' or ','.  Since there is no parser
+// needed for this encoding and its only requirement is to be unique,
+// this choice is arbitrary.  Users will see these in some exporters
+// (e.g., stdout), so the backslash ('\') is used a conventional choice.
+const escapeChar = '\\'
+
 type defaultLabelEncoder struct {
 	// pool is a pool of labelset builders.  The buffers in this
 	// pool grow to a size that most label encodings will not
@@ -54,9 +61,25 @@ func (d *defaultLabelEncoder) Encode(labels []core.KeyValue) string {
 		if i > 0 {
 			_, _ = buf.WriteRune(',')
 		}
-		_, _ = buf.WriteString(string(kv.Key))
+		copyAndEscape(buf, string(kv.Key))
+
 		_, _ = buf.WriteRune('=')
-		_, _ = buf.WriteString(kv.Value.Emit())
+
+		if kv.Value.Type() == core.STRING {
+			copyAndEscape(buf, kv.Value.AsString())
+		} else {
+			_, _ = buf.WriteString(kv.Value.Emit())
+		}
 	}
 	return buf.String()
 }
+
+func copyAndEscape(buf *bytes.Buffer, val string) {
+	for _, ch := range val {
+		switch ch {
+		case '=', ',', escapeChar:
+			buf.WriteRune(escapeChar)
+		}
+		buf.WriteRune(ch)
+	}
+}