Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add influxdb client #49

Merged
merged 4 commits into from
Jul 24, 2017
Merged

add influxdb client #49

merged 4 commits into from
Jul 24, 2017

Conversation

achille-roussel
Copy link
Contributor

This PR adds support for InfluxDB to the stats package. The intent here is to have a InfluxDB client so I can start experimenting with it without having to rewrite any instrumentation code.

The client API is very similar to the one for datadog, basically:

stats.Register(influxdb.NewClient("localhost:8086"))

I spent a bit of time profiling and optimizing the code, basically I was thinking of taking two different approaches to manage the batches of metrics:

  • manage a list of buffers, each containing the serialized representation of a metric
  • capture the metric objects themselves in a list which is then serialized when the batch is flushed

I ended going for the first solution because it was the most time-efficient implementation (~200ns per metric), however it requires sorting the tags when handling the metrics, which is on the hot path here... if sorting ends up being a bottleneck we can revisit and see if we can optimize the second approach.
The implementation also is fully wait-free, it uses atomic operations to synchronize concurrent operations on shared resources, this prevents having a high contention point if we were using a mutex in HandleMetrics.

Please take a look and let me know if you'd like to see anything changed.

Copy link
Contributor

@yields yields left a comment

Choose a reason for hiding this comment

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

this looks great to me! :)

@achille-roussel
Copy link
Contributor Author

Thanks for the review!

@achille-roussel achille-roussel merged commit 29b56b2 into master Jul 24, 2017
@achille-roussel achille-roussel deleted the influxdb branch July 24, 2017 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants