From f95bee22b92fb323eef80a3a3c74bbbf80fd2937 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy <126021+ash2k@users.noreply.github.com> Date: Thu, 18 May 2023 02:28:44 +1000 Subject: [PATCH] Use strings.Cut() instead of string.SplitN() (#4049) strings.Cut() generates less garbage as it does not allocate the slice to hold parts. --- CHANGELOG.md | 4 ++ baggage/baggage.go | 56 ++++++++----------- .../lib/go/thrift/multiplexed_protocol.go | 24 ++++---- .../otlp/internal/envconfig/envconfig.go | 14 ++--- internal/internaltest/text_map_propagator.go | 6 +- sdk/resource/env.go | 14 ++--- sdk/resource/os_release_unix.go | 8 +-- semconv/internal/http.go | 8 ++- 8 files changed, 65 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8da7403482..9c277bb9a02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/otel/semconv/v1.20.0` package. The package contains semantic conventions from the `v1.20.0` version of the OpenTelemetry specification. (#4078) +### Changed + +- Use `strings.Cut()` instead of `string.SplitN()` for better readability and memory use. (#4049) + ### Removed - The deprecated `go.opentelemetry.io/otel/metric/instrument` package is removed. diff --git a/baggage/baggage.go b/baggage/baggage.go index a36db8f8d85..46e523a80e4 100644 --- a/baggage/baggage.go +++ b/baggage/baggage.go @@ -289,45 +289,37 @@ func parseMember(member string) (Member, error) { props properties ) - parts := strings.SplitN(member, propertyDelimiter, 2) - switch len(parts) { - case 2: + keyValue, properties, found := strings.Cut(member, propertyDelimiter) + if found { // Parse the member properties. - for _, pStr := range strings.Split(parts[1], propertyDelimiter) { + for _, pStr := range strings.Split(properties, propertyDelimiter) { p, err := parseProperty(pStr) if err != nil { return newInvalidMember(), err } props = append(props, p) } - fallthrough - case 1: - // Parse the member key/value pair. - - // Take into account a value can contain equal signs (=). - kv := strings.SplitN(parts[0], keyValueDelimiter, 2) - if len(kv) != 2 { - return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidMember, member) - } - // "Leading and trailing whitespaces are allowed but MUST be trimmed - // when converting the header into a data structure." - key = strings.TrimSpace(kv[0]) - var err error - value, err = url.QueryUnescape(strings.TrimSpace(kv[1])) - if err != nil { - return newInvalidMember(), fmt.Errorf("%w: %q", err, value) - } - if !keyRe.MatchString(key) { - return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) - } - if !valueRe.MatchString(value) { - return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) - } - default: - // This should never happen unless a developer has changed the string - // splitting somehow. Panic instead of failing silently and allowing - // the bug to slip past the CI checks. - panic("failed to parse baggage member") + } + // Parse the member key/value pair. + + // Take into account a value can contain equal signs (=). + k, v, found := strings.Cut(keyValue, keyValueDelimiter) + if !found { + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidMember, member) + } + // "Leading and trailing whitespaces are allowed but MUST be trimmed + // when converting the header into a data structure." + key = strings.TrimSpace(k) + var err error + value, err = url.QueryUnescape(strings.TrimSpace(v)) + if err != nil { + return newInvalidMember(), fmt.Errorf("%w: %q", err, value) + } + if !keyRe.MatchString(key) { + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) + } + if !valueRe.MatchString(value) { + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) } return Member{key: key, value: value, properties: props, hasData: true}, nil diff --git a/exporters/jaeger/internal/third_party/thrift/lib/go/thrift/multiplexed_protocol.go b/exporters/jaeger/internal/third_party/thrift/lib/go/thrift/multiplexed_protocol.go index cacbf6bef3e..d542b23a998 100644 --- a/exporters/jaeger/internal/third_party/thrift/lib/go/thrift/multiplexed_protocol.go +++ b/exporters/jaeger/internal/third_party/thrift/lib/go/thrift/multiplexed_protocol.go @@ -160,15 +160,13 @@ func (t *TMultiplexedProcessor) ProcessorMap() map[string]TProcessorFunction { // the given ProcessorName or if all that is given is the FunctionName and there // is no DefaultProcessor set. func (t *TMultiplexedProcessor) AddToProcessorMap(name string, processorFunc TProcessorFunction) { - components := strings.SplitN(name, MULTIPLEXED_SEPARATOR, 2) - if len(components) != 2 { - if t.DefaultProcessor != nil && len(components) == 1 { - t.DefaultProcessor.AddToProcessorMap(components[0], processorFunc) + processorName, funcName, found := strings.Cut(name, MULTIPLEXED_SEPARATOR) + if !found { + if t.DefaultProcessor != nil { + t.DefaultProcessor.AddToProcessorMap(processorName, processorFunc) } return } - processorName := components[0] - funcName := components[1] if processor, ok := t.serviceProcessorMap[processorName]; ok { processor.AddToProcessorMap(funcName, processorFunc) } @@ -197,9 +195,9 @@ func (t *TMultiplexedProcessor) Process(ctx context.Context, in, out TProtocol) if typeId != CALL && typeId != ONEWAY { return false, NewTProtocolException(fmt.Errorf("Unexpected message type %v", typeId)) } - //extract the service name - v := strings.SplitN(name, MULTIPLEXED_SEPARATOR, 2) - if len(v) != 2 { + // extract the service name + processorName, funcName, found := strings.Cut(name, MULTIPLEXED_SEPARATOR) + if !found { if t.DefaultProcessor != nil { smb := NewStoredMessageProtocol(in, name, typeId, seqid) return t.DefaultProcessor.Process(ctx, smb, out) @@ -209,18 +207,18 @@ func (t *TMultiplexedProcessor) Process(ctx context.Context, in, out TProtocol) name, )) } - actualProcessor, ok := t.serviceProcessorMap[v[0]] + actualProcessor, ok := t.serviceProcessorMap[processorName] if !ok { return false, NewTProtocolException(fmt.Errorf( "Service name not found: %s. Did you forget to call registerProcessor()?", - v[0], + processorName, )) } - smb := NewStoredMessageProtocol(in, v[1], typeId, seqid) + smb := NewStoredMessageProtocol(in, funcName, typeId, seqid) return actualProcessor.Process(ctx, smb, out) } -//Protocol that use stored message for ReadMessageBegin +// Protocol that use stored message for ReadMessageBegin type storedMessageProtocol struct { TProtocol name string diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 53ff3126b6e..444eefbb388 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -166,20 +166,20 @@ func stringToHeader(value string) map[string]string { headers := make(map[string]string) for _, header := range headersPairs { - nameValue := strings.SplitN(header, "=", 2) - if len(nameValue) < 2 { - global.Error(errors.New("missing '="), "parse headers", "input", nameValue) + n, v, found := strings.Cut(header, "=") + if !found { + global.Error(errors.New("missing '="), "parse headers", "input", header) continue } - name, err := url.QueryUnescape(nameValue[0]) + name, err := url.QueryUnescape(n) if err != nil { - global.Error(err, "escape header key", "key", nameValue[0]) + global.Error(err, "escape header key", "key", n) continue } trimmedName := strings.TrimSpace(name) - value, err := url.QueryUnescape(nameValue[1]) + value, err := url.QueryUnescape(v) if err != nil { - global.Error(err, "escape header value", "value", nameValue[1]) + global.Error(err, "escape header value", "value", v) continue } trimmedValue := strings.TrimSpace(value) diff --git a/internal/internaltest/text_map_propagator.go b/internal/internaltest/text_map_propagator.go index 4873e330d02..4a4743cad9e 100644 --- a/internal/internaltest/text_map_propagator.go +++ b/internal/internaltest/text_map_propagator.go @@ -35,9 +35,9 @@ func newState(encoded string) state { if encoded == "" { return state{} } - split := strings.SplitN(encoded, ",", 2) - injects, _ := strconv.ParseUint(split[0], 10, 64) - extracts, _ := strconv.ParseUint(split[1], 10, 64) + s0, s1, _ := strings.Cut(encoded, ",") + injects, _ := strconv.ParseUint(s0, 10, 64) + extracts, _ := strconv.ParseUint(s1, 10, 64) return state{ Injections: injects, Extractions: extracts, diff --git a/sdk/resource/env.go b/sdk/resource/env.go index e32843cad14..f09a7819067 100644 --- a/sdk/resource/env.go +++ b/sdk/resource/env.go @@ -82,23 +82,23 @@ func constructOTResources(s string) (*Resource, error) { return Empty(), nil } pairs := strings.Split(s, ",") - attrs := []attribute.KeyValue{} + var attrs []attribute.KeyValue var invalid []string for _, p := range pairs { - field := strings.SplitN(p, "=", 2) - if len(field) != 2 { + k, v, found := strings.Cut(p, "=") + if !found { invalid = append(invalid, p) continue } - k := strings.TrimSpace(field[0]) - v, err := url.QueryUnescape(strings.TrimSpace(field[1])) + key := strings.TrimSpace(k) + val, err := url.QueryUnescape(strings.TrimSpace(v)) if err != nil { // Retain original value if decoding fails, otherwise it will be // an empty string. - v = field[1] + val = v otel.Handle(err) } - attrs = append(attrs, attribute.String(k, v)) + attrs = append(attrs, attribute.String(key, val)) } var err error if len(invalid) > 0 { diff --git a/sdk/resource/os_release_unix.go b/sdk/resource/os_release_unix.go index fba6790e445..c771942deec 100644 --- a/sdk/resource/os_release_unix.go +++ b/sdk/resource/os_release_unix.go @@ -85,14 +85,14 @@ func skip(line string) bool { // parse attempts to split the provided line on the first '=' character, and then // sanitize each side of the split before returning them as a key-value pair. func parse(line string) (string, string, bool) { - parts := strings.SplitN(line, "=", 2) + k, v, found := strings.Cut(line, "=") - if len(parts) != 2 || len(parts[0]) == 0 { + if !found || len(k) == 0 { return "", "", false } - key := strings.TrimSpace(parts[0]) - value := unescape(unquote(strings.TrimSpace(parts[1]))) + key := strings.TrimSpace(k) + value := unescape(unquote(strings.TrimSpace(v))) return key, value, true } diff --git a/semconv/internal/http.go b/semconv/internal/http.go index b580eedeff7..19c394c69b6 100644 --- a/semconv/internal/http.go +++ b/semconv/internal/http.go @@ -232,10 +232,12 @@ func (sc *SemanticConventions) HTTPServerAttributesFromHTTPRequest(serverName, r if route != "" { attrs = append(attrs, sc.HTTPRouteKey.String(route)) } - if values, ok := request.Header["X-Forwarded-For"]; ok && len(values) > 0 { - if addresses := strings.SplitN(values[0], ",", 2); len(addresses) > 0 { - attrs = append(attrs, sc.HTTPClientIPKey.String(addresses[0])) + if values := request.Header["X-Forwarded-For"]; len(values) > 0 { + addr := values[0] + if i := strings.Index(addr, ","); i > 0 { + addr = addr[:i] } + attrs = append(attrs, sc.HTTPClientIPKey.String(addr)) } return append(attrs, sc.httpCommonAttributesFromHTTPRequest(request)...)