Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Remove NoStats (#603)
Browse files Browse the repository at this point in the history
stats.Record is a low-cost call if there are no views the
user has subscribed to.
  • Loading branch information
rakyll authored Mar 19, 2018
1 parent 2869e62 commit 283e903
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 65 deletions.
12 changes: 2 additions & 10 deletions plugin/ocgrpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ import (
// ClientHandler implements a gRPC stats.Handler for recording OpenCensus stats and
// traces. Use with gRPC clients only.
type ClientHandler struct {
// NoStats may be set to disable recording OpenCensus Stats around each
// gRPC method.
NoStats bool

// StartOptions allows configuring the StartOptions used to create new spans.
StartOptions trace.StartOptions
}
Expand All @@ -45,16 +41,12 @@ func (c *ClientHandler) TagConn(ctx context.Context, cti *stats.ConnTagInfo) con
// HandleRPC implements per-RPC tracing and stats instrumentation.
func (c *ClientHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
traceHandleRPC(ctx, rs)
if !c.NoStats {
c.statsHandleRPC(ctx, rs)
}
c.statsHandleRPC(ctx, rs)
}

// TagRPC implements per-RPC context management.
func (c *ClientHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo) context.Context {
ctx = c.traceTagRPC(ctx, rti)
if !c.NoStats {
ctx = c.statsTagRPC(ctx, rti)
}
ctx = c.statsTagRPC(ctx, rti)
return ctx
}
11 changes: 2 additions & 9 deletions plugin/ocgrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ import (
// present but the SpanContext isn't sampled, then a new trace may be started
// (as determined by Sampler).
type ServerHandler struct {
// NoStats may be set to true to disable recording OpenCensus stats for RPCs.
NoStats bool

// IsPublicEndpoint may be set to true to always start a new trace around
// each RPC. Any SpanContext in the RPC metadata will be added as a linked
// span instead of making it the parent of the span created around the
Expand Down Expand Up @@ -68,16 +65,12 @@ func (s *ServerHandler) TagConn(ctx context.Context, cti *stats.ConnTagInfo) con
// HandleRPC implements per-RPC tracing and stats instrumentation.
func (s *ServerHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
traceHandleRPC(ctx, rs)
if !s.NoStats {
s.statsHandleRPC(ctx, rs)
}
s.statsHandleRPC(ctx, rs)
}

// TagRPC implements per-RPC context management.
func (s *ServerHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo) context.Context {
ctx = s.traceTagRPC(ctx, rti)
if !s.NoStats {
ctx = s.statsTagRPC(ctx, rti)
}
ctx = s.statsTagRPC(ctx, rti)
return ctx
}
4 changes: 2 additions & 2 deletions plugin/ocgrpc/trace_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ func newTracingOnlyTestClientAndServer() (client testpb.FooClient, server *grpc.
if err != nil {
return nil, nil, nil, fmt.Errorf("net.Listen: %v", err)
}
server = grpc.NewServer(grpc.StatsHandler(&ServerHandler{NoStats: true}))
server = grpc.NewServer(grpc.StatsHandler(&ServerHandler{}))
testpb.RegisterFooServer(server, &testServer{})
go server.Serve(listener)

// initialize client
clientConn, err := grpc.Dial(listener.Addr().String(), grpc.WithInsecure(), grpc.WithStatsHandler(&ClientHandler{NoStats: true}), grpc.WithBlock())
clientConn, err := grpc.Dial(listener.Addr().String(), grpc.WithInsecure(), grpc.WithStatsHandler(&ClientHandler{}), grpc.WithBlock())
if err != nil {
return nil, nil, nil, fmt.Errorf("grpc.Dial: %v", err)
}
Expand Down
9 changes: 1 addition & 8 deletions plugin/ochttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ type Transport struct {
// the returned round tripper will be cancelable.
Base http.RoundTripper

// NoStats may be set to disable recording of stats.
NoStats bool

// Propagation defines how traces are propagated. If unspecified, a default
// (currently B3 format) will be used.
Propagation propagation.HTTPFormat
Expand All @@ -59,11 +56,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
format: format,
startOptions: t.StartOptions,
}
if !t.NoStats {
rt = statsTransport{
base: rt,
}
}
rt = statsTransport{base: rt}
return rt.RoundTrip(req)
}

Expand Down
24 changes: 4 additions & 20 deletions plugin/ochttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,11 @@ func TestClient(t *testing.T) {

var noTrace = trace.StartOptions{Sampler: trace.NeverSample()}

func BenchmarkTransportNoInstrumentation(b *testing.B) {
benchmarkClientServer(b, &ochttp.Transport{NoStats: true, StartOptions: noTrace})
}

func BenchmarkTransportTraceOnly(b *testing.B) {
benchmarkClientServer(b, &ochttp.Transport{NoStats: true})
}

func BenchmarkTransportStatsOnly(b *testing.B) {
func BenchmarkTransportNoTrace(b *testing.B) {
benchmarkClientServer(b, &ochttp.Transport{StartOptions: noTrace})
}

func BenchmarkTransportAllInstrumentation(b *testing.B) {
func BenchmarkTransport(b *testing.B) {
benchmarkClientServer(b, &ochttp.Transport{})
}

Expand Down Expand Up @@ -169,19 +161,11 @@ func benchmarkClientServer(b *testing.B, transport *ochttp.Transport) {
}
}

func BenchmarkTransportParallel64NoInstrumentation(b *testing.B) {
benchmarkClientServerParallel(b, 64, &ochttp.Transport{StartOptions: noTrace, NoStats: true})
}

func BenchmarkTransportParallel64TraceOnly(b *testing.B) {
benchmarkClientServerParallel(b, 64, &ochttp.Transport{NoStats: true})
}

func BenchmarkTransportParallel64StatsOnly(b *testing.B) {
func BenchmarkTransportParallel64NoTrace(b *testing.B) {
benchmarkClientServerParallel(b, 64, &ochttp.Transport{StartOptions: noTrace})
}

func BenchmarkTransportParallel64AllInstrumentation(b *testing.B) {
func BenchmarkTransportParallel64(b *testing.B) {
benchmarkClientServerParallel(b, 64, &ochttp.Transport{})
}

Expand Down
16 changes: 5 additions & 11 deletions plugin/ochttp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ import (
//
// Incoming propagation mechanism is determined by the given HTTP propagators.
type Handler struct {
// NoStats may be set to disable recording of stats.
NoStats bool

// Propagation defines how traces are propagated. If unspecified,
// B3 propagation will be used.
Propagation propagation.HTTPFormat
Expand All @@ -60,14 +57,11 @@ type Handler struct {
}

func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var end func()
r, end = h.startTrace(w, r)
defer end()
if !h.NoStats {
var end func()
w, end = h.startStats(w, r)
defer end()
}
var traceEnd, statsEnd func()
r, traceEnd = h.startTrace(w, r)
defer traceEnd()
w, statsEnd = h.startStats(w, r)
defer statsEnd()

handler := h.Handler
if handler == nil {
Expand Down
9 changes: 4 additions & 5 deletions plugin/ochttp/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func TestTransport_RoundTrip(t *testing.T) {
transport := &testTransport{ch: make(chan *http.Request, 1)}

rt := &Transport{
NoStats: true,
Propagation: &testPropagator{},
Base: transport,
}
Expand Down Expand Up @@ -185,26 +184,26 @@ func TestEndToEnd(t *testing.T) {
{
name: "internal default propagation",
handler: &Handler{},
transport: &Transport{NoStats: true},
transport: &Transport{},
wantSameTraceID: true,
},
{
name: "external default propagation",
handler: &Handler{IsPublicEndpoint: true},
transport: &Transport{NoStats: true},
transport: &Transport{},
wantSameTraceID: false,
wantLinks: true,
},
{
name: "internal TraceContext propagation",
handler: &Handler{Propagation: &tracecontext.HTTPFormat{}},
transport: &Transport{NoStats: true, Propagation: &tracecontext.HTTPFormat{}},
transport: &Transport{Propagation: &tracecontext.HTTPFormat{}},
wantSameTraceID: true,
},
{
name: "misconfigured propagation",
handler: &Handler{IsPublicEndpoint: true, Propagation: &tracecontext.HTTPFormat{}},
transport: &Transport{NoStats: true, Propagation: &b3.HTTPFormat{}},
transport: &Transport{Propagation: &b3.HTTPFormat{}},
wantSameTraceID: false,
wantLinks: false,
},
Expand Down

0 comments on commit 283e903

Please sign in to comment.