Skip to content

Commit

Permalink
Fix concurrent map panic on inject metadata
Browse files Browse the repository at this point in the history
Signed-off-by: Jin Dong <[email protected]>
  • Loading branch information
djdongjin committed Dec 27, 2024
1 parent 3e9a4c1 commit b9ec520
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 11 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
go.opentelemetry.io/otel/metric v1.19.0
go.opentelemetry.io/otel/sdk v1.19.0
go.opentelemetry.io/otel/trace v1.19.0
golang.org/x/sync v0.10.0
google.golang.org/grpc v1.57.0
google.golang.org/protobuf v1.30.0
)
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,6 @@ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
23 changes: 15 additions & 8 deletions interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"net"
"runtime"
"strings"
"sync"
"testing"

"github.com/containerd/otelttrpc/internal"
Expand All @@ -47,7 +48,6 @@ import (
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/sdk/trace/tracetest"
"go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"
)

const serviceName = "testService"
Expand Down Expand Up @@ -122,21 +122,28 @@ func TestClientCallServerConcurrent(t *testing.T) {
_ = server.Shutdown(ctx)
}()

var eg errgroup.Group
var wg sync.WaitGroup
var errs []error
var mu sync.Mutex

for _, testClient := range testClients {
// capture range variable
// TODO: we can remove this once we upgrade golang to >= 1.22
testClient := testClient
eg.Go(func() error {
wg.Add(1)
go func() {
defer wg.Done()
if _, err := testClient.Test(ctx, payload); err != nil {
return err
mu.Lock()
defer mu.Unlock()
errs = append(errs, err)
}
return nil
})
}()
}

if err := eg.Wait(); err != nil {
t.Fatal(err)
wg.Wait()
if len(errs) > 0 {
t.Fatalf("unexpected errors: %v", errs)
}

// get exported spans
Expand Down
11 changes: 11 additions & 0 deletions metadata_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,21 @@ func (s *metadataSupplier) Keys() []string {
return out
}

func copyMD(src ttrpc.MD) ttrpc.MD {
md := make(ttrpc.MD, len(src))
for k, v := range src {
md[k] = append(md[k], v...)
}
return md
}

func inject(ctx context.Context, propagators propagation.TextMapPropagator, req *ttrpc.Request) context.Context {
md, ok := ttrpc.GetMetadata(ctx)
if !ok {
md = make(ttrpc.MD)
} else {
// make a copy to avoid concurrent read/write panic
md = copyMD(md)
}

propagators.Inject(ctx, &metadataSupplier{
Expand Down

0 comments on commit b9ec520

Please sign in to comment.