diff --git a/tag/doc.go b/tag/doc.go index da16b74e4..32860811c 100644 --- a/tag/doc.go +++ b/tag/doc.go @@ -22,5 +22,12 @@ the OpenCensus instrumentation data. Tags can be propagated on the wire and in the same process via context.Context. Encode and Decode should be used to represent tags into their binary propagation form. + +This package supports a restrictive set of characters in tag keys which +we believe are supported by most metrics backends. Tag values are not limited in +this way, but specific exporters may have their own restrictions on tag +values and if so, should provide a way to sanitize tag values for use +with that backend. + */ package tag // import "go.opencensus.io/tag" diff --git a/tag/map_codec_test.go b/tag/map_codec_test.go index 3327adad0..abb0ead53 100644 --- a/tag/map_codec_test.go +++ b/tag/map_codec_test.go @@ -67,7 +67,7 @@ func TestEncodeDecode(t *testing.T) { []keyValue{ {k1, "v1"}, {k2, "v2"}, - {k3, "v3"}, + {k3, "数据库着火了 🔥🔥🔥"}, {k4, "v4 is very weird <>.,?/'\";:`~!@#$%^&*()_-+={[}]|\\"}, }, }, @@ -107,6 +107,7 @@ func TestEncodeDecode(t *testing.T) { func TestDecode(t *testing.T) { k1, _ := NewKey("k1") ctx, _ := New(context.Background(), Insert(k1, "v1")) + ctx2, _ := New(context.Background(), Insert(k1, "数据库着火了 🔥🔥🔥")) tests := []struct { name string @@ -121,14 +122,14 @@ func TestDecode(t *testing.T) { wantErr: false, }, { - name: "non-ascii key", - bytes: []byte{0, 0, 2, 107, 49, 2, 118, 49, 0, 2, 107, 25, 2, 118, 49}, - want: nil, - wantErr: true, + name: "valid (non-ascii value)", + bytes: []byte{0x0, 0x0, 0x2, 0x6b, 0x31, 0x1f, 0xe6, 0x95, 0xb0, 0xe6, 0x8d, 0xae, 0xe5, 0xba, 0x93, 0xe7, 0x9d, 0x80, 0xe7, 0x81, 0xab, 0xe4, 0xba, 0x86, 0x20, 0xf0, 0x9f, 0x94, 0xa5, 0xf0, 0x9f, 0x94, 0xa5, 0xf0, 0x9f, 0x94, 0xa5}, + want: FromContext(ctx2), + wantErr: false, }, { - name: "non-ascii value", - bytes: []byte{0, 0, 2, 107, 49, 2, 118, 49, 0, 2, 107, 50, 2, 118, 25}, + name: "non-ascii key", + bytes: []byte{0, 0, 2, 107, 49, 2, 118, 49, 0, 2, 107, 25, 2, 118, 49}, want: nil, wantErr: true, }, @@ -147,7 +148,11 @@ func TestDecode(t *testing.T) { return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("Decode() = %v, want %v", got, tt.want) + var encoded []byte + if tt.want != nil { + encoded = Encode(tt.want) + } + t.Errorf("Decode() = %v, want %v = Decode(%#v)", got, tt.want, encoded) } }) } diff --git a/tag/map_test.go b/tag/map_test.go index 6d2704703..aca6e883e 100644 --- a/tag/map_test.go +++ b/tag/map_test.go @@ -129,9 +129,7 @@ func TestNewMap(t *testing.T) { name: "from empty; invalid", initial: nil, mods: []Mutator{ - Insert(k5, "v\x19"), - Upsert(k5, "v\x19"), - Update(k5, "v\x19"), + Insert(k5, strings.Repeat("x", 256)), }, want: nil, }, @@ -140,31 +138,33 @@ func TestNewMap(t *testing.T) { initial: nil, mods: []Mutator{ Insert(k5, "v1"), - Update(k5, "v\x19"), + Update(k5, strings.Repeat("x", 256)), }, want: nil, }, } for _, tt := range tests { - mods := []Mutator{ - Insert(k1, "v1"), - Insert(k2, "v2"), - Update(k3, "v3"), - Upsert(k4, "v4"), - Insert(k2, "v2"), - Delete(k1), - } - mods = append(mods, tt.mods...) - ctx := NewContext(context.Background(), tt.initial) - ctx, err := New(ctx, mods...) - if tt.want != nil && err != nil { - t.Errorf("%v: New = %v", tt.name, err) - } + t.Run(tt.name, func(t *testing.T) { + mods := []Mutator{ + Insert(k1, "v1"), + Insert(k2, "v2"), + Update(k3, "v3"), + Upsert(k4, "v4"), + Insert(k2, "v2"), + Delete(k1), + } + mods = append(mods, tt.mods...) + ctx := NewContext(context.Background(), tt.initial) + ctx, err := New(ctx, mods...) + if tt.want != nil && err != nil { + t.Errorf("New = %v", err) + } - if got, want := FromContext(ctx), tt.want; !reflect.DeepEqual(got, want) { - t.Errorf("%v: got %v; want %v", tt.name, got, want) - } + if got, want := FromContext(ctx), tt.want; !reflect.DeepEqual(got, want) { + t.Errorf("got %v; want %v", got, want) + } + }) } } @@ -183,7 +183,7 @@ func TestNewMapValidation(t *testing.T) { // Value validation {err: "", seed: &Map{m: map[Key]string{{name: "key"}: ""}}}, {err: "", seed: &Map{m: map[Key]string{{name: "key"}: strings.Repeat("a", 255)}}}, - {err: "invalid value", seed: &Map{m: map[Key]string{{name: "key"}: "Приве́т"}}}, + {err: "", seed: &Map{m: map[Key]string{{name: "key"}: "Приве́т"}}}, {err: "invalid value", seed: &Map{m: map[Key]string{{name: "key"}: strings.Repeat("a", 256)}}}, } diff --git a/tag/validate.go b/tag/validate.go index 0939fc674..92c5678b4 100644 --- a/tag/validate.go +++ b/tag/validate.go @@ -26,7 +26,7 @@ const ( var ( errInvalidKeyName = errors.New("invalid key name: only ASCII characters accepted; max length must be 255 characters") - errInvalidValue = errors.New("invalid value: only ASCII characters accepted; max length must be 255 characters") + errInvalidValue = errors.New("invalid value: max length must be 255 UTF-8 characters") ) func checkKeyName(name string) bool { @@ -49,8 +49,5 @@ func isASCII(s string) bool { } func checkValue(v string) bool { - if len(v) > maxKeyLength { - return false - } - return isASCII(v) + return len(v) <= maxKeyLength } diff --git a/tag/validate_test.go b/tag/validate_test.go index 371a775c4..553069b52 100644 --- a/tag/validate_test.go +++ b/tag/validate_test.go @@ -75,16 +75,6 @@ func TestCheckValue(t *testing.T) { value: "v1", wantOK: true, }, - { - name: "invalid i", - value: "k\x19", - wantOK: false, - }, - { - name: "invalid ii", - value: "k\x7f", - wantOK: false, - }, { name: "empty", value: "", @@ -100,6 +90,21 @@ func TestCheckValue(t *testing.T) { value: strings.Repeat("a", 256), wantOK: false, }, + { + name: "emoji", + value: "🔥🔥🔥", + wantOK: true, + }, + { + name: "Simplified Chinese", + value: "上海", + wantOK: true, + }, + { + name: "Bengali", + value: "বাংলা/বঙ্গ", + wantOK: true, + }, } for _, tt := range tests { ok := checkValue(tt.value)