[client] Fix race condition in Metadata.Get

.chloggen/make-metadata-thread-safe.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: client
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Make `Metadata.Get` thread safe
+
+# One or more tracking issues or pull requests related to the change
+issues: [9595]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

client/client.go

@@ -141,28 +141,24 @@ func FromContext(ctx context.Context) Info {
 
 // NewMetadata creates a new Metadata object to use in Info. md is used as-is.
 func NewMetadata(md map[string][]string) Metadata {
+	c := make(map[string][]string, len(md))
+	for k, v := range md {
+		c[strings.ToLower(k)] = v
+	}
 	return Metadata{
-		data: md,
+		data: c,
 	}
 }
 
 // Get gets the value of the key from metadata, returning a copy.
 func (m Metadata) Get(key string) []string {
-	vals := m.data[key]
+	if len(m.data) == 0 {
+		return nil
+	}
+
+	vals := m.data[strings.ToLower(key)]
 	if len(vals) == 0 {
-		// we didn't find the key, but perhaps it just has different cases?
-		for k, v := range m.data {
-			if strings.EqualFold(key, k) {
-				vals = v
-				// we optimize for the next lookup
-				m.data[key] = v
-			}
-		}
-
-		// if it's still not found, it's really not here
-		if len(vals) == 0 {
-			return nil
-		}
+		return nil
 	}
 
 	ret := make([]string, len(vals))

client/client_test.go

@@ -77,10 +77,11 @@ func TestFromContext(t *testing.T) {
 }
 
 func TestMetadata(t *testing.T) {
-	source := map[string][]string{"test-key": {"test-val"}}
+	source := map[string][]string{"test-key": {"test-val"}, "TEST-KEY-2": {"test-val"}}
 	md := NewMetadata(source)
 	assert.Equal(t, []string{"test-val"}, md.Get("test-key"))
-	assert.Equal(t, []string{"test-val"}, md.Get("test-KEY")) // case insensitive lookup
+	assert.Equal(t, []string{"test-val"}, md.Get("test-KEY"))   // case insensitive lookup
+	assert.Equal(t, []string{"test-val"}, md.Get("test-key-2")) // case insensitive lookup
 
 	// test if copy. In regular use, source cannot change
 	val := md.Get("test-key")
@@ -89,3 +90,8 @@ func TestMetadata(t *testing.T) {
 
 	assert.Empty(t, md.Get("non-existent-key"))
 }
+
+func TestUninstantiatedMetadata(t *testing.T) {
+	i := Info{}
+	assert.Empty(t, i.Metadata.Get("test"))
+}

config/configgrpc/configgrpc_test.go

@@ -9,6 +9,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"testing"
 	"time"
@@ -796,7 +797,9 @@ func TestContextWithClient(t *testing.T) {
 	for _, tC := range testCases {
 		t.Run(tC.desc, func(t *testing.T) {
 			cl := client.FromContext(contextWithClient(tC.input, tC.doMetadata))
-			assert.Equal(t, tC.expected, cl)
+			assert.Equal(t, tC.expected.Addr, cl.Addr)
+			assert.Equal(t, tC.expected.Auth, cl.Auth)
+			assert.True(t, reflect.DeepEqual(tC.expected.Metadata, cl.Metadata))
 		})
 	}
 }