Skip to content

Commit

Permalink
stats/opencensus: the backend to Sent. Attempt. and Recv. (#6173)
Browse files Browse the repository at this point in the history
  • Loading branch information
zasweq committed Apr 5, 2023
1 parent b0a8b1b commit 10401b9
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 27 deletions.
2 changes: 2 additions & 0 deletions gcp/observability/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ require (
google.golang.org/genproto v0.0.0-20230125152338-dcaf20b6aeaa // indirect
google.golang.org/protobuf v1.30.0 // indirect
)

replace google.golang.org/grpc/stats/opencensus => ../../stats/opencensus
2 changes: 0 additions & 2 deletions gcp/observability/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -598,8 +598,6 @@ google.golang.org/grpc v1.40.0/go.mod h1:ogyxbiOoUXAkP+4+xa6PZSE9DZgIHtSpzjDTB9K
google.golang.org/grpc v1.53.0-dev.0.20230315171901-a1e657ce53ba h1:puuDphNHQZRngQpzUGvfXMBFBv6DuahfWMZaj0jVtjw=
google.golang.org/grpc v1.53.0-dev.0.20230315171901-a1e657ce53ba/go.mod h1:PUSEXI6iWghWaB6lXM4knEgpJNu2qUcKfDtNci3EC2g=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
google.golang.org/grpc/stats/opencensus v0.0.0-20230330193705-4a12595692ae h1:40UWCQ40A2NTDabsmbZNznFf9SUftDlaBASj7OCdKDY=
google.golang.org/grpc/stats/opencensus v0.0.0-20230330193705-4a12595692ae/go.mod h1:qPsHQZhltTPryCUC0naykSpbIJDodlCLM/vNa607CrE=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
Expand Down
39 changes: 22 additions & 17 deletions gcp/observability/observability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package observability
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand All @@ -31,6 +30,7 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"go.opencensus.io/stats/view"
"go.opencensus.io/trace"
"google.golang.org/grpc/internal/envconfig"
Expand Down Expand Up @@ -114,12 +114,15 @@ type traceAndSpanID struct {
traceID trace.TraceID
spanID trace.SpanID
isSampled bool
spanKind int
}

type traceAndSpanIDString struct {
traceID string
spanID string
isSampled bool
// SpanKind is the type of span.
SpanKind int
}

// idsToString is a helper that converts from generated trace and span IDs to
Expand All @@ -129,6 +132,7 @@ func (tasi *traceAndSpanID) idsToString(projectID string) traceAndSpanIDString {
traceID: "projects/" + projectID + "/traces/" + tasi.traceID.String(),
spanID: tasi.spanID.String(),
isSampled: tasi.isSampled,
SpanKind: tasi.spanKind,
}
}

Expand All @@ -142,6 +146,7 @@ func (fe *fakeOpenCensusExporter) ExportSpan(vd *trace.SpanData) {
traceID: vd.TraceID,
spanID: vd.SpanID,
isSampled: vd.IsSampled(),
spanKind: vd.SpanKind,
})
}

Expand Down Expand Up @@ -653,16 +658,16 @@ func (s) TestLoggingLinkedWithTraceClientSide(t *testing.T) {
<-unaryDone.Done()
var tasiSent traceAndSpanIDString
for _, tasi := range traceAndSpanIDs {
if strings.HasPrefix(tasi.spanName, "Sent.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindClient {
tasiSent = tasi.idsToString(projectID)
continue
}
}

fle.mu.Lock()
for _, tasiSeen := range fle.idsSeen {
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{})); diff != "" {
readerErrCh.Send(errors.New("got unexpected id, should be a client span"))
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff != "" {
readerErrCh.Send(fmt.Errorf("got unexpected id, should be a client span (-got, +want): %v", diff))
}
}

Expand Down Expand Up @@ -795,16 +800,16 @@ func (s) TestLoggingLinkedWithTraceServerSide(t *testing.T) {
<-unaryDone.Done()
var tasiServer traceAndSpanIDString
for _, tasi := range traceAndSpanIDs {
if strings.HasPrefix(tasi.spanName, "grpc.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindServer {
tasiServer = tasi.idsToString(projectID)
continue
}
}

fle.mu.Lock()
for _, tasiSeen := range fle.idsSeen {
if diff := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{})); diff != "" {
readerErrCh.Send(errors.New("got unexpected id, should be a server span"))
if diff := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff != "" {
readerErrCh.Send(fmt.Errorf("got unexpected id, should be a server span (-got, +want): %v", diff))
}
}

