can you use this contraption instead of the EventLog directly?

I was just talking to @RaduBerinde about figuring out something such that people wanting to log to such EventsLog can use the normal logging functions with a Context with a span from some magic Tracer in it such that the client is oblivious to the fact that it's going to a EventLog underneath.

Review status: 0 of 2 files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

gossip/gossip.go, line 1035 [r1] (raw file):

          log.Infof(ctx, "node has connected to cluster via gossip")
          g.stalled = false
          g.signalConnectedLocked()

This is a logic change not related to logging, right? This call wasn't there before. Perhaps a comment is warranted to explain.

Comments from Reviewable

Yeah - I plan to make EventLogging happen automatically through the Context when you log (just like tracing). That will be useful in a bunch of of other places. I will refactor this code then. In fact I welcome a simple usecase (the queueLog has some races that we need to fix first).

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from Reviewable

We need to Finish() the EventLog, no? Otherwise it will stay open forever (problematic for tests)

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from Reviewable

I don't think we have to (no goroutine attached to it I think) but we should (Done).
Unless Andrei feels strongly, I'd like to go with what Radu proposed and leave the prettifying to him.

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

gossip/gossip.go, line 1035 [r1] (raw file):

Comments from Reviewable

SGTM

On Thu, Aug 25, 2016 at 11:33 AM, Tobias Schottdorf <
notifications@github.com> wrote:

I don't think we have to (no goroutine attached to it I think) but we
should (Done).
Unless Andrei feels strongly, I'd like to go with what Radu proposed and

leave the prettifying to him.

Review status: all files reviewed at latest revision, 1 unresolved

discussion, all commit checks successful.

gossip/gossip.go, line 1035 [r1]
https://reviewable.io:443/reviews/cockroachdb/cockroach/8812#-KQ08PI-TtuhUhiIFptF:-KQ1K7i78dfcpnxUxzdL:b-2i779g
(raw file

cockroach/gossip/gossip.go

Line 1035 in baccf3a

g.signalConnectedLocked()

):
Previously, knz (kena) wrote…

This is a logic change not related to logging, right? This call wasn't
there before. Perhaps a comment is warranted to explain.

Agreed, I pulled out a commit and expanded somewhat.

---

Comments from Reviewable
https://reviewable.io:443/reviews/cockroachdb/cockroach/8812#-:-KQ0x0ae22Fkou3uUOn5:bwksa16

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#8812 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAXBcToNCxrgtRmPLk-RgMWKHuIVmIyMks5qjbW5gaJpZM4Jshsb
.

PTAL

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from Reviewable

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

gossip/gossip.go, line 218 [r2] (raw file):

      events:            trace.NewEventLog("gossip", "gossip"),
  }
  stopper.AddCloser(stop.CloserFn(g.events.Finish))

shouldn't this be inside the New fun above?

gossip/gossip.go, line 872 [r2] (raw file):

func (g *Gossip) bootstrap() {
  g.server.stopper.RunWorker(func() {
      events := trace.NewEventLog("gossip", "bootstrap")

do we really need a new EventLog for bootstrap? Can't you just print begin/end bootstrap events on the main EventLog?

gossip/gossip.go, line 1087 [r3] (raw file):

// (with the exception of the call bootstrap loop) yet necessary. Consolidate
// and add commentary at each callsite.
func (g *Gossip) signalConnectedLocked() {

signal*If*ConnectedLocked ?
or otherwise change the imperative tone of the method name

Comments from Reviewable

Reviewed 1 of 2 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

gossip/gossip.go, line 218 [r2] (raw file):

Comments from Reviewable

Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

gossip/gossip.go, line 218 [r2] (raw file):

Comments from Reviewable

Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

gossip/gossip.go, line 872 [r2] (raw file):

gossip/gossip.go, line 1087 [r3] (raw file):

Comments from Reviewable