Skip to content

Commit

Permalink
baggage: Improve performance (#4743)
Browse files Browse the repository at this point in the history
  • Loading branch information
cdvr1993 authored Dec 14, 2023
1 parent 0c1c434 commit 057f897
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Improve `go.opentelemetry.io/otel/trace.TraceState`'s performance. (#4722)
- Improve `go.opentelemetry.io/otel/propagation.TraceContext`'s performance. (#4721)
- Improve `go.opentelemetry.io/otel/baggage` performance. (#4743)

## [1.21.0/0.44.0] 2023-11-16

Expand Down
172 changes: 141 additions & 31 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"errors"
"fmt"
"net/url"
"regexp"
"strings"

"go.opentelemetry.io/otel/internal/baggage"
Expand All @@ -32,16 +31,6 @@ const (
listDelimiter = ","
keyValueDelimiter = "="
propertyDelimiter = ";"

keyDef = `([\x21\x23-\x27\x2A\x2B\x2D\x2E\x30-\x39\x41-\x5a\x5e-\x7a\x7c\x7e]+)`
valueDef = `([\x21\x23-\x2b\x2d-\x3a\x3c-\x5B\x5D-\x7e]*)`
keyValueDef = `\s*` + keyDef + `\s*` + keyValueDelimiter + `\s*` + valueDef + `\s*`
)

var (
keyRe = regexp.MustCompile(`^` + keyDef + `$`)
valueRe = regexp.MustCompile(`^` + valueDef + `$`)
propertyRe = regexp.MustCompile(`^(?:\s*` + keyDef + `\s*|` + keyValueDef + `)$`)
)

var (
Expand All @@ -67,7 +56,7 @@ type Property struct {
//
// If key is invalid, an error will be returned.
func NewKeyProperty(key string) (Property, error) {
if !keyRe.MatchString(key) {
if !validateKey(key) {
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)
}

Expand All @@ -79,10 +68,10 @@ func NewKeyProperty(key string) (Property, error) {
//
// If key or value are invalid, an error will be returned.
func NewKeyValueProperty(key, value string) (Property, error) {
if !keyRe.MatchString(key) {
if !validateKey(key) {
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)
}
if !valueRe.MatchString(value) {
if !validateValue(value) {
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value)
}

Expand All @@ -106,20 +95,11 @@ func parseProperty(property string) (Property, error) {
return newInvalidProperty(), nil
}

match := propertyRe.FindStringSubmatch(property)
if len(match) != 4 {
p, ok := parsePropertyInternal(property)
if !ok {
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidProperty, property)
}

var p Property
if match[1] != "" {
p.key = match[1]
} else {
p.key = match[2]
p.value = match[3]
p.hasValue = true
}

return p, nil
}

Expand All @@ -130,10 +110,10 @@ func (p Property) validate() error {
return fmt.Errorf("invalid property: %w", err)
}

if !keyRe.MatchString(p.key) {
if !validateKey(p.key) {
return errFunc(fmt.Errorf("%w: %q", errInvalidKey, p.key))
}
if p.hasValue && !valueRe.MatchString(p.value) {
if p.hasValue && !validateValue(p.value) {
return errFunc(fmt.Errorf("%w: %q", errInvalidValue, p.value))
}
if !p.hasValue && p.value != "" {
Expand Down Expand Up @@ -305,10 +285,10 @@ func parseMember(member string) (Member, error) {
if err != nil {
return newInvalidMember(), fmt.Errorf("%w: %q", err, value)
}
if !keyRe.MatchString(key) {
if !validateKey(key) {
return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key)
}
if !valueRe.MatchString(value) {
if !validateValue(value) {
return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
}

Expand All @@ -323,10 +303,10 @@ func (m Member) validate() error {
return fmt.Errorf("%w: %q", errInvalidMember, m)
}

if !keyRe.MatchString(m.key) {
if !validateKey(m.key) {
return fmt.Errorf("%w: %q", errInvalidKey, m.key)
}
if !valueRe.MatchString(m.value) {
if !validateValue(m.value) {
return fmt.Errorf("%w: %q", errInvalidValue, m.value)
}
return m.properties.validate()
Expand Down Expand Up @@ -550,3 +530,133 @@ func (b Baggage) String() string {
}
return strings.Join(members, listDelimiter)
}

// parsePropertyInternal attempts to decode a Property from the passed string.
// It follows the spec at https://www.w3.org/TR/baggage/#definition.
func parsePropertyInternal(s string) (p Property, ok bool) {
// For the entire function we will use " key = value " as an example.
// Attempting to parse the key.
// First skip spaces at the beginning "< >key = value " (they could be empty).
index := skipSpace(s, 0)

// Parse the key: " <key> = value ".
keyStart := index
keyEnd := index
for _, c := range s[keyStart:] {
if !validateKeyChar(c) {
break
}
keyEnd++
}

// If we couldn't find any valid key character,
// it means the key is either empty or invalid.
if keyStart == keyEnd {
return
}

// Skip spaces after the key: " key< >= value ".
index = skipSpace(s, keyEnd)

if index == len(s) {
// A key can have no value, like: " key ".
ok = true
p.key = s[keyStart:keyEnd]
return
}

// If we have not reached the end and we can't find the '=' delimiter,
// it means the property is invalid.
if s[index] != keyValueDelimiter[0] {
return
}

// Attempting to parse the value.
// Match: " key =< >value ".
index = skipSpace(s, index+1)

// Match the value string: " key = <value> ".
// A valid property can be: " key =".
// Therefore, we don't have to check if the value is empty.
valueStart := index
valueEnd := index
for _, c := range s[valueStart:] {
if !validateValueChar(c) {
break
}
valueEnd++
}

// Skip all trailing whitespaces: " key = value< >".
index = skipSpace(s, valueEnd)

// If after looking for the value and skipping whitespaces
// we have not reached the end, it means the property is
// invalid, something like: " key = value value1".
if index != len(s) {
return
}

ok = true
p.key = s[keyStart:keyEnd]
p.hasValue = true
p.value = s[valueStart:valueEnd]
return
}

func skipSpace(s string, offset int) int {
i := offset
for ; i < len(s); i++ {
c := s[i]
if c != ' ' && c != '\t' {
break
}
}
return i
}

func validateKey(s string) bool {
if len(s) == 0 {
return false
}

for _, c := range s {
if !validateKeyChar(c) {
return false
}
}

return true
}

func validateKeyChar(c int32) bool {
return (c >= 0x23 && c <= 0x27) ||
(c >= 0x30 && c <= 0x39) ||
(c >= 0x41 && c <= 0x5a) ||
(c >= 0x5e && c <= 0x7a) ||
c == 0x21 ||
c == 0x2a ||
c == 0x2b ||
c == 0x2d ||
c == 0x2e ||
c == 0x7c ||
c == 0x7e
}

func validateValue(s string) bool {
for _, c := range s {
if !validateValueChar(c) {
return false
}
}

return true
}

func validateValueChar(c int32) bool {
return (c >= 0x23 && c <= 0x2b) ||
(c >= 0x2d && c <= 0x3a) ||
(c >= 0x3c && c <= 0x5b) ||
(c >= 0x5d && c <= 0x7e) ||
c == 0x21
}
15 changes: 10 additions & 5 deletions baggage/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func init() {
rng = rand.New(rand.NewSource(1))
}

func TestKeyRegExp(t *testing.T) {
func TestValidateKeyChar(t *testing.T) {
// ASCII only
invalidKeyRune := []rune{
'\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07',
Expand All @@ -45,11 +45,11 @@ func TestKeyRegExp(t *testing.T) {
}

for _, ch := range invalidKeyRune {
assert.NotRegexp(t, keyDef, fmt.Sprintf("%c", ch))
assert.False(t, validateKeyChar(ch))
}
}

func TestValueRegExp(t *testing.T) {
func TestValidateValueChar(t *testing.T) {
// ASCII only
invalidValueRune := []rune{
'\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07',
Expand All @@ -60,7 +60,7 @@ func TestValueRegExp(t *testing.T) {
}

for _, ch := range invalidValueRune {
assert.NotRegexp(t, `^`+valueDef+`$`, fmt.Sprintf("invalid-%c-value", ch))
assert.False(t, validateValueChar(ch))
}
}

Expand Down Expand Up @@ -415,10 +415,15 @@ func TestBaggageParse(t *testing.T) {
err: errInvalidValue,
},
{
name: "invalid property: invalid key",
name: "invalid property: no key",
in: "foo=1;=v",
err: errInvalidProperty,
},
{
name: "invalid property: invalid key",
in: "foo=1;key\\=v",
err: errInvalidProperty,
},
{
name: "invalid property: invalid value",
in: "foo=1;key=\\",
Expand Down

0 comments on commit 057f897

Please sign in to comment.