Skip to content

Commit

Permalink
simplify the Tracer interface by combining the TracerFor... methods
Browse files Browse the repository at this point in the history
  • Loading branch information
marten-seemann committed Jul 11, 2020
1 parent ece3592 commit 269edf1
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 97 deletions.
2 changes: 1 addition & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func dialContext(
c.packetHandlers = packetHandlers

if c.config.Tracer != nil {
c.tracer = c.config.Tracer.TracerForClient(c.destConnID)
c.tracer = c.config.Tracer.TracerForConnection(protocol.PerspectiveClient, c.destConnID)
}
if err := c.dial(ctx); err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ var _ = Describe("Client", func() {
originalClientSessConstructor = newClientSession
tracer = mocks.NewMockConnectionTracer(mockCtrl)
tr := mocks.NewMockTracer(mockCtrl)
tr.EXPECT().TracerForClient(gomock.Any()).Return(tracer).MaxTimes(1)
tr.EXPECT().TracerForConnection(protocol.PerspectiveClient, gomock.Any()).Return(tracer).MaxTimes(1)
config = &Config{Tracer: tr}
Eventually(areSessionsRunning).Should(BeFalse())
// sess = NewMockQuicSession(mockCtrl)
Expand Down
26 changes: 6 additions & 20 deletions internal/mocks/tracer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions logging/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,11 @@ const (

// A Tracer traces events.
type Tracer interface {
// TracerForServer requests a new tracer for a connection that was accepted by the server.
// ConnectionTracer requests a new tracer for a connection.
// The ODCID is the original destination connection ID:
// The destination connection ID that the client used on the first Initial packet it sent on this connection.
// If nil is returned, tracing will be disabled for this connection.
TracerForServer(odcid ConnectionID) ConnectionTracer
// TracerForServer requests a new tracer for a connection that was dialed by the client.
// If nil is returned, tracing will be disabled for this connection.
TracerForClient(odcid ConnectionID) ConnectionTracer
TracerForConnection(p Perspective, odcid ConnectionID) ConnectionTracer
}

// A ConnectionTracer records events.
Expand Down
26 changes: 6 additions & 20 deletions logging/mock_tracer_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 2 additions & 12 deletions logging/multiplex.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,10 @@ func NewMultiplexedTracer(tracers ...Tracer) Tracer {
return &tracerMultiplexer{tracers}
}

func (m *tracerMultiplexer) TracerForServer(odcid ConnectionID) ConnectionTracer {
func (m *tracerMultiplexer) TracerForConnection(p Perspective, odcid ConnectionID) ConnectionTracer {
var connTracers []ConnectionTracer
for _, t := range m.tracers {
if ct := t.TracerForServer(odcid); ct != nil {
connTracers = append(connTracers, ct)
}
}
return newConnectionMultiplexer(connTracers...)
}

func (m *tracerMultiplexer) TracerForClient(odcid ConnectionID) ConnectionTracer {
var connTracers []ConnectionTracer
for _, t := range m.tracers {
if ct := t.TracerForClient(odcid); ct != nil {
if ct := t.TracerForConnection(p, odcid); ct != nil {
connTracers = append(connTracers, ct)
}
}
Expand Down
32 changes: 13 additions & 19 deletions logging/multiplex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,42 +23,36 @@ var _ = Describe("Tracing", func() {
tracer = NewMultiplexedTracer(tr1, tr2)
})

It("multiplexes the TracerForServer call", func() {
tr1.EXPECT().TracerForServer(ConnectionID{1, 2, 3})
tr2.EXPECT().TracerForServer(ConnectionID{1, 2, 3})
tracer.TracerForServer(ConnectionID{1, 2, 3})
})

It("multiplexes the TracerForClient call", func() {
tr1.EXPECT().TracerForClient(ConnectionID{1, 2, 3})
tr2.EXPECT().TracerForClient(ConnectionID{1, 2, 3})
tracer.TracerForClient(ConnectionID{1, 2, 3})
It("multiplexes the TracerForConnection call", func() {
tr1.EXPECT().TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})
tr2.EXPECT().TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})
tracer.TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})
})

It("uses multiple connection tracers", func() {
ctr1 := NewMockConnectionTracer(mockCtrl)
ctr2 := NewMockConnectionTracer(mockCtrl)
tr1.EXPECT().TracerForClient(ConnectionID{1, 2, 3}).Return(ctr1)
tr2.EXPECT().TracerForClient(ConnectionID{1, 2, 3}).Return(ctr2)
tr := tracer.TracerForClient(ConnectionID{1, 2, 3})
tr1.EXPECT().TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3}).Return(ctr1)
tr2.EXPECT().TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3}).Return(ctr2)
tr := tracer.TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3})
ctr1.EXPECT().LossTimerCanceled()
ctr2.EXPECT().LossTimerCanceled()
tr.LossTimerCanceled()
})

