Skip to content

Commit 36d5ca0

Browse files
purnesh42Hpurnesh42H
authored
stats: deprecate trace and tags methods and remove all usages from internal code (#7837)
1 parent ee3fb29 commit 36d5ca0

File tree

7 files changed

+82
-100
lines changed

7 files changed

+82
-100
lines changed

‎internal/transport/http2_client.go

-6
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,6 @@ func (t *http2Client) createHeaderFields(ctx context.Context, callHdr *CallHdr)
599599
for k, v := range callAuthData {
600600
headerFields = append(headerFields, hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)})
601601
}
602-
if b := stats.OutgoingTags(ctx); b != nil {
603-
headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-tags-bin", Value: encodeBinHeader(b)})
604-
}
605-
if b := stats.OutgoingTrace(ctx); b != nil {
606-
headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-trace-bin", Value: encodeBinHeader(b)})
607-
}
608602

609603
if md, added, ok := metadataFromOutgoingContextRaw(ctx); ok {
610604
var k string

‎internal/transport/http2_server.go

-6
Original file line numberDiff line numberDiff line change
@@ -539,12 +539,6 @@ func (t *http2Server) operateHeaders(ctx context.Context, frame *http2.MetaHeade
539539
// Attach the received metadata to the context.
540540
if len(mdata) > 0 {
541541
s.ctx = metadata.NewIncomingContext(s.ctx, mdata)
542-
if statsTags := mdata["grpc-tags-bin"]; len(statsTags) > 0 {
543-
s.ctx = stats.SetIncomingTags(s.ctx, []byte(statsTags[len(statsTags)-1]))
544-
}
545-
if statsTrace := mdata["grpc-trace-bin"]; len(statsTrace) > 0 {
546-
s.ctx = stats.SetIncomingTrace(s.ctx, []byte(statsTrace[len(statsTrace)-1]))
547-
}
548542
}
549543
t.mu.Lock()
550544
if t.state != reachable {

‎stats/opencensus/stats.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"google.golang.org/grpc/codes"
3030
"google.golang.org/grpc/grpclog"
3131
"google.golang.org/grpc/internal"
32+
"google.golang.org/grpc/metadata"
3233
"google.golang.org/grpc/stats"
3334
"google.golang.org/grpc/status"
3435
)
@@ -86,26 +87,29 @@ func (csh *clientStatsHandler) statsTagRPC(ctx context.Context, info *stats.RPCT
8687

8788
// Populate gRPC Metadata with OpenCensus tag map if set by application.
8889
if tm := tag.FromContext(ctx); tm != nil {
89-
ctx = stats.SetTags(ctx, tag.Encode(tm))
90+
ctx = metadata.AppendToOutgoingContext(ctx, "grpc-tags-bin", string(tag.Encode(tm)))
9091
}
9192
return ctx, mi
9293
}
9394

9495
// statsTagRPC creates a recording object to derive measurements from in the
9596
// context, scoping the recordings to per RPC server side (scope of the
9697
// context). It also deserializes the opencensus tags set in the context's gRPC
97-
// Metadata, and adds a server method tag to the opencensus tags.
98+
// Metadata, and adds a server method tag to the opencensus tags. If multiple
99+
// tags exist, it adds the last one.
98100
func (ssh *serverStatsHandler) statsTagRPC(ctx context.Context, info *stats.RPCTagInfo) (context.Context, *metricsInfo) {
99101
mi := &metricsInfo{
100102
startTime: time.Now(),
101103
method: info.FullMethodName,
102104
}
103105

104-
if tagsBin := stats.Tags(ctx); tagsBin != nil {
106+
if tgValues := metadata.ValueFromIncomingContext(ctx, "grpc-tags-bin"); len(tgValues) > 0 {
107+
tagsBin := []byte(tgValues[len(tgValues)-1])
105108
if tags, err := tag.Decode(tagsBin); err == nil {
106109
ctx = tag.NewContext(ctx, tags)
107110
}
108111
}
112+
109113
// We can ignore the error here because in the error case, the context
110114
// passed in is returned. If the call errors, the server side application
111115
// layer won't get this key server method information in the tag map, but

‎stats/opencensus/trace.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"go.opencensus.io/trace"
2525
"go.opencensus.io/trace/propagation"
2626

27+
"google.golang.org/grpc/metadata"
2728
"google.golang.org/grpc/stats"
2829
"google.golang.org/grpc/status"
2930
)
@@ -46,7 +47,7 @@ func (csh *clientStatsHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTa
4647
_, span := trace.StartSpan(ctx, mn, trace.WithSampler(csh.to.TS))
4748

4849
tcBin := propagation.Binary(span.SpanContext())
49-
return stats.SetTrace(ctx, tcBin), &traceInfo{
50+
return metadata.AppendToOutgoingContext(ctx, "grpc-trace-bin", string(tcBin)), &traceInfo{
5051
span: span,
5152
countSentMsg: 0, // msg events scoped to scope of context, per attempt client side
5253
countRecvMsg: 0,
@@ -55,12 +56,17 @@ func (csh *clientStatsHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTa
5556

5657
// traceTagRPC populates context with new span data, with a parent based on the
5758
// spanContext deserialized from context passed in (wire data in gRPC metadata)
58-
// if present.
59+
// if present. If multiple spanContexts exist, it takes the last one.
5960
func (ssh *serverStatsHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTagInfo) (context.Context, *traceInfo) {
6061
mn := strings.ReplaceAll(removeLeadingSlash(rti.FullMethodName), "/", ".")
6162

63+
var tcBin []byte
64+
if tcValues := metadata.ValueFromIncomingContext(ctx, "grpc-trace-bin"); len(tcValues) > 0 {
65+
tcBin = []byte(tcValues[len(tcValues)-1])
66+
}
67+
6268
var span *trace.Span
63-
if sc, ok := propagation.FromBinary(stats.Trace(ctx)); ok {
69+
if sc, ok := propagation.FromBinary(tcBin); ok {
6470
// Returned context is ignored because will populate context with data
6571
// that wraps the span instead.
6672
_, span = trace.StartSpanWithRemoteParent(ctx, mn, sc, trace.WithSpanKind(trace.SpanKindServer), trace.WithSampler(ssh.to.TS))

‎stats/stats.go

+16-58
Original file line numberDiff line numberDiff line change
@@ -260,84 +260,42 @@ func (s *ConnEnd) IsClient() bool { return s.Client }
260260

261261
func (s *ConnEnd) isConnStats() {}
262262

263-
type incomingTagsKey struct{}
264-
type outgoingTagsKey struct{}
265-
266263
// SetTags attaches stats tagging data to the context, which will be sent in
267264
// the outgoing RPC with the header grpc-tags-bin. Subsequent calls to
268265
// SetTags will overwrite the values from earlier calls.
269266
//
270-
// NOTE: this is provided only for backward compatibility with existing clients
271-
// and will likely be removed in an upcoming release. New uses should transmit
272-
// this type of data using metadata with a different, non-reserved (i.e. does
273-
// not begin with "grpc-") header name.
267+
// Deprecated: set the `grpc-tags-bin` header in the metadata instead.
274268
func SetTags(ctx context.Context, b []byte) context.Context {
275-
return context.WithValue(ctx, outgoingTagsKey{}, b)
269+
return metadata.AppendToOutgoingContext(ctx, "grpc-tags-bin", string(b))
276270
}
277271

278272
// Tags returns the tags from the context for the inbound RPC.
279273
//
280-
// NOTE: this is provided only for backward compatibility with existing clients
281-
// and will likely be removed in an upcoming release. New uses should transmit
282-
// this type of data using metadata with a different, non-reserved (i.e. does
283-
// not begin with "grpc-") header name.
274+
// Deprecated: obtain the `grpc-tags-bin` header from metadata instead.
284275
func Tags(ctx context.Context) []byte {
285-
b, _ := ctx.Value(incomingTagsKey{}).([]byte)
286-
return b
287-
}
288-
289-
// SetIncomingTags attaches stats tagging data to the context, to be read by
290-
// the application (not sent in outgoing RPCs).
291-
//
292-
// This is intended for gRPC-internal use ONLY.
293-
func SetIncomingTags(ctx context.Context, b []byte) context.Context {
294-
return context.WithValue(ctx, incomingTagsKey{}, b)
295-
}
296-
297-
// OutgoingTags returns the tags from the context for the outbound RPC.
298-
//
299-
// This is intended for gRPC-internal use ONLY.
300-
func OutgoingTags(ctx context.Context) []byte {
301-
b, _ := ctx.Value(outgoingTagsKey{}).([]byte)
302-
return b
276+
traceValues := metadata.ValueFromIncomingContext(ctx, "grpc-tags-bin")
277+
if len(traceValues) == 0 {
278+
return nil
279+
}
280+
return []byte(traceValues[len(traceValues)-1])
303281
}
304282

305-
type incomingTraceKey struct{}
306-
type outgoingTraceKey struct{}
307-
308283
// SetTrace attaches stats tagging data to the context, which will be sent in
309284
// the outgoing RPC with the header grpc-trace-bin. Subsequent calls to
310285
// SetTrace will overwrite the values from earlier calls.
311286
//
312-
// NOTE: this is provided only for backward compatibility with existing clients
313-
// and will likely be removed in an upcoming release. New uses should transmit
314-
// this type of data using metadata with a different, non-reserved (i.e. does
315-
// not begin with "grpc-") header name.
287+
// Deprecated: set the `grpc-trace-bin` header in the metadata instead.
316288
func SetTrace(ctx context.Context, b []byte) context.Context {
317-
return context.WithValue(ctx, outgoingTraceKey{}, b)
289+
return metadata.AppendToOutgoingContext(ctx, "grpc-trace-bin", string(b))
318290
}
319291

320292
// Trace returns the trace from the context for the inbound RPC.
321293
//
322-
// NOTE: this is provided only for backward compatibility with existing clients
323-
// and will likely be removed in an upcoming release. New uses should transmit
324-
// this type of data using metadata with a different, non-reserved (i.e. does
325-
// not begin with "grpc-") header name.
294+
// Deprecated: obtain the `grpc-trace-bin` header from metadata instead.
326295
func Trace(ctx context.Context) []byte {
327-
b, _ := ctx.Value(incomingTraceKey{}).([]byte)
328-
return b
329-
}
330-
331-
// SetIncomingTrace attaches stats tagging data to the context, to be read by
332-
// the application (not sent in outgoing RPCs). It is intended for
333-
// gRPC-internal use.
334-
func SetIncomingTrace(ctx context.Context, b []byte) context.Context {
335-
return context.WithValue(ctx, incomingTraceKey{}, b)
336-
}
337-
338-
// OutgoingTrace returns the trace from the context for the outbound RPC. It is
339-
// intended for gRPC-internal use.
340-
func OutgoingTrace(ctx context.Context) []byte {
341-
b, _ := ctx.Value(outgoingTraceKey{}).([]byte)
342-
return b
296+
traceValues := metadata.ValueFromIncomingContext(ctx, "grpc-trace-bin")
297+
if len(traceValues) == 0 {
298+
return nil
299+
}
300+
return []byte(traceValues[len(traceValues)-1])
343301
}

‎stats/stats_test.go

+31-10
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,27 @@ func payloadToID(p *testpb.Payload) int32 {
9494
return int32(p.Body[0]) + int32(p.Body[1])<<8 + int32(p.Body[2])<<16 + int32(p.Body[3])<<24
9595
}
9696

97+
func setIncomingStats(ctx context.Context, mdKey string, b []byte) context.Context {
98+
md, ok := metadata.FromIncomingContext(ctx)
99+
if !ok {
100+
md = metadata.MD{}
101+
}
102+
md.Set(mdKey, string(b))
103+
return metadata.NewIncomingContext(ctx, md)
104+
}
105+
106+
func getOutgoingStats(ctx context.Context, mdKey string) []byte {
107+
md, ok := metadata.FromOutgoingContext(ctx)
108+
if !ok {
109+
return nil
110+
}
111+
tagValues := md.Get(mdKey)
112+
if len(tagValues) == 0 {
113+
return nil
114+
}
115+
return []byte(tagValues[len(tagValues)-1])
116+
}
117+
97118
type testServer struct {
98119
testgrpc.UnimplementedTestServiceServer
99120
}
@@ -1312,19 +1333,19 @@ func (s) TestTags(t *testing.T) {
13121333
tCtx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
13131334
defer cancel()
13141335
ctx := stats.SetTags(tCtx, b)
1315-
if tg := stats.OutgoingTags(ctx); !reflect.DeepEqual(tg, b) {
1316-
t.Errorf("OutgoingTags(%v) = %v; want %v", ctx, tg, b)
1336+
if tg := getOutgoingStats(ctx, "grpc-tags-bin"); !reflect.DeepEqual(tg, b) {
1337+
t.Errorf("getOutgoingStats(%v, grpc-tags-bin) = %v; want %v", ctx, tg, b)
13171338
}
13181339
if tg := stats.Tags(ctx); tg != nil {
13191340
t.Errorf("Tags(%v) = %v; want nil", ctx, tg)
13201341
}
13211342

1322-
ctx = stats.SetIncomingTags(tCtx, b)
1343+
ctx = setIncomingStats(tCtx, "grpc-tags-bin", b)
13231344
if tg := stats.Tags(ctx); !reflect.DeepEqual(tg, b) {
13241345
t.Errorf("Tags(%v) = %v; want %v", ctx, tg, b)
13251346
}
1326-
if tg := stats.OutgoingTags(ctx); tg != nil {
1327-
t.Errorf("OutgoingTags(%v) = %v; want nil", ctx, tg)
1347+
if tg := getOutgoingStats(ctx, "grpc-tags-bin"); tg != nil {
1348+
t.Errorf("getOutgoingStats(%v, grpc-tags-bin) = %v; want nil", ctx, tg)
13281349
}
13291350
}
13301351

@@ -1333,19 +1354,19 @@ func (s) TestTrace(t *testing.T) {
13331354
tCtx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
13341355
defer cancel()
13351356
ctx := stats.SetTrace(tCtx, b)
1336-
if tr := stats.OutgoingTrace(ctx); !reflect.DeepEqual(tr, b) {
1337-
t.Errorf("OutgoingTrace(%v) = %v; want %v", ctx, tr, b)
1357+
if tr := getOutgoingStats(ctx, "grpc-trace-bin"); !reflect.DeepEqual(tr, b) {
1358+
t.Errorf("getOutgoingStats(%v, grpc-trace-bin) = %v; want %v", ctx, tr, b)
13381359
}
13391360
if tr := stats.Trace(ctx); tr != nil {
13401361
t.Errorf("Trace(%v) = %v; want nil", ctx, tr)
13411362
}
13421363

1343-
ctx = stats.SetIncomingTrace(tCtx, b)
1364+
ctx = setIncomingStats(tCtx, "grpc-trace-bin", b)
13441365
if tr := stats.Trace(ctx); !reflect.DeepEqual(tr, b) {
13451366
t.Errorf("Trace(%v) = %v; want %v", ctx, tr, b)
13461367
}
1347-
if tr := stats.OutgoingTrace(ctx); tr != nil {
1348-
t.Errorf("OutgoingTrace(%v) = %v; want nil", ctx, tr)
1368+
if tr := getOutgoingStats(ctx, "grpc-trace-bin"); tr != nil {
1369+
t.Errorf("getOutgoingStats(%v, grpc-trace-bin) = %v; want nil", ctx, tr)
13491370
}
13501371
}
13511372

‎test/end2end_test.go

+19-14
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"testing"
4242
"time"
4343

44+
"github.com/google/go-cmp/cmp"
4445
"golang.org/x/net/http2"
4546
"golang.org/x/net/http2/hpack"
4647
"google.golang.org/grpc"
@@ -4618,6 +4619,16 @@ func (s) TestStreamingProxyDoesNotForwardMetadata(t *testing.T) {
46184619
}
46194620

46204621
func (s) TestStatsTagsAndTrace(t *testing.T) {
4622+
const mdTraceKey = "grpc-trace-bin"
4623+
const mdTagsKey = "grpc-tags-bin"
4624+
4625+
setTrace := func(ctx context.Context, b []byte) context.Context {
4626+
return metadata.AppendToOutgoingContext(ctx, mdTraceKey, string(b))
4627+
}
4628+
setTags := func(ctx context.Context, b []byte) context.Context {
4629+
return metadata.AppendToOutgoingContext(ctx, mdTagsKey, string(b))
4630+
}
4631+
46214632
// Data added to context by client (typically in a stats handler).
46224633
tags := []byte{1, 5, 2, 4, 3}
46234634
trace := []byte{5, 2, 1, 3, 4}
@@ -4627,17 +4638,11 @@ func (s) TestStatsTagsAndTrace(t *testing.T) {
46274638
endpoint := &stubserver.StubServer{
46284639
EmptyCallF: func(ctx context.Context, _ *testpb.Empty) (*testpb.Empty, error) {
46294640
md, _ := metadata.FromIncomingContext(ctx)
4630-
if tg := stats.Tags(ctx); !reflect.DeepEqual(tg, tags) {
4631-
return nil, status.Errorf(codes.Internal, "stats.Tags(%v)=%v; want %v", ctx, tg, tags)
4632-
}
4633-
if !reflect.DeepEqual(md["grpc-tags-bin"], []string{string(tags)}) {
4634-
return nil, status.Errorf(codes.Internal, "md['grpc-tags-bin']=%v; want %v", md["grpc-tags-bin"], tags)
4635-
}
4636-
if tr := stats.Trace(ctx); !reflect.DeepEqual(tr, trace) {
4637-
return nil, status.Errorf(codes.Internal, "stats.Trace(%v)=%v; want %v", ctx, tr, trace)
4641+
if md[mdTagsKey] == nil || !cmp.Equal(md[mdTagsKey][len(md[mdTagsKey])-1], string(tags)) {
4642+
return nil, status.Errorf(codes.Internal, "md['grpc-tags-bin']=%v; want %v", md[mdTagsKey], tags)
46384643
}
4639-
if !reflect.DeepEqual(md["grpc-trace-bin"], []string{string(trace)}) {
4640-
return nil, status.Errorf(codes.Internal, "md['grpc-trace-bin']=%v; want %v", md["grpc-trace-bin"], trace)
4644+
if md[mdTraceKey] == nil || !cmp.Equal(md[mdTraceKey][len(md[mdTraceKey])-1], string(trace)) {
4645+
return nil, status.Errorf(codes.Internal, "md['grpc-trace-bin']=%v; want %v", md[mdTraceKey], trace)
46414646
}
46424647
return &testpb.Empty{}, nil
46434648
},
@@ -4655,10 +4660,10 @@ func (s) TestStatsTagsAndTrace(t *testing.T) {
46554660
want codes.Code
46564661
}{
46574662
{ctx: ctx, want: codes.Internal},
4658-
{ctx: stats.SetTags(ctx, tags), want: codes.Internal},
4659-
{ctx: stats.SetTrace(ctx, trace), want: codes.Internal},
4660-
{ctx: stats.SetTags(stats.SetTrace(ctx, tags), tags), want: codes.Internal},
4661-
{ctx: stats.SetTags(stats.SetTrace(ctx, trace), tags), want: codes.OK},
4663+
{ctx: setTags(ctx, tags), want: codes.Internal},
4664+
{ctx: setTrace(ctx, trace), want: codes.Internal},
4665+
{ctx: setTags(setTrace(ctx, tags), tags), want: codes.Internal},
4666+
{ctx: setTags(setTrace(ctx, trace), tags), want: codes.OK},
46624667
}
46634668

46644669
for _, tc := range testCases {

0 commit comments

Comments
 (0)