Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DialOptions field to Config struct #168

Merged

Conversation

lujiajing1126
Copy link
Contributor

@lujiajing1126 lujiajing1126 commented Mar 10, 2021

Signed-off-by: Megrez Lu [email protected]

Resolve issue #166

Motivation

In order to connect the inter-process traces between implementations of go-plugin and its peer services, we have to inject the existing tracing context from the client side while extracting context should also be done from the server side.

Design

Server Side

No necessary since we can already do the following, (excerpted from jaeger plugin from our private repository)

			grpc.ServeWithGRPCServer(
				store.NewAliyunSLSStore(&appConfig, logger),
				func(options []googleGRPC.ServerOption) *googleGRPC.Server {
					return plugin.DefaultGRPCServer([]googleGRPC.ServerOption{
						googleGRPC.UnaryInterceptor(otgrpc.OpenTracingServerInterceptor(tracer)),
						googleGRPC.StreamInterceptor(otgrpc.OpenTracingStreamServerInterceptor(tracer)),
					})
				},
			)

Client Side

The only possibility to inject the tracing context is to add grpc.UnaryClientInterceptor or grpc.StreamClientInterceptor as an extra DialOption following the idea from grpc-opentracing,

https://github.com/grpc-ecosystem/grpc-opentracing/blob/master/go/otgrpc/client.go

// OpenTracingClientInterceptor returns a grpc.UnaryClientInterceptor suitable
// for use in a grpc.Dial call.
//
// For example:
//
//     conn, err := grpc.Dial(
//         address,
//         ...,  // (existing DialOptions)
//         grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(tracer)))
//
// All gRPC client spans will inject the OpenTracing SpanContext into the gRPC
// metadata; they will also look in the context.Context for an active
// in-process parent Span and establish a ChildOf reference if such a parent
// Span could be found.

Comments

I am not sure about the Broker module.

Signed-off-by: Megrez Lu <[email protected]>
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 10, 2021

CLA assistant check
All committers have signed the CLA.

@tanner-bruce
Copy link

@mitchellh @jbardin what would it take to get this merged? We'd like to use this for jaeger as well as grafana. Thanks!

@genert
Copy link

genert commented May 1, 2021

CC: @mitchellh @jbardin

@mitchellh
Copy link
Contributor

This looks good, sorry I missed this. I'm going to redo some of the docs but otherwise I'll send this.

@mitchellh mitchellh merged commit 87f3bd0 into hashicorp:master Jun 3, 2021
@lujiajing1126 lujiajing1126 deleted the feature/allow-grpc-dial-option branch June 4, 2021 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants