event: Builder.Start returns a builder
Currently, Builder.Start's second return value is a function which,
when called, sends an End event with a copy of the Start event's
builder. This makes the common case
ctx, end := event.To(ctx).Start(name)
defer end()
easy to write.
But it's not possible to add additional labels that way; instead a
whole new builder needs to be constructed from scratch.
So in this CL, Start's second return value is a Builder. The simple
case is almost as short:
ctx, b := event.To(ctx).Start(name)
defer b.End()
but now it's equally easy to add more information to the end event:
ctx, b := event.To(ctx).Start(name)
defer b.With(Result.Of(err)).End()
Change-Id: Ib489a2704ac78479317eeb3f1f3de0f1b38d610c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/326849
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/event/builder.go b/event/builder.go
index f90804f..a0567e3 100644
--- a/event/builder.go
+++ b/event/builder.go
@@ -259,26 +259,26 @@
}
// Start delivers a start event with the given name and labels.
-// Its second return value is a function that should be called to deliver the
-// matching end event.
+// Its second return value is a Builder whose End method can be
+// called to deliver the matching End event.
+// (It is also fine to ignore the Builder.)
// All events created from the returned context will have this start event
// as their parent.
-func (b Builder) Start(name string) (context.Context, func()) {
+func (b Builder) Start(name string) (context.Context, Builder) {
if b.data == nil {
- return b.ctx, func() {}
+ return b.ctx, Builder{}
}
checkValid(b.data, b.builderID)
ctx := b.ctx
- end := func() {}
+ var endBuilder Builder
if b.data.exporter.tracingEnabled() {
b.data.exporter.mu.Lock()
defer b.data.exporter.mu.Unlock()
b.data.exporter.lastEvent++
traceID := b.data.exporter.lastEvent
// create the end builder
- eb := b.Clone()
- eb.data.Event.Parent = traceID
-
+ endBuilder = b.Clone()
+ endBuilder.data.Event.Parent = traceID
b.data.Event.Labels = append(b.data.Event.Labels, Trace.Of(traceID))
b.data.exporter.prepare(&b.data.Event)
// and now deliver the start event
@@ -286,12 +286,11 @@
now := time.Now()
ctx = newContext(ctx, b.data.exporter, traceID, now)
ctx = b.data.exporter.handler.Start(ctx, &b.data.Event)
- eb.data.parentStart = now
- eb.ctx = ctx
- end = eb.End
+ endBuilder.data.parentStart = now
+ endBuilder.ctx = ctx
}
b.done()
- return ctx, end
+ return ctx, endBuilder
}
func checkValid(b *builder, wantID uint64) {
diff --git a/event/builder_test.go b/event/builder_test.go
index 4f4166c..c7d698f 100644
--- a/event/builder_test.go
+++ b/event/builder_test.go
@@ -68,12 +68,12 @@
// Verify that the context returned from the handler is also returned from Start,
// and is the context passed to End.
ctx := event.WithExporter(context.Background(), event.NewExporter(&testTraceHandler{t: t}, nil))
- ctx, end := event.To(ctx).Start("s")
+ ctx, eb := event.To(ctx).Start("s")
val := ctx.Value("x")
if val != 1 {
t.Fatal("context not returned from Start")
}
- end()
+ eb.End()
}
type testTraceHandler struct {
@@ -142,15 +142,15 @@
}
}
- t.Run("end function", func(t *testing.T) {
+ t.Run("returned builder", func(t *testing.T) {
h := &testTraceDurationHandler{}
ctx := event.WithExporter(context.Background(), event.NewExporter(h, nil))
- ctx, end := event.To(ctx).With(event.DurationMetric.Of(dur)).Start("s")
+ ctx, eb := event.To(ctx).With(event.DurationMetric.Of(dur)).Start("s")
time.Sleep(want)
- end()
+ eb.End()
check(t, h)
})
- t.Run("End method", func(t *testing.T) {
+ t.Run("separate builder", func(t *testing.T) {
h := &testTraceDurationHandler{}
ctx := event.WithExporter(context.Background(), event.NewExporter(h, nil))
ctx, _ = event.To(ctx).Start("s")
diff --git a/event/common_test.go b/event/common_test.go
index 1fdfbd5..c175dae 100644
--- a/event/common_test.go
+++ b/event/common_test.go
@@ -35,12 +35,12 @@
checkFind(t, h, "Annotate", event.Name, false, "")
h.Reset()
- _, end := event.To(ctx).Start(trace)
+ _, eb := event.To(ctx).Start(trace)
checkFind(t, h, "Start", event.Message, false, "")
checkFind(t, h, "Start", event.Name, true, trace)
h.Reset()
- end()
+ eb.End()
checkFind(t, h, "End", event.Message, false, "")
checkFind(t, h, "End", event.Name, false, "")
}
diff --git a/event/event_test.go b/event/event_test.go
index 4a01b72..d2321bb 100644
--- a/event/event_test.go
+++ b/event/event_test.go
@@ -53,8 +53,8 @@
}, {
name: "span",
events: func(ctx context.Context) {
- ctx, end := event.To(ctx).Start("span")
- end()
+ ctx, eb := event.To(ctx).Start("span")
+ eb.End()
},
expect: `
time=2020-03-05T14:27:48 trace=1 name=span
@@ -62,10 +62,10 @@
`}, {
name: "span nested",
events: func(ctx context.Context) {
- ctx, end := event.To(ctx).Start("parent")
- defer end()
- child, end2 := event.To(ctx).Start("child")
- defer end2()
+ ctx, eb := event.To(ctx).Start("parent")
+ defer eb.End()
+ child, eb2 := event.To(ctx).Start("child")
+ defer eb2.End()
event.To(child).Log("message")
},
expect: `
diff --git a/event/otel/trace_test.go b/event/otel/trace_test.go
index fe05ef0..c29f392 100644
--- a/event/otel/trace_test.go
+++ b/event/otel/trace_test.go
@@ -95,9 +95,9 @@
ctx, span = s.tracer.Start(ctx, s.name)
defer span.End()
} else {
- var end func()
- ctx, end = event.To(ctx).Start(s.name)
- defer end()
+ var eb event.Builder
+ ctx, eb = event.To(ctx).Start(s.name)
+ defer eb.End()
}
for _, c := range s.children {
c.apply(ctx)
