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.

Sign up for GitHub

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

Tempo: Fix grpc streaming support over pdc-agent #1

Closed
wants to merge 2 commits into from
Closed

Tempo: Fix grpc streaming support over pdc-agent #1

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9efb0e6
Tempo: Fix grpc streaming support over pdc-agent
taylor-s-dean Jun 30, 2024
be93902
Fix a spelling error and formatting
taylor-s-dean Jun 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion pkg/tsdb/tempo/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,26 @@ func newGrpcClient(ctx context.Context, settings backend.DataSourceInstanceSetti
if err != nil {
return nil, fmt.Errorf("error getting dial options: %w", err)
}
clientConn, err := grpc.NewClient(onlyHost, dialOpts...)

// grpc.Dial() is deprecated in favor of grpc.NewClient(), but grpc.NewClient() changed the default resolver to dns from passthrough.
// This is a problem because the getDialOpts() function appends a custom dialer to the dial options to support Grafana Cloud PDC.
//
// See the following quote from the grpc package documentation:
// One subtle difference between NewClient and Dial and DialContext is that the
// former uses "dns" as the default name resolver, while the latter use
// "passthrough" for backward compatibility. This distinction should not matter
// to most users, but could matter to legacy users that specify a custom dialer
// and expect it to receive the target string directly.
// https://github.com/grpc/grpc-go/blob/fa274d77904729c2893111ac292048d56dcf0bb1/clientconn.go#L209
//
// Unfortunately, the passthrough resolver isn't exported by the grpc package, so we can't use it.
// The options are to continue using grpc.Dial() or implement a custom resolver.
// Since the go-grpc package maintainers intend to continue supporting grpc.Dial() through the 1.x series,
// we'll continue using grpc.Dial() until we have a compelling reason or bandwidth to implement the custom resolver.
// Reference: https://github.com/grpc/grpc-go/blob/f199062ef31ddda54152e1ca5e3d15fb63903dc3/clientconn.go#L204
//
// See this issue for more information: https://github.com/grpc/grpc-go/issues/7091
clientConn, err := grpc.Dial(onlyHost, dialOpts...)
if err != nil {
logger.Error("Error dialing gRPC client", "error", err, "URL", settings.URL, "function", logEntrypoint())
return nil, err
Expand Down