Adds InfluxDB store.

[chris-ramon]

Details

Issue: #98

• Adds new store engine named: InfluxDBStore, which implements Store & Queryer interfaces as:
  • Collect(...) implementation:
    • Saves spans in a measurement named spans in the InfluxDB database named appdash.
    • Span.ID.Trace, Span.ID.Span & Span.ID.Parent are saved as tags - [InfluxDB indexes tags]
    • Span.Annotations are save as fields - [InfluxDB does not index fields]
    • Uses default 'ms' (milliseconds) as Point.Precision.
  • Trace(...) implementation:
    • Queries for all the spans for the given trace, then is wired and returned.
    • Builds tree trace: wires the trace to be returned to include it's children to the correct nesting.
  • Traces(...) implementation.
    • Executes two queries: first gets all root's span for the given traces, second query gets all children spans for each trace, then traces are wired and returned; only N(default: 10) traces are returned.
    • For each trace, builds a tree trace: wires it to include it's children to the correct nesting.
• Adds tests for InfluxDBStore.
• Adds NewInfluxDBStore:
  • Head function which runs the configuration needed for a new instance of InfluxDBStore - such as: run an InfluxDBServer, creates an InfluxDBClient, creates an admin user & appdash database with a retention policy if those do not already exist.
• Adds authentication to InfluxDBServer.
• Adds support to set default retention policy, which tells appdash database for how long time, data should be preserved before deleting it. (it's optional).
• Adds new InfluxDBStore webapp example:
  • Based on examples/cmd/webapp/main.go - run it with:
  • go run examples/cmd/webapp-influxdb/main.go

---

Notes to consider:

• InfluxDB sends anonymous data to m.influxdb.com, see:

  • https://docs.influxdata.com/influxdb/v0.10/administration/config/#reporting-disabled-false
  • https://github.com/influxdata/influxdb/blob/master/etc/config.sample.toml#L3

• InfluxDB version being use is v0.10:

  Clustering, replication, and high-availability are in a beta state.
  The query engine is not optimized for the new TSM engine. A significant refactor of the query engine is in progress targeted for release in version 0.10.1. source

Key Concepts

Measurement

The measurement acts as a container for tags, fields, and the time column, and the measurement name is the description of the data that are stored in the associated fields. Measurement names are strings, and, for any SQL users out there, a measurement is conceptually similar to a table.

Tags

Tags are optional. You don’t need to have tags in your data structure, but it’s generally a good idea to make use of them because, unlike fields, tags are indexed. This means that queries on tags are faster and that tags are ideal for storing commonly-queried metadata.

Fields

Fields are a required piece of InfluxDB’s data structure - you cannot have data in InfluxDB without fields. It’s also important to note that fields are not indexed. Queries that use field values as filters must scan all values that match the other conditions in the query. As a result, those queries are not performant relative to queries on tags (more on tags below). In general, fields should not contain commonly-queried metadata.

Retention Policy

The part of InfluxDB’s data structure that describes for how long InfluxDB keeps data (duration) and how many copies of those data are stored in the cluster (replication factor). RPs are unique per database and along with the measurement and tag set define a series. When you create a database, InfluxDB automatically creates a retention policy called default with an infinite duration and a replication factor set to the number of nodes in the cluster. See Database Management for retention policy management.

Continuous Queries

A CQ is an InfluxQL query that the system runs automatically and periodically within a database. InfluxDB stores the results of the CQ in a specified measurement. CQs require a function in the SELECT clause and must include a GROUP BY time() clause.

[emidoots]

I haven't had a chance to review the entire update yet -- but this is extremely awesome progress! :) Keep up the great work!

Just responding to a few key points, and I'll have more feedback tomorrow.

TODO: decide which Point.Precision should we use here.

Looking that the doc you linked to, it appears to go with ms by default -- which should be precise enough for most (all?) use cases we and others will have. I think using the default is probably fine.

Would it be easy for users to configure this to something else if they do want to change it?

Span.ID.Trace, Span.ID.Span & Span.ID.Parent are save as tags - [InfluxDB indexes tags]

We should not have Span.ID.Span or Span.ID.Parent be tags, they should be fields. Nobody will ever want to query by these, only by Span.ID.Trace -- so the indexing InfluxDB would do would be wasteful I think.

[chris-ramon]

Thanks taking the time to review this one @slimsag! 👍

Looking that the doc you linked to, it appears to go with ms by default -- which should be precise enough for most (all?) use cases we and others will have. I think using the default is probably fine.

Would it be easy for users to configure this to something else if they do want to change it?

Good call, I agree we should let appdash users provide an optional config struct - we could add a new param to NewInfluxDBStore named PointConfig:

func NewInfluxDBStore(c *influxDBServer.Config, bi *influxDBServer.BuildInfo, p PointConfig) (*InfluxDBStore, error) {
  // ...
}

type PointPrecision string
type PointConfig struct {
  Precision PointPrecision
}

Perhaps we should wrap NewInfluxDBStore params within a struct called InfluxDBStoreConfig:

func NewInfluxDBStore(config InfluxDBStoreConfig) (*InfluxDBStore, error) {
  // ...
}

type InfluxDBStoreConfig struct {
  ServerConfig *influxDBServer.Config
  ServerBuildInfo *influxDBServer.BuildInfo
  PointConfig PointConfig
}

---

We should not have Span.ID.Span or Span.ID.Parent be tags, they should be fields. Nobody will ever want to query by these, only by Span.ID.Trace -- so the indexing InfluxDB would do would be wasteful I think.

Yes, we can only keep Span.ID.Trace as tag - current InfluxDBStore implementation relies on Span.ID.Parent to check; if value is empty then must be the root Span otherwise a children Span - but we can improve this by saving a new field named: is_root.

In regards removing Span.ID.Span I've found this method func (t *Trace) FindSpan(spanID ID) *Trace

[emidoots]

Can be written more clearly as just:

tags := map[string]string{
    "trace_id": id.Trace.String(),
    "span_id": id.Span.String(),
    "parent_id": id.Parent.String(),
}

[chris-ramon]

Def agree, we should use map literals here - fixed on: 815bd39

[emidoots]

Perhaps we should wrap NewInfluxDBStore params within a struct called InfluxDBStoreConfig

I like this idea a lot!

[emidoots]

Thanks for the hard work on this @chris-ramon ! I left some comments inline, and everything else you have said I agree with.

Once you're satisfied with the state of this PR for having MemoryStore-like functionality, we should:

1. Make the Queryer interface and the frontend support pagination.
2. Start investigating how we can add AggregateStore-like functionality (N slowest traces, full trace times for last ~72/hours, +last 20000 traces for developer inspection).

Happy to chat with you more on the details about these, just wanted to provide an overview of where to go from here. I've tried out your PR locally and it's great! You've made some awesome progress here!

[emidoots]

If possible, could you adjust the style a bit here (and in other places too)? Prefer a blank newline after comments, like this:

    trace := &Trace{}

    // GROUP BY * -> meaning group by all tags(trace_id, span_id & parent_id)
    // grouping by all tags includes those and it's values on the query response.
    q := fmt.Sprintf("SELECT * FROM spans WHERE trace_id='%s' GROUP BY *", id)
    result, err := in.executeOneQuery(q)
    if err != nil {
        return nil, err
    }

    // result.Series -> A slice containing all the spans.
    if len(result.Series) == 0 {
        return nil, errors.New("trace not found")
    }
    var isRootSpan bool

    // Iterate over series(spans) to create trace children's & set trace fields.
    for _, s := range result.Series {
        span, err := newSpanFromRow(&s)

[chris-ramon]

good call, def improves readability - fixed on: 11e791d

[emidoots]

typo here. s/are uselater/are used later/g

[chris-ramon]

good catch! - fixed on 5fc2a65

[dmitshur]

BTW, I don't think 80 char width should be considered a mandatory strict limit.

Rob Pike said it himself.

https://twitter.com/rob_pike/status/563798709868056576
https://twitter.com/rob_pike/status/563801489190043648
golang/go@a625b91

Most of my code falls within 40-140 characters wide. Sometimes longer when the stuff on the right side is not important.

[chris-ramon]

Good call @shurcooL, I did revert d589fcb which follows strictly 80-chars-wide code style and preserved the one that @slimsag pointed-out above.

[emidoots]

Looks great, thanks! @chris-ramon & @shurcooL :)

[emidoots]

LGTM