baggage: Add NewMemberRaw and NewKeyValuePropertyRaw

CHANGELOG.md

@@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add `WithResourceAsConstantLabels` option to apply resource attributes for every metric emitted by the Prometheus exporter. (#4733)
 - Experimental cardinality limiting is added to the metric SDK.
   See [metric documentation](./sdk/metric/EXPERIMENTAL.md#cardinality-limit) for more information about this feature and how to enable it. (#4457)
+- Add `NewMemberRaw` and `NewKeyValuePropertyRaw` in `go.opentelemetry.io/otel/baggage`. (#4804)
 
 ### Changed
 
@@ -30,9 +31,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Improve `go.opentelemetry.io/otel/propagation.TraceContext`'s performance. (#4721)
 - Improve `go.opentelemetry.io/otel/baggage` performance. (#4743)
 - `Member.String` in `go.opentelemetry.io/otel/baggage` percent-encodes only when necessary. (#4775)
-- `NewMember` and `NewKeyValueProperty` in `go.opentelemetry.io/otel/baggage` does no longer require percent-encoding of values. Make sure to not precent-encode the value yourself (e.g. by using `url.PathEscape`) before passing it to `NewMember` and `NewKeyValueProperty`. `Member.String` and `Property.String` are encoding the value when necessary. (#4804)
 - `Property.Value` in `go.opentelemetry.io/otel/baggage` now returns a vanilla string instead of a percent-encoded value. (#4804)
 
+### Deprecated
+
+- `NewMember` and `NewKeyValueProperty` in `go.opentelemetry.io/otel/baggage` are deprecated. Use `NewMemberRaw` and `NewKeyValuePropertyRaw` instead which do not require percent-encoding of the value. (#4804)
+
 ### Fixed
 
 - Fix `Parse` in `go.opentelemetry.io/otel/baggage` to validate member value before percent-decoding. (#4755)

baggage/baggage.go

@@ -67,7 +67,24 @@ func NewKeyProperty(key string) (Property, error) {
 // NewKeyValueProperty returns a new Property for key with value.
 //
 // If key or value are invalid, an error will be returned.
+//
+// Deprecated: Use NewKeyValuePropertyRaw instead
+// that does not require precent-encoding of the value.
 func NewKeyValueProperty(key, value string) (Property, error) {
+	if !validateValue(value) {
+		return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value)
+	}
+	decodedValue, err := url.PathUnescape(value)
+	if err != nil {
+		return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value)
+	}
+	return NewKeyValuePropertyRaw(key, decodedValue)
+}
+
+// NewKeyValuePropertyRaw returns a new Property for key with value.
+//
+// If key or value are invalid, an error will be returned.
+func NewKeyValuePropertyRaw(key, value string) (Property, error) {
 	if !validateKey(key) {
 		return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)
 	}
@@ -128,7 +145,7 @@ func (p Property) Value() (string, bool) {
 	return p.value, p.hasValue
 }
 
-// String encodes Property into a string compliant with the W3C Baggage
+// String encodes Property into a header string compliant with the W3C Baggage
 // specification.
 func (p Property) String() string {
 	if p.hasValue {
@@ -192,7 +209,7 @@ func (p properties) validate() error {
 	return nil
 }
 
-// String encodes properties into a string compliant with the W3C Baggage
+// String encodes properties into a header string compliant with the W3C Baggage
 // specification.
 func (p properties) String() string {
 	props := make([]string, len(p))
@@ -214,10 +231,27 @@ type Member struct {
 	hasData bool
 }
 
-// NewMember returns a new Member from the passed arguments. An error
+// NewMemberRaw returns a new Member from the passed arguments. An error
 // is returned if the created Member would be invalid according to the W3C
 // Baggage specification.
+//
+// Deprecated: Use NewMemberRaw instead
+// that does not require precent-encoding of the value.
 func NewMember(key, value string, props ...Property) (Member, error) {
+	if !validateValue(value) {
+		return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
+	}
+	decodedValue, err := url.PathUnescape(value)
+	if err != nil {
+		return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
+	}
+	return NewMemberRaw(key, decodedValue, props...)
+}
+
+// NewMemberRaw returns a new Member from the passed arguments. An error
+// is returned if the created Member would be invalid according to the W3C
+// Baggage specification.
+func NewMemberRaw(key, value string, props ...Property) (Member, error) {
 	m := Member{
 		key:        key,
 		value:      value,
@@ -303,7 +337,7 @@ func (m Member) Value() string { return m.value }
 // Properties returns a copy of the Member properties.
 func (m Member) Properties() []Property { return m.properties.Copy() }
 
-// String encodes Member into a string compliant with the W3C Baggage
+// String encodes Member into a header string compliant with the W3C Baggage
 // specification.
 func (m Member) String() string {
 	// A key is just an ASCII string. A value is restricted to be
@@ -500,9 +534,8 @@ func (b Baggage) Len() int {
 	return len(b.list)
 }
 
-// String encodes Baggage into a string compliant with the W3C Baggage
-// specification. The returned string will be invalid if the Baggage contains
-// any invalid list-members.
+// String encodes Baggage into a header string compliant with the W3C Baggage
+// specification.
 func (b Baggage) String() string {
 	members := make([]string, 0, len(b.list))
 	for k, v := range b.list {

baggage/baggage_test.go

@@ -143,11 +143,25 @@ func TestNewKeyProperty(t *testing.T) {
 }
 
 func TestNewKeyValueProperty(t *testing.T) {
-	p, err := NewKeyValueProperty(" ", "")
+	p, err := NewKeyValueProperty(" ", "value")
 	assert.ErrorIs(t, err, errInvalidKey)
 	assert.Equal(t, Property{}, p)
 
-	p, err = NewKeyValueProperty("key", "Witaj Świecie!")
+	p, err = NewKeyValueProperty("key", ";")
+	assert.ErrorIs(t, err, errInvalidValue)
+	assert.Equal(t, Property{}, p)
+
+	p, err = NewKeyValueProperty("key", "value")
+	assert.NoError(t, err)
+	assert.Equal(t, Property{key: "key", value: "value", hasValue: true}, p)
+}
+
+func TestNewKeyValuePropertyRaw(t *testing.T) {
+	p, err := NewKeyValuePropertyRaw(" ", "")
+	assert.ErrorIs(t, err, errInvalidKey)
+	assert.Equal(t, Property{}, p)
+
+	p, err = NewKeyValuePropertyRaw("key", "Witaj Świecie!")
 	assert.NoError(t, err)
 	assert.Equal(t, Property{key: "key", value: "Witaj Świecie!", hasValue: true}, p)
 }
@@ -879,6 +893,45 @@ func TestNewMember(t *testing.T) {
 	}
 	assert.Equal(t, expected, m)
 
+	// wrong value with wrong decoding
+	val = "%zzzzz"
+	_, err = NewMember(key, val, p)
+	assert.ErrorIs(t, err, errInvalidValue)
+
+	// value should be decoded
+	val = "%3B"
+	m, err = NewMember(key, val, p)
+	expected = Member{
+		key:        key,
+		value:      ";",
+		properties: properties{{key: "foo"}},
+		hasData:    true,
+	}
+	assert.NoError(t, err)
+	assert.Equal(t, expected, m)
+
+	// Ensure new member is immutable.
+	p.key = "bar"
+	assert.Equal(t, expected, m)
+}
+
+func TestNewMemberRaw(t *testing.T) {
+	m, err := NewMemberRaw("", "")
+	assert.ErrorIs(t, err, errInvalidKey)
+	assert.Equal(t, Member{hasData: false}, m)
+
+	key, val := "k", "v"
+	p := Property{key: "foo"}
+	m, err = NewMemberRaw(key, val, p)
+	assert.NoError(t, err)
+	expected := Member{
+		key:        key,
+		value:      val,
+		properties: properties{{key: "foo"}},
+		hasData:    true,
+	}
+	assert.Equal(t, expected, m)
+
 	// Ensure new member is immutable.
 	p.key = "bar"
 	assert.Equal(t, expected, m)
@@ -897,23 +950,23 @@ func TestPropertiesValidate(t *testing.T) {
 
 func TestMemberString(t *testing.T) {
 	// normal key value pair
-	member, _ := NewMember("key", "value")
+	member, _ := NewMemberRaw("key", "value")
 	memberStr := member.String()
 	assert.Equal(t, memberStr, "key=value")
 
 	// encoded value
-	member, _ = NewMember("key", "; ")
+	member, _ = NewMemberRaw("key", "; ")
 	memberStr = member.String()
 	assert.Equal(t, memberStr, "key=%3B%20")
 }
 
 var benchBaggage Baggage
 
 func BenchmarkNew(b *testing.B) {
-	mem1, _ := NewMember("key1", "val1")
-	mem2, _ := NewMember("key2", "val2")
-	mem3, _ := NewMember("key3", "val3")
-	mem4, _ := NewMember("key4", "val4")
+	mem1, _ := NewMemberRaw("key1", "val1")
+	mem2, _ := NewMemberRaw("key2", "val2")
+	mem3, _ := NewMemberRaw("key3", "val3")
+	mem4, _ := NewMemberRaw("key4", "val4")
 
 	b.ReportAllocs()
 	b.ResetTimer()
@@ -929,7 +982,7 @@ func BenchmarkNewMember(b *testing.B) {
 	b.ReportAllocs()
 
 	for i := 0; i < b.N; i++ {
-		benchMember, _ = NewMember("key", "value")
+		benchMember, _ = NewMemberRaw("key", "value")
 	}
 }
 
@@ -944,7 +997,7 @@ func BenchmarkParse(b *testing.B) {
 func BenchmarkString(b *testing.B) {
 	var members []Member
 	addMember := func(k, v string) {
-		m, err := NewMember(k, valueEscape(v))
+		m, err := NewMemberRaw(k, valueEscape(v))
 		require.NoError(b, err)
 		members = append(members, m)
 	}

bridge/opentracing/bridge.go

@@ -73,7 +73,7 @@ func (c *bridgeSpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
 }
 
 func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) {
-	m, err := baggage.NewMember(restrictedKey, value)
+	m, err := baggage.NewMemberRaw(restrictedKey, value)
 	if err != nil {
 		return
 	}

bridge/opentracing/mix_test.go

@@ -515,7 +515,7 @@ func (bio *baggageInteroperationTest) addAndRecordBaggage(t *testing.T, ctx cont
 
 	otSpan.SetBaggageItem(otKey, value)
 
-	m, err := baggage.NewMember(otelKey, value)
+	m, err := baggage.NewMemberRaw(otelKey, value)
 	if err != nil {
 		t.Error(err)
 		return ctx

example/namedtracer/main.go

@@ -67,8 +67,8 @@ func main() {
 	ctx := context.Background()
 	defer func() { _ = tp.Shutdown(ctx) }()
 
-	m0, _ := baggage.NewMember(string(fooKey), "foo1")
-	m1, _ := baggage.NewMember(string(barKey), "bar1")
+	m0, _ := baggage.NewMemberRaw(string(fooKey), "foo1")
+	m1, _ := baggage.NewMemberRaw(string(barKey), "bar1")
 	b, _ := baggage.New(m0, m1)
 	ctx = baggage.ContextWithBaggage(ctx, b)
 

propagation/baggage_test.go

@@ -40,13 +40,13 @@ type member struct {
 func (m member) Member(t *testing.T) baggage.Member {
 	props := make([]baggage.Property, 0, len(m.Properties))
 	for _, p := range m.Properties {
-		p, err := baggage.NewKeyValueProperty(p.Key, p.Value)
+		p, err := baggage.NewKeyValuePropertyRaw(p.Key, p.Value)
 		if err != nil {
 			t.Fatal(err)
 		}
 		props = append(props, p)
 	}
-	bMember, err := baggage.NewMember(m.Key, m.Value, props...)
+	bMember, err := baggage.NewMemberRaw(m.Key, m.Value, props...)
 	if err != nil {
 		t.Fatal(err)
 	}