Skip to content

Commit 208e03b

Browse files
authored
Cherry-pick #8164 #8166 to v1.71.x (#8182)
1 parent 8b7d2fe commit 208e03b

File tree

8 files changed

+119
-25
lines changed

8 files changed

+119
-25
lines changed

Diff for: stats/opentelemetry/client_tracing.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,24 @@ import (
2020
"context"
2121
"strings"
2222

23-
"go.opentelemetry.io/otel"
2423
"go.opentelemetry.io/otel/trace"
24+
"google.golang.org/grpc"
2525
otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
2626
)
2727

28+
const tracerName = "grpc-go"
29+
2830
// traceTagRPC populates provided context with a new span using the
2931
// TextMapPropagator supplied in trace options and internal itracing.carrier.
3032
// It creates a new outgoing carrier which serializes information about this
3133
// span into gRPC Metadata, if TextMapPropagator is provided in the trace
3234
// options. if TextMapPropagator is not provided, it returns the context as is.
3335
func (h *clientStatsHandler) traceTagRPC(ctx context.Context, ai *attemptInfo) (context.Context, *attemptInfo) {
3436
mn := "Attempt." + strings.Replace(ai.method, "/", ".", -1)
35-
tracer := otel.Tracer("grpc-open-telemetry")
37+
tracer := h.options.TraceOptions.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(grpc.Version))
3638
ctx, span := tracer.Start(ctx, mn)
3739
carrier := otelinternaltracing.NewOutgoingCarrier(ctx)
38-
otel.GetTextMapPropagator().Inject(ctx, carrier)
40+
h.options.TraceOptions.TextMapPropagator.Inject(ctx, carrier)
3941
ai.traceSpan = span
4042
return carrier.Context(), ai
4143
}
@@ -48,7 +50,7 @@ func (h *clientStatsHandler) createCallTraceSpan(ctx context.Context, method str
4850
return ctx, nil
4951
}
5052
mn := strings.Replace(removeLeadingSlash(method), "/", ".", -1)
51-
tracer := otel.Tracer("grpc-open-telemetry")
53+
tracer := h.options.TraceOptions.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(grpc.Version))
5254
ctx, span := tracer.Start(ctx, mn, trace.WithSpanKind(trace.SpanKindClient))
5355
return ctx, span
5456
}

Diff for: stats/opentelemetry/e2e_test.go

-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"testing"
2424
"time"
2525

26-
"go.opentelemetry.io/otel"
2726
otelcodes "go.opentelemetry.io/otel/codes"
2827
oteltrace "go.opentelemetry.io/otel/trace"
2928

