dd-trace-go icon indicating copy to clipboard operation
dd-trace-go copied to clipboard

proposal: contrib/google.golang.org/grpc: make MDCarrier public

Open iamnande opened this issue 2 years ago • 3 comments

The MDCarrier is generally sufficient for non-custom metadata implementations.

My proposal would be to either include the MDCarrier inside of the grpc folder(s), grpc and grpc.v12, or move the grpcutil package under contrib/google.golang.org so that consumers can use it without having to copy+paste into their repositories.

note: i'm happy to make the contribution myself assuming we come to a conclusion on the outcome.

iamnande avatar Feb 16 '22 19:02 iamnande

@iamnande thanks for filing this, and apologies for taking a while to get back to you. Can you clarify what you would like to be able to use the MDCarrier type for? That can help us decide where it should be and how it should be documented.

katiehockman avatar Apr 04 '23 14:04 katiehockman

@iamnande thanks for filing this, and apologies for taking a while to get back to you. Can you clarify what you would like to be able to use the MDCarrier type for? That can help us decide where it should be and how it should be documented.

Wicked. Below is a rough example of how a copy of MDCarrier is used today.

md_extractor.go:

package tracing

import (
	// stuff and things
)

func ExtraceTraceMD(ctx context.Context) metadata.MD {

	// empty metadata
	md := metadata.New(nil)

	// extract tracer
	span, ok := tracer.SpanFromContext(ctx)
	if !ok {
		// handle this
	}

	// inject it for outbound calls
	if err := tracer.Inject(span.Context(), mdCarrier(md)); err != nil {
		// handle this
	}

	// return the hydrated metadata
	return md

}

md_carrier.go - this is essentially 100% copy of the mdCarrier code.

Then in outgoing calls of differing types (REST -> gRPC, gRPC -> REST, REST -> gRPC gateway -> gRPC, etc.) we leverage the above code like this:

ctx = metadata.NewOutgoingContext(tracing.ExtractTraceMD(ctx))
res, err := downstreamAPI.MyAPICall(ctx, &downstreamAPI.MyAPICallRequest{})

Without the copy of the MDCarrier in our codebase, the tracing spans were not being transported between the various services. Hopefully this helps? I won't be upset if you tell me I'm doing something wrong either.

iamnande avatar May 10 '23 14:05 iamnande

Hello! We would totally approve any PR that exposes MDCarrier if you would like to open something.

Just wanted to ask though if our current MDCarrier extraction isn’t sufficient or if you wanted some more custom handling? Right now to our knowledge, our GRPC integrations should handle the MDCarrier extraction properly regardless of the outgoing call so we would love to work together closely to solve a deeper issue if there is one. Thank you!

zarirhamza avatar May 30 '23 14:05 zarirhamza

We understand this is not longer required, and therefore this can be closed.

darccio avatar Apr 16 '24 14:04 darccio