Expand Down Expand Up @@ -947,20 +952,20 @@ func (s) TestLoggingLinkedWithTrace(t *testing.T) {
var tasiSent traceAndSpanIDString
var tasiServer traceAndSpanIDString
for _, tasi := range traceAndSpanIDs {
if strings.HasPrefix(tasi.spanName, "Sent.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindClient {
tasiSent = tasi.idsToString(projectID)
continue
}
if strings.HasPrefix(tasi.spanName, "grpc.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindServer {
tasiServer = tasi.idsToString(projectID)
}
}

fle.mu.Lock()
for _, tasiSeen := range fle.idsSeen {
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{})); diff != "" {
if diff2 := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{})); diff2 != "" {
readerErrCh.Send(errors.New("got unexpected id, should be client or server span"))
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff != "" {
if diff2 := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff2 != "" {
readerErrCh.Send(fmt.Errorf("got unexpected id, should be a client or server span (-got, +want): %v, %v", diff, diff2))
}
}
}
Expand Down Expand Up @@ -1027,20 +1032,20 @@ func (s) TestLoggingLinkedWithTrace(t *testing.T) {
var tasiSent traceAndSpanIDString
var tasiServer traceAndSpanIDString
for _, tasi := range traceAndSpanIDs {
if strings.HasPrefix(tasi.spanName, "Sent.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindClient {
tasiSent = tasi.idsToString(projectID)
continue
}
if strings.HasPrefix(tasi.spanName, "grpc.") {
if strings.HasPrefix(tasi.spanName, "grpc.") && tasi.spanKind == trace.SpanKindServer {
tasiServer = tasi.idsToString(projectID)
}
}

fle.mu.Lock()
for _, tasiSeen := range fle.idsSeen {
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{})); diff != "" {
if diff2 := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{})); diff2 != "" {
readerErrCh.Send(errors.New("got unexpected id, should be client or server span"))
if diff := cmp.Diff(tasiSeen, &tasiSent, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff != "" {
if diff2 := cmp.Diff(tasiSeen, &tasiServer, cmp.AllowUnexported(traceAndSpanIDString{}), cmpopts.IgnoreFields(traceAndSpanIDString{}, "SpanKind")); diff2 != "" {
readerErrCh.Send(fmt.Errorf("got unexpected id, should be a client or server span (-got, +want): %v, %v", diff, diff2))
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions stats/opencensus/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,8 +1426,7 @@ func (s) TestSpan(t *testing.T) {
sc: trace.SpanContext{
TraceOptions: 1,
},
spanKind: trace.SpanKindClient,
name: "Attempt.grpc.testing.TestService.UnaryCall",
name: "Attempt.grpc.testing.TestService.UnaryCall",
messageEvents: []trace.MessageEvent{
{
EventType: trace.MessageEventTypeSent,
Expand All @@ -1447,7 +1446,7 @@ func (s) TestSpan(t *testing.T) {
TraceOptions: 1,
},
spanKind: trace.SpanKindClient,
name: "Sent.grpc.testing.TestService.UnaryCall",
name: "grpc.testing.TestService.UnaryCall",
hasRemoteParent: false,
childSpanCount: 1,
},
Expand Down Expand Up @@ -1517,16 +1516,15 @@ func (s) TestSpan(t *testing.T) {
TraceOptions: 1,
},
spanKind: trace.SpanKindClient,
name: "Sent.grpc.testing.TestService.FullDuplexCall",
name: "grpc.testing.TestService.FullDuplexCall",
hasRemoteParent: false,
childSpanCount: 1,
},
{
sc: trace.SpanContext{
TraceOptions: 1,
},
spanKind: trace.SpanKindClient,
name: "Attempt.grpc.testing.TestService.FullDuplexCall",
name: "Attempt.grpc.testing.TestService.FullDuplexCall",
messageEvents: []trace.MessageEvent{
{
EventType: trace.MessageEventTypeSent,
Expand Down
2 changes: 1 addition & 1 deletion stats/opencensus/opencensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func ServerOption(to TraceOptions) grpc.ServerOption {
func (csh *clientStatsHandler) createCallSpan(ctx context.Context, method string) (context.Context, *trace.Span) {
var span *trace.Span
if !csh.to.DisableTrace {
mn := "Sent." + strings.Replace(removeLeadingSlash(method), "/", ".", -1)
mn := strings.Replace(removeLeadingSlash(method), "/", ".", -1)
ctx, span = trace.StartSpan(ctx, mn, trace.WithSampler(csh.to.TS), trace.WithSpanKind(trace.SpanKindClient))
}
return ctx, span
Expand Down
2 changes: 1 addition & 1 deletion stats/opencensus/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (csh *clientStatsHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTa
mn := "Attempt." + strings.Replace(removeLeadingSlash(rti.FullMethodName), "/", ".", -1)
// Returned context is ignored because will populate context with data
// that wraps the span instead.
_, span := trace.StartSpan(ctx, mn, trace.WithSampler(csh.to.TS), trace.WithSpanKind(trace.SpanKindClient))
_, span := trace.StartSpan(ctx, mn, trace.WithSampler(csh.to.TS))

tcBin := propagation.Binary(span.SpanContext())
return stats.SetTrace(ctx, tcBin), &traceInfo{
Expand Down

0 comments on commit 10401b9

Please sign in to comment.