It("handles tracers that return a nil ConnectionTracer", func() {
ctr1 := NewMockConnectionTracer(mockCtrl)
tr1.EXPECT().TracerForClient(ConnectionID{1, 2, 3}).Return(ctr1)
tr2.EXPECT().TracerForClient(ConnectionID{1, 2, 3})
tr := tracer.TracerForClient(ConnectionID{1, 2, 3})
tr1.EXPECT().TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3}).Return(ctr1)
tr2.EXPECT().TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3})
tr := tracer.TracerForConnection(PerspectiveServer, ConnectionID{1, 2, 3})
ctr1.EXPECT().LossTimerCanceled()
tr.LossTimerCanceled()
})

It("returns nil when all tracers return a nil ConnectionTracer", func() {
tr1.EXPECT().TracerForClient(ConnectionID{1, 2, 3})
tr2.EXPECT().TracerForClient(ConnectionID{1, 2, 3})
Expect(tracer.TracerForClient(ConnectionID{1, 2, 3})).To(BeNil())
tr1.EXPECT().TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})
tr2.EXPECT().TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})
Expect(tracer.TracerForConnection(PerspectiveClient, ConnectionID{1, 2, 3})).To(BeNil())
})
})

Expand Down
8 changes: 2 additions & 6 deletions metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,8 @@ var _ logging.Tracer = &tracer{}
// NewTracer creates a new metrics tracer.
func NewTracer() logging.Tracer { return &tracer{} }

func (t *tracer) TracerForServer(logging.ConnectionID) logging.ConnectionTracer {
return newConnTracer(t, logging.PerspectiveServer)
}

func (t *tracer) TracerForClient(logging.ConnectionID) logging.ConnectionTracer {
return newConnTracer(t, logging.PerspectiveClient)
func (t *tracer) TracerForConnection(p logging.Perspective, _ logging.ConnectionID) logging.ConnectionTracer {
return newConnTracer(t, p)
}

type connTracer struct {
Expand Down
11 changes: 2 additions & 9 deletions qlog/qlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,9 @@ func NewTracer(getLogWriter func(connectionID []byte) io.WriteCloser) logging.Tr
return &tracer{getLogWriter: getLogWriter}
}

func (t *tracer) TracerForServer(odcid protocol.ConnectionID) logging.ConnectionTracer {
func (t *tracer) TracerForConnection(p logging.Perspective, odcid protocol.ConnectionID) logging.ConnectionTracer {
if w := t.getLogWriter(odcid.Bytes()); w != nil {
return newConnectionTracer(w, protocol.PerspectiveServer, odcid)
}
return nil
}

func (t *tracer) TracerForClient(odcid protocol.ConnectionID) logging.ConnectionTracer {
if w := t.getLogWriter(odcid.Bytes()); w != nil {
return newConnectionTracer(w, protocol.PerspectiveClient, odcid)
return newConnectionTracer(w, p, odcid)
}
return nil
}
Expand Down
5 changes: 2 additions & 3 deletions qlog/qlog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ var _ = Describe("Tracing", func() {
Context("tracer", func() {
It("returns nil when there's no io.WriteCloser", func() {
t := NewTracer(func([]byte) io.WriteCloser { return nil })
Expect(t.TracerForClient(logging.ConnectionID{1, 2, 3, 4})).To(BeNil())
Expect(t.TracerForServer(logging.ConnectionID{1, 2, 3, 4})).To(BeNil())
Expect(t.TracerForConnection(logging.PerspectiveClient, logging.ConnectionID{1, 2, 3, 4})).To(BeNil())
})
})

Expand All @@ -66,7 +65,7 @@ var _ = Describe("Tracing", func() {
BeforeEach(func() {
buf = &bytes.Buffer{}
t := NewTracer(func([]byte) io.WriteCloser { return nopWriteCloser(buf) })
tracer = t.TracerForServer(logging.ConnectionID{0xde, 0xad, 0xbe, 0xef})
tracer = t.TracerForConnection(logging.PerspectiveServer, logging.ConnectionID{0xde, 0xad, 0xbe, 0xef})
})

It("exports a trace that has the right metadata", func() {
Expand Down
2 changes: 1 addition & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ func (s *baseServer) createNewSession(
if origDestConnID.Len() > 0 {
connID = origDestConnID
}
tracer = s.config.Tracer.TracerForServer(connID)
tracer = s.config.Tracer.TracerForConnection(protocol.PerspectiveServer, connID)
}
sess = s.newSession(
&conn{pconn: s.conn, currentAddr: remoteAddr},
Expand Down

0 comments on commit 269edf1

Please sign in to comment.