@@ -100,8 +99,6 @@ func defaultTraceOptions(_ *testing.T) (*experimental.TraceOptions, *tracetest.I
10099
spanProcessor := trace.NewSimpleSpanProcessor(spanExporter)
101100
tracerProvider := trace.NewTracerProvider(trace.WithSpanProcessor(spanProcessor))
102101
textMapPropagator := propagation.NewCompositeTextMapPropagator(opentelemetry.GRPCTraceBinPropagator{})
103-
otel.SetTextMapPropagator(textMapPropagator)
104-
otel.SetTracerProvider(tracerProvider)
105102
traceOptions := &experimental.TraceOptions{
106103
TracerProvider: tracerProvider,
107104
TextMapPropagator: textMapPropagator,

Diff for: stats/opentelemetry/server_metrics.go

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ func (h *serverStatsHandler) unaryInterceptor(ctx context.Context, req any, _ *g
9696
metadataExchangeLabels = h.options.MetricsOptions.pluginOption.GetMetadata()
9797
}
9898

99+
// - Server-side: The first stats event after the RPC request is received.
99100
sts := grpc.ServerTransportStreamFromContext(ctx)
100101

101102
alts := &attachLabelsTransportStream{

Diff for: stats/opentelemetry/server_tracing.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import (
2020
"context"
2121
"strings"
2222

23-
"go.opentelemetry.io/otel"
2423
"go.opentelemetry.io/otel/trace"
24+
"google.golang.org/grpc"
2525
otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
2626
)
2727

@@ -35,8 +35,8 @@ import (
3535
func (h *serverStatsHandler) traceTagRPC(ctx context.Context, ai *attemptInfo) (context.Context, *attemptInfo) {
3636
mn := strings.Replace(ai.method, "/", ".", -1)
3737
var span trace.Span
38-
tracer := otel.Tracer("grpc-open-telemetry")
39-
ctx = otel.GetTextMapPropagator().Extract(ctx, otelinternaltracing.NewIncomingCarrier(ctx))
38+
tracer := h.options.TraceOptions.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(grpc.Version))
39+
ctx = h.options.TraceOptions.TextMapPropagator.Extract(ctx, otelinternaltracing.NewIncomingCarrier(ctx))
4040
// If the context.Context provided in `ctx` to tracer.Start(), contains a
4141
// span then the newly-created Span will be a child of that span,
4242
// otherwise it will be a root span.

Diff for: xds/internal/xdsclient/clientimpl.go

+1-13
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,7 @@ func init() {
113113
internal.TriggerXDSResourceNotFoundForTesting = triggerXDSResourceNotFoundForTesting
114114
xdsclientinternal.ResourceWatchStateForTesting = resourceWatchStateForTesting
115115

116-
// DefaultPool is initialized with bootstrap configuration from one of the
117-
// supported environment variables. If the environment variables are not
118-
// set, then fallback bootstrap configuration should be set before
119-
// attempting to create an xDS client, else xDS client creation will fail.
120-
config, err := bootstrap.GetConfiguration()
121-
if err != nil {
122-
if logger.V(2) {
123-
logger.Infof("Failed to read xDS bootstrap config from env vars: %v", err)
124-
}
125-
DefaultPool = &Pool{clients: make(map[string]*clientRefCounted)}
126-
return
127-
}
128-
DefaultPool = &Pool{clients: make(map[string]*clientRefCounted), config: config}
116+
DefaultPool = &Pool{clients: make(map[string]*clientRefCounted)}
129117
}
130118

131119
// newClientImpl returns a new xdsClient with the given config.

Diff for: xds/internal/xdsclient/pool.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,25 @@ func (p *Pool) newRefCounted(name string, watchExpiryTimeout time.Duration, stre
220220
defer p.mu.Unlock()
221221

222222
if p.config == nil {
223-
return nil, nil, fmt.Errorf("xds: bootstrap configuration not set in the pool")
223+
if len(p.clients) != 0 || p != DefaultPool {
224+
// If the current pool `p` already contains xDS clients or it is not
225+
// the `DefaultPool`, the bootstrap config should have been already
226+
// present in the pool.
227+
return nil, nil, fmt.Errorf("xds: bootstrap configuration not set in the pool")
228+
}
229+
// If the current pool `p` is the `DefaultPool` and has no clients, it
230+
// might be the first time an xDS client is being created on it. So,
231+
// the bootstrap configuration is read from environment variables.
232+
//
233+
// DefaultPool is initialized with bootstrap configuration from one of the
234+
// supported environment variables. If the environment variables are not
235+
// set, then fallback bootstrap configuration should be set before
236+
// attempting to create an xDS client, else xDS client creation will fail.
237+
config, err := bootstrap.GetConfiguration()
238+
if err != nil {
239+
return nil, nil, fmt.Errorf("xds: failed to read xDS bootstrap config from env vars: %v", err)
240+
}
241+
p.config = config
224242
}
225243

226244
if c := p.clients[name]; c != nil {

Diff for: xds/internal/xdsclient/pool/pool_test.go

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
*
3+
* Copyright 2025 gRPC authors.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package pool_test
20+
21+
import (
22+
"encoding/json"
23+
"fmt"
24+
"testing"
25+
26+
"github.com/google/uuid"
27+
"google.golang.org/grpc/internal/grpctest"
28+
"google.golang.org/grpc/internal/testutils"
29+
"google.golang.org/grpc/internal/testutils/stats"
30+
"google.golang.org/grpc/internal/xds/bootstrap"
31+
"google.golang.org/grpc/xds/internal/xdsclient"
32+
)
33+
34+
type s struct {
35+
grpctest.Tester
36+
}
37+
38+
func Test(t *testing.T) {
39+
grpctest.RunSubTests(t, s{})
40+
}
41+
42+
// TestDefaultPool_LazyLoadBootstrapConfig verifies that the DefaultPool
43+
// lazily loads the bootstrap configuration from environment variables when
44+
// an xDS client is created for the first time.
45+
//
46+
// If tries to create the new client in DefaultPool at the start of test when
47+
// none of the env vars are set. This should fail.
48+
//
49+
// Then it sets the env var XDSBootstrapFileName and retry creating a client
50+
// in DefaultPool. This should succeed.
51+
func (s) TestDefaultPool_LazyLoadBootstrapConfig(t *testing.T) {
52+
_, closeFunc, err := xdsclient.DefaultPool.NewClient(t.Name(), &stats.NoopMetricsRecorder{})
53+
if err == nil {
54+
t.Fatalf("xdsclient.DefaultPool.NewClient() succeeded without setting bootstrap config env vars, want failure")
55+
}
56+
57+
bs, err := bootstrap.NewContentsForTesting(bootstrap.ConfigOptionsForTesting{
58+
Servers: []byte(fmt.Sprintf(`[{
59+
"server_uri": %q,
60+
"channel_creds": [{"type": "insecure"}]
61+
}]`, "non-existent-management-server")),
62+
Node: []byte(fmt.Sprintf(`{"id": "%s"}`, uuid.New().String())),
63+
CertificateProviders: map[string]json.RawMessage{
64+
"cert-provider-instance": json.RawMessage("{}"),
65+
},
66+
})
67+
if err != nil {
68+
t.Fatalf("Failed to create bootstrap configuration: %v", err)
69+
}
70+
71+
testutils.CreateBootstrapFileForTesting(t, bs)
72+
if cfg := xdsclient.DefaultPool.BootstrapConfigForTesting(); cfg != nil {
73+
t.Fatalf("DefaultPool.BootstrapConfigForTesting() = %v, want nil", cfg)
74+
}
75+
76+
_, closeFunc, err = xdsclient.DefaultPool.NewClient(t.Name(), &stats.NoopMetricsRecorder{})
77+
if err != nil {
78+
t.Fatalf("Failed to create xDS client: %v", err)
79+
}
80+
defer func() {
81+
closeFunc()
82+
xdsclient.DefaultPool.UnsetBootstrapConfigForTesting()
83+
}()
84+
85+
if xdsclient.DefaultPool.BootstrapConfigForTesting() == nil {
86+
t.Fatalf("DefaultPool.BootstrapConfigForTesting() = nil, want non-nil")
87+
}
88+
}

Diff for: xds/server_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func (s) TestNewServer_Failure(t *testing.T) {
178178
{
179179
desc: "bootstrap env var not set",
180180
serverOpts: []grpc.ServerOption{grpc.Creds(xdsCreds), BootstrapContentsForTesting(nil)},
181-
wantErr: "bootstrap configuration not set in the pool",
181+
wantErr: "failed to read xDS bootstrap config from env vars",
182182
},
183183
{
184184
desc: "empty bootstrap config",

0 commit comments

Comments
 (0)