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

feat(new sink): Initial influxdb_metrics sink implementation #1759

Merged
merged 23 commits into from
Feb 19, 2020

Conversation

bednar
Copy link
Contributor

@bednar bednar commented Feb 10, 2020

Closes #562

There is also a work to do, but I hope so that is a good start point to accept PR

TODO:

  • documentation
  • integration tests

@bednar bednar requested a review from lukesteensen as a code owner February 10, 2020 07:45
@bednar bednar changed the title feat(new sink): Add new InfluxDB sink feat(new sink): Add new InfluxDB 2 sink Feb 10, 2020
@binarylogic binarylogic assigned lukesteensen and ghost and unassigned lukesteensen Feb 10, 2020
@binarylogic binarylogic requested review from a user and removed request for lukesteensen February 10, 2020 16:41
@bednar bednar requested a review from binarylogic as a code owner February 11, 2020 09:51
@bednar
Copy link
Contributor Author

bednar commented Feb 11, 2020

I added a documentation so the pull request is ready to review.

@binarylogic
Copy link
Contributor

Thanks! We'll have @a-rodin review this week.

.meta/sinks/influxdb_metrics.toml Outdated Show resolved Hide resolved
.meta/links.toml Outdated Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
.meta/sinks/influxdb_metrics.toml Outdated Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
src/sinks/influxdb_metrics.rs Outdated Show resolved Hide resolved
@binarylogic
Copy link
Contributor

@bednar I know Influx has support for ingesting logs, right? Would it make sense to follow this up with an influx_logs sink at some point?

@binarylogic binarylogic changed the title feat(new sink): Add new InfluxDB 2 sink feat(new sink): Initial influx_metrics sink implementation Feb 11, 2020
@binarylogic binarylogic changed the title feat(new sink): Initial influx_metrics sink implementation feat(new sink): Initial influxdb_metrics sink implementation Feb 11, 2020
@bednar
Copy link
Contributor Author

bednar commented Feb 12, 2020

@binarylogic I think a preferred way is transform logs to metrics and push these metrics into InfluxDB.

@binarylogic binarylogic requested a review from a user February 13, 2020 18:27
use tower::Service;

pub enum Field {
/// string
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like these are very unnecessary doc comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments removed

/// float
Float(f64),
/// unsigned integer
UnsignedInt(u32),
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use std::collections::{BTreeMap, HashMap};
use tower::Service;

pub enum Field {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should include the other types listed on https://v2.docs.influxdata.com/v2.0/reference/syntax/line-protocol/#data-types-and-format ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that other types are unnecessary, but Field is useful for extending in future.

let mut res = client
.post("http://localhost:9999/api/v2/query?org=my-org")
.json(&body)
.header("accept", "application/json")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.header("accept", "application/json")
.header("Accept", "application/json")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ghost
Copy link

ghost commented Feb 14, 2020

@bednar Are you planning to add #1759 (comment) as part of this PR? If not, it is fine, we can merge it as it is and then add support for v1.x later if necessary.

@bednar
Copy link
Contributor Author

bednar commented Feb 14, 2020

@a-rodin I am currently works on it. I hope so that today i fix code and at Monday documentation for that.

@syepes
Copy link

syepes commented Feb 14, 2020

Can wait to see this merged, thanks for the work

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The code looks good to me now. After the docs are updated, this can be merged.

Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
…ring in URL, rename host to endpoint, unsigned int serialized as '*u', InfluxDB start with disabled reporting

Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
@bednar
Copy link
Contributor Author

bednar commented Feb 18, 2020

Conflict with master is resolved

@binarylogic
Copy link
Contributor

@bednar great! Could you give me permission to push to your branch? I'd like to make some documentation changes and then we can merge.

@bednar
Copy link
Contributor Author

bednar commented Feb 18, 2020

@binarylogic Awesome information! Permission granted ;)

@binarylogic binarylogic merged commit b89aded into vectordotdev:master Feb 19, 2020
@binarylogic
Copy link
Contributor

Thanks @bednar! Appreciate the attention to detail and clean commits. Let us know if you'd be interested in working on anything else!

@bednar
Copy link
Contributor Author

bednar commented Feb 19, 2020

Thanks guys for cooperation! @binarylogic @a-rodin

@binarylogic
Copy link
Contributor

@bednar I'm curious if you'd be interested in working on other Vector components? Feel free to email me if you are -- ben@timber.io.

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.

New influxdb sink
5 participants