Improvements on `InfluxDBStore.Trace` & `InfluxDBStore.Traces`

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 `InfluxDBStore.Trace` & `InfluxDBStore.Traces` #119

Closed chris-ramon closed 8 years ago

chris-ramon commented 8 years ago

Details

Issue: #117

[x] Updates: InfluxDB.Trace(...) & InfluxDB.Traces(...) to exclude certain Annotations from the trace/traces being returned:
- For root spans: httptrace.Client & InfluxDB related annotations are excluded.
- For sub spans: InfluxDB related annotations are excluded.

slimsag commented 8 years ago

Thanks for the prompt PR, but sadly I think this doesn't actually fix the issue but rather just addresses the symptoms of the issue.

InfluxDBStore should have no knowledge of httptrace.ClientEvent trace tag prefix at all. I think the real issue here is most likely with how we are gathering the data back out of InfluxDB and reassembling the full trace. #117 and #118 might even be directly related.

I'll have time tomorrow to investigate and discuss the proper fix here further.

chris-ramon commented 8 years ago

Definitely agree on having a better strategy on how we handle the reassemble part when building back annotations from InfluxDB fields, I've submitted a PR: #121 that works/handles it quite better.

slimsag commented 8 years ago

Closing in favor of https://github.com/sourcegraph/appdash/pull/121