Skip to content

Commit 85e85ad

Browse files
mohanli-mlmohanli-ml
authored
feat(transport): Log DirectPath misconfiguration (#2225)
We met issues where customers wanted to use DirectPath, but DirectPath was misconfigured and it took 2 weeks for debugging. So we want to add WARNING logs if DirectPath flag/option is set but DirectPath is not used. Issue: https://togithub.com/googleapis/sdk-platform-java/issues/2164. Java PR: https://togithub.com/googleapis/sdk-platform-java/pull/2105. Note that the log is added when a channel is created because we need to check the OAuth token, which means it will be logged every channel. This is a bad behavior if customer has a large channel pool size. Ideally I want to use something like LOG_EVERY_N_SEC, and I found `rate.Sometimes` can do this. Cody, do you have suggestions here? @codyoss @frankyn
1 parent f56fb11 commit 85e85ad

File tree

4 files changed

+107
-0
lines changed

4 files changed

+107
-0
lines changed

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ require (
2626
golang.org/x/crypto v0.14.0 // indirect
2727
golang.org/x/sys v0.13.0 // indirect
2828
golang.org/x/text v0.13.0 // indirect
29+
golang.org/x/time v0.3.0 // indirect
2930
google.golang.org/appengine v1.6.7 // indirect
3031
google.golang.org/genproto v0.0.0-20231016165738-49dd2c1f3d0b // indirect
3132
google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b // indirect

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
9595
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
9696
golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
9797
golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
98+
golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
99+
golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
98100
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
99101
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
100102
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=

‎transport/grpc/dial.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ import (
1414
"net"
1515
"os"
1616
"strings"
17+
"time"
1718

1819
"cloud.google.com/go/compute/metadata"
1920
"go.opencensus.io/plugin/ocgrpc"
2021
"golang.org/x/oauth2"
22+
"golang.org/x/time/rate"
2123
"google.golang.org/api/internal"
2224
"google.golang.org/api/option"
2325
"google.golang.org/grpc"
@@ -38,6 +40,9 @@ const enableDirectPathXds = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS"
3840
// Set at init time by dial_socketopt.go. If nil, socketopt is not supported.
3941
var timeoutDialerOption grpc.DialOption
4042

43+
// Log rate limiter
44+
var logRateLimiter = rate.Sometimes{Interval: 1 * time.Second}
45+
4146
// Dial returns a GRPC connection for use communicating with a Google cloud
4247
// service, configured with the given ClientOptions.
4348
func Dial(ctx context.Context, opts ...option.ClientOption) (*grpc.ClientConn, error) {
@@ -153,6 +158,9 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C
153158
)
154159

155160
// Attempt Direct Path:
161+
logRateLimiter.Do(func() {
162+
logDirectPathMisconfig(endpoint, creds.TokenSource, o)
163+
})
156164
if isDirectPathEnabled(endpoint, o) && isTokenSourceDirectPathCompatible(creds.TokenSource, o) && metadata.OnGCE() {
157165
// Overwrite all of the previously specific DialOptions, DirectPath uses its own set of credentials and certificates.
158166
grpcOpts = []grpc.DialOption{
@@ -296,6 +304,24 @@ func checkDirectPathEndPoint(endpoint string) bool {
296304
return true
297305
}
298306

307+
func logDirectPathMisconfig(endpoint string, ts oauth2.TokenSource, o *internal.DialSettings) {
308+
if isDirectPathXdsUsed(o) {
309+
// Case 1: does not enable DirectPath
310+
if !isDirectPathEnabled(endpoint, o) {
311+
log.Println("WARNING: DirectPath is misconfigured. Please set the EnableDirectPath option along with the EnableDirectPathXds option.")
312+
} else {
313+
// Case 2: credential is not correctly set
314+
if !isTokenSourceDirectPathCompatible(ts, o) {
315+
log.Println("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used.")
316+
}
317+
// Case 3: not running on GCE
318+
if !metadata.OnGCE() {
319+
log.Println("WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.")
320+
}
321+
}
322+
}
323+
}
324+
299325
func processAndValidateOpts(opts []option.ClientOption) (*internal.DialSettings, error) {
300326
var o internal.DialSettings
301327
for _, opt := range opts {

‎transport/grpc/dial_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,15 @@
55
package grpc
66

77
import (
8+
"bytes"
9+
"context"
10+
"log"
11+
"strings"
812
"testing"
13+
14+
"cloud.google.com/go/compute/metadata"
15+
"golang.org/x/oauth2/google"
16+
"google.golang.org/api/internal"
917
)
1018

1119
func TestCheckDirectPathEndPoint(t *testing.T) {
@@ -47,3 +55,73 @@ func TestCheckDirectPathEndPoint(t *testing.T) {
4755
})
4856
}
4957
}
58+
59+
func TestLogDirectPathMisconfigAttrempDirectPathNotSet(t *testing.T) {
60+
o := &internal.DialSettings{}
61+
o.EnableDirectPathXds = true
62+
63+
endpoint := "abc.googleapis.com"
64+
65+
creds, err := internal.Creds(context.Context(context.Background()), o)
66+
if err != nil {
67+
t.Fatalf("failed to create creds")
68+
}
69+
70+
buf := bytes.Buffer{}
71+
log.SetOutput(&buf)
72+
73+
logDirectPathMisconfig(endpoint, creds.TokenSource, o)
74+
75+
wantedLog := "WARNING: DirectPath is misconfigured. Please set the EnableDirectPath option along with the EnableDirectPathXds option."
76+
if !strings.Contains(buf.String(), wantedLog) {
77+
t.Fatalf("got: %v, want: %v", buf.String(), wantedLog)
78+
}
79+
80+
}
81+
82+
func TestLogDirectPathMisconfigWrongCredential(t *testing.T) {
83+
o := &internal.DialSettings{}
84+
o.EnableDirectPath = true
85+
o.EnableDirectPathXds = true
86+
87+
endpoint := "abc.googleapis.com"
88+
89+
creds := &google.Credentials{}
90+
91+
buf := bytes.Buffer{}
92+
log.SetOutput(&buf)
93+
94+
logDirectPathMisconfig(endpoint, creds.TokenSource, o)
95+
96+
wantedLog := "WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used."
97+
if !strings.Contains(buf.String(), wantedLog) {
98+
t.Fatalf("got: %v, want: %v", buf.String(), wantedLog)
99+
}
100+
101+
}
102+
103+
func TestLogDirectPathMisconfigNotOnGCE(t *testing.T) {
104+
o := &internal.DialSettings{}
105+
o.EnableDirectPath = true
106+
o.EnableDirectPathXds = true
107+
108+
endpoint := "abc.googleapis.com"
109+
110+
creds, err := internal.Creds(context.Context(context.Background()), o)
111+
if err != nil {
112+
t.Fatalf("failed to create creds")
113+
}
114+
115+
buf := bytes.Buffer{}
116+
log.SetOutput(&buf)
117+
118+
logDirectPathMisconfig(endpoint, creds.TokenSource, o)
119+
120+
if !metadata.OnGCE() {
121+
wantedLog := "WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.."
122+
if !strings.Contains(buf.String(), wantedLog) {
123+
t.Fatalf("got: %v, want: %v", buf.String(), wantedLog)
124+
}
125+
}
126+
127+
}

0 commit comments

Comments
 (0)