Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

Improvements on Recorder. #135

Merged
merged 1 commit into from
Apr 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions aggregate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestAggregateStore(t *testing.T) {
E: time.Now(),
}
rec.Event(e)
rec.Finish()
if errs := rec.Errors(); len(errs) > 0 {
t.Fatal(errs)
}
Expand Down Expand Up @@ -103,6 +104,7 @@ func TestAggregateStoreNSlowest(t *testing.T) {
E: now.Add(times[i]),
}
rec.Event(e)
rec.Finish()
if errs := rec.Errors(); len(errs) > 0 {
t.Fatal(errs)
}
Expand Down Expand Up @@ -218,6 +220,7 @@ func TestAggregateStoreMinEvictAge(t *testing.T) {
E: time.Now(),
}
rec.Event(e)
rec.Finish()
if errs := rec.Errors(); len(errs) > 0 {
t.Fatal(errs)
}
Expand All @@ -238,6 +241,7 @@ func TestAggregateStoreMinEvictAge(t *testing.T) {
// Trigger the eviction by making any sort of collection.
rec := NewRecorder(NewRootSpanID(), as)
rec.Name("collect")
rec.Finish()
if errs := rec.Errors(); len(errs) > 0 {
t.Fatal(errs)
}
Expand Down
1 change: 1 addition & 0 deletions httptrace/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ func (t *Transport) RoundTrip(original *http.Request) (*http.Response, error) {
e.Response.StatusCode = -1
}
child.Event(e)
child.Finish()
return resp, err
}

Expand Down
1 change: 1 addition & 0 deletions httptrace/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func Middleware(c appdash.Collector, conf *MiddlewareConfig) func(rw http.Respon
rec.Name(r.URL.Host + r.URL.Path)
}
rec.Event(e)
rec.Finish()
}
}

Expand Down
27 changes: 25 additions & 2 deletions recorder.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package appdash

import (
"errors"
"fmt"
"sync"
"time"
)

var (
errMultipleFinishCalls = errors.New("multiple Recorder.Finish calls")
)

// A Recorder is associated with a span and records annotations on the
// span by sending them to a collector.
type Recorder struct {
SpanID // the span ID that annotations are about
SpanID // the span ID that annotations are about
annotations []Annotation // SpanID's annotations to be collected
finished bool // finished is whether Recorder.Finish was called

collector Collector // the collector to send to

Expand Down Expand Up @@ -65,7 +72,23 @@ func (r *Recorder) Event(e Event) {
r.error("Event", err)
return
}
r.Annotation(as...)
r.annotations = append(r.annotations, as...)
}

// Finish finishes recording and saves the recorded information to the
// underlying collector. If Finish is not called, then no data will be written
// to the underlying collector.
// Finish must be called once, otherwise r.error is called, this constraint
// ensures that collector is called once per Recorder, in order to avoid
// for performance reasons extra operations(span look up & span's annotations update)
// within the collector.
func (r *Recorder) Finish() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add:

// Finish finishes recording and saves the recorded information to the
// underlying collector. If Finish is not called, then no data will be written
// to the underlying collector.

if r.finished {
r.error("Finish", errMultipleFinishCalls)
return
}
r.finished = true
r.Annotation(r.annotations...)
}

// Annotation records raw annotations on the span.
Expand Down
23 changes: 15 additions & 8 deletions recorder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,18 @@ func TestRecorder(t *testing.T) {
r := NewRecorder(id, c)

r.Msg("msg")
if calledCollect != 1 {
t.Errorf("got calledCollect %d, want 1", calledCollect)
r.Name("name")

if calledCollect != 0 {
t.Errorf("got calledCollect %d, want 0", calledCollect)
}

r.Finish()

if diff := diffAnnotationsFromEvent(anns, Msg("msg")); len(diff) > 0 {
t.Errorf("got diff annotations for Msg event:\n%s", strings.Join(diff, "\n"))
}

r.Name("name")
if calledCollect != 2 {
t.Errorf("got calledCollect %d, want 1", calledCollect)
}
if diff := diffAnnotationsFromEvent(anns, spanName{"name"}); len(diff) > 0 {
t.Errorf("got diff annotations for spanName event:\n%s", strings.Join(diff, "\n"))
}
Expand All @@ -53,9 +54,10 @@ func TestRecorder_Errors(t *testing.T) {
r := NewRecorder(SpanID{}, c)

r.Msg("msg")
if calledCollect != 2 {
t.Errorf("got calledCollect %d, want 1", calledCollect)
if calledCollect != 0 {
t.Errorf("got calledCollect %d, want 0", calledCollect)
}
r.Finish()
errs := r.Errors()
if want := []error{collectErr}; !reflect.DeepEqual(errs, want) {
t.Errorf("got errors %v, want %v", errs, want)
Expand All @@ -64,6 +66,11 @@ func TestRecorder_Errors(t *testing.T) {
if errs := r.Errors(); len(errs) != 0 {
t.Errorf("got len(errs) == %d, want 0 (after call to Errors)", len(errs))
}

r.Finish()
if errs := r.Errors(); len(errs) != 1 && errs[0] != errMultipleFinishCalls {
t.Errorf("got %d errors, want 1", len(errs))
}
}

func diffAnnotationsFromEvent(anns Annotations, e Event) (diff []string) {
Expand Down