Improvements on `Recorder`.

sourcegraph / appdash

Application tracing system for Go, based on Google's Dapper.

https://sourcegraph.com

Other

1.72k stars 137 forks source link

Improvements on `Recorder`. #135

Closed chris-ramon closed 8 years ago

chris-ramon commented 8 years ago

Details

Adds Recorder.Finish(...) method, such that a span will only be recorded to once (a recorder produces only one Store.Collect(...) call for a given SpanID).
Updates httptrace client & server to call Recorder.Finish(...) in order to collect the span.
Fixes recorder tests.

bg451 commented 8 years ago

I'd vote for AggregateStore not being removed, since one of the nice things about Appdash is that it can be embedded in an application without many dependencies, ie influxdb in this case.

chris-ramon commented 8 years ago

I'd vote for AggregateStore not being removed, since one of the nice things about Appdash is that it can be embedded in an application without many dependencies, ie influxdb in this case.

Hi @bg451, thanks for giving ur thoughts about the AggregateStore being removed, would you mind continue the conversation about it in #127, this being is initial comment. This PR is just being build in top of it but it's main purpose is to improve the Recorder.

slimsag commented 8 years ago

LGTM

Let's def. move the AggregateStore conversation elsewhere.

EDIT: see https://github.com/sourcegraph/appdash/issues/137