feat(new source): New prometheus_remote_write source #5144

bruceg · 2020-11-20T16:37:04Z

I think this is code complete, but I have a couple of questions about non-code bits:

I renamed the existing prometheus source to prometheus_metrics to disambiguate it from this prometheus_remote_write source. I really don't like this choice of name but couldn't think of anything more descriptive. I encourage suggestions so I can improve it.
The output stanza of the cue file is almost certainly wrong. This source translates all input metrics into gauges, unless they are named *_total in which case they are interpreted as counters. Help with encoding that into the metadata would be appreciated.

Closes #4697

Signed-off-by: Bruce Guenter bruce@timber.io

Signed-off-by: Bruce Guenter <bruce@timber.io>

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Signed-off-by: Bruce Guenter <bruce@timber.io>

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

ktff

Regarding names, it is a bit confusing with what's the difference between the two prometheus sources, this also applies to their sinks, so we can use the naming to clarify this a bit. It seams that their differentiating feature is if it's push/pull or serving/accepting based, so what about using that in names, ex. :

prometheus_pull for old prometheus source
prometheus_push for prometheus_remote_write sink
prometheus_serve for prometheus_exporter sink
prometheus_accept for prometheus_remote_write source

binarylogic · 2020-11-25T04:48:53Z

@ktff we landed on:

prometheus_remote_write source
prometheus_remote_write sink
prometheus_exporter sink
prometheus_scrape source

We're trying to use known Prometheus terms. "remote_write" and "exporter" are the standard terms for each Prometheus protocol.

bruceg · 2020-11-25T16:24:00Z

Ping @lukesteensen @JeanMertz @lucperkins Could I get another review on this?

Signed-off-by: Bruce Guenter <bruce@timber.io>

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Signed-off-by: Bruce Guenter <bruce@timber.io>

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Signed-off-by: Bruce Guenter <bruce@timber.io>

docs/reference/components/sources/prometheus_scrape.cue

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Signed-off-by: Bruce Guenter <bruce@timber.io>

StephenWakely · 2020-11-27T17:40:15Z

src/sources/prometheus/scrape.rs

+                                            message = "No path is set on the endpoint and we got a parse error, did you mean to use /metrics? This behavior changed in version 0.11.",
+                                            endpoint = %url
+                                        );
+                            }


Has the formatting messed up here?

I'm surprised the CI doesn't seem to have caught it.

rustfmt occasionally gives up in the presence of macros and leaves code un-reformatted. It's quite frustrating.

In any case, that code should be unchanged since it's just a rename. I'm not sure why GitHub is showing it as a separate add.

src/sources/prometheus/scrape.rs

jszwedko

Nice work! I left a few in-line comments, but nothing strictly blocking.

jszwedko · 2020-11-27T17:58:19Z

Cargo.toml

@@ -335,7 +335,7 @@ sources-kubernetes-logs = ["kubernetes", "transforms-merge", "transforms-regex_p
 sources-logplex = ["sources-utils-http"]
 sources-mongodb_metrics = ["mongodb"]
 sources-nginx_metrics = []
-sources-prometheus = ["prometheus-parser"]
+sources-prometheus = ["prometheus-parser", "snap", "sources-utils-http", "warp"]


I think I would prefer to have sources-prometheus_scrape and sources-promotheus_remote_write as separate features. This would be:

Consistent with the rest of the sources list

Allow easily building just one of the components without the features needed of the other

OTOH there are several interrelated sinks that have a single feature flag: sinks-gcp and sinks-influxdb (and I added sinks-prometheus).

🤔 I see. I feel like I'd prefer to see those ones split up too, but we can punt on that.

jszwedko · 2020-11-27T18:01:04Z

src/internal_events/prometheus.rs

@@ -97,6 +97,69 @@ impl InternalEvent for PrometheusHttpError {
    }
 }

+#[derive(Debug)]
+pub struct PrometheusRemoteWriteParseError {


Nit: I might put these events in separate modules for each source.

src/prometheus.rs

src/sources/prometheus/remote_write.rs

jszwedko · 2020-11-27T18:16:25Z

src/sources/util/mod.rs

@@ -6,8 +6,10 @@ mod tcp;
 #[cfg(all(unix, feature = "sources-utils-unix",))]
 mod unix;

+#[cfg(any(feature = "sources-http", feature = "sources-logplex"))]


As mentioned above, we could choose to support this feature with this source. That would alleviate the need to conditionally compile it here.

Signed-off-by: Bruce Guenter <bruce@timber.io>

src/sources/prometheus/remote_write.rs

Signed-off-by: Bruce Guenter <bruce@timber.io>

* Rename prometheus source to prometheus_scrape (with alias) * Move prometheus shared bits to top level * Add prometheus remote_write source * Unify all the TLS test settings * Add documentation alias for renamed prometheus_exporter sink Signed-off-by: Bruce Guenter <bruce@timber.io> Signed-off-by: Brian Menges <brian.menges@anaplan.com>

Bruce Guenter added 7 commits November 20, 2020 10:24

Rename prometheus source to prometheus_metrics (with alias)

418cf9a

Signed-off-by: Bruce Guenter <bruce@timber.io>

Move prometheus shared bits to top level

8891ea5

Signed-off-by: Bruce Guenter <bruce@timber.io>

Add prometheus remote_write source

847ab87

Signed-off-by: Bruce Guenter <bruce@timber.io>

Unify all the TLS test settings

718320c

Signed-off-by: Bruce Guenter <bruce@timber.io>

Run the tests over both HTTP and HTTPS

d23a3b1

Signed-off-by: Bruce Guenter <bruce@timber.io>

Break request decoding out of decode_body

e815006

Signed-off-by: Bruce Guenter <bruce@timber.io>

Avoid repeatedly cloning snappy decompressor

fbad474

Signed-off-by: Bruce Guenter <bruce@timber.io>

bruceg added source: prometheus type: feature A value-adding code addition that introduce new functionality. labels Nov 20, 2020

bruceg requested review from lukesteensen and lucperkins November 20, 2020 16:37

bruceg requested a review from JeanMertz as a code owner November 20, 2020 16:37

bruceg self-assigned this Nov 20, 2020

Bruce Guenter added 4 commits November 20, 2020 14:35

Fix some issues with make check-component-features

dcf46fd

Signed-off-by: Bruce Guenter <bruce@timber.io>

Merge remote-tracking branch 'origin/master' into prometheus-remote-w…

c8ad9b7

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Chrono has support for rounding subseconds

4a0ad43

Signed-off-by: Bruce Guenter <bruce@timber.io>

Fix the shutdown test for the new prometheus names

4458924

Signed-off-by: Bruce Guenter <bruce@timber.io>

bruceg requested a review from ktff as a code owner November 20, 2020 23:23

Merge remote-tracking branch 'origin/master' into prometheus-remote-w…

a59d051

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

ktff approved these changes Nov 22, 2020

View reviewed changes

Bruce Guenter added 5 commits November 25, 2020 11:18

Rename existing prometheus source to prometheus_scrape

5d066f4

Signed-off-by: Bruce Guenter <bruce@timber.io>

Merge remote-tracking branch 'origin/master' into prometheus-remote-w…

9fded04

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Fix up partial prometheus_scrape source rename

529afc6

Signed-off-by: Bruce Guenter <bruce@timber.io>

Merge remote-tracking branch 'origin/master' into prometheus-remote-w…

ab1b0f3

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Fix some metadata issues

6d24fba

Signed-off-by: Bruce Guenter <bruce@timber.io>

lucperkins reviewed Nov 26, 2020

View reviewed changes

docs/reference/components/sources/prometheus_scrape.cue Show resolved Hide resolved

Bruce Guenter added 2 commits November 27, 2020 10:04

Merge remote-tracking branch 'origin/master' into prometheus-remote-w…

862bf0a

…rite-source Signed-off-by: Bruce Guenter <bruce@timber.io>

Add documentation alias for renamed prometheus_scrape source

acb5dcb

Signed-off-by: Bruce Guenter <bruce@timber.io>

Add documentation alias for renamed prometheus_exporter sink

4a15421

Signed-off-by: Bruce Guenter <bruce@timber.io>

StephenWakely reviewed Nov 27, 2020

View reviewed changes

binarylogic requested review from jszwedko and removed request for lukesteensen and JeanMertz November 27, 2020 17:46

StephenWakely reviewed Nov 27, 2020

View reviewed changes

src/sources/prometheus/scrape.rs Show resolved Hide resolved

jszwedko approved these changes Nov 27, 2020

View reviewed changes

Bruce Guenter added 3 commits November 27, 2020 14:35

Simplify handling of timestamp to UTC conversion

3d605ce

Signed-off-by: Bruce Guenter <bruce@timber.io>

Move the remote_write source tests out of an integration config

bbcaac7

Signed-off-by: Bruce Guenter <bruce@timber.io>

Move defaults only used by the sinks back into the sinks

903bcbb

Signed-off-by: Bruce Guenter <bruce@timber.io>

bruceg requested a review from lukesteensen as a code owner November 27, 2020 20:55

bruceg removed the request for review from lukesteensen November 27, 2020 20:58

StephenWakely reviewed Nov 27, 2020

View reviewed changes

src/sources/prometheus/remote_write.rs Show resolved Hide resolved

Bruce Guenter added 2 commits November 27, 2020 16:31

Fix problem with hosts alias in compat config

3e2d4aa

Signed-off-by: Bruce Guenter <bruce@timber.io>

Add metadata note about metric interpretation.

aa159f9

Signed-off-by: Bruce Guenter <bruce@timber.io>

bruceg mentioned this pull request Nov 27, 2020

Add support for query parameters to the prometheus remote_write source #5268

Open

StephenWakely approved these changes Nov 27, 2020

View reviewed changes

bruceg merged commit ba910af into master Nov 27, 2020

bruceg deleted the prometheus-remote-write-source branch November 27, 2020 23:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(new source): New prometheus_remote_write source #5144

feat(new source): New prometheus_remote_write source #5144

bruceg commented Nov 20, 2020

ktff left a comment •

edited

Loading

binarylogic commented Nov 25, 2020

bruceg commented Nov 25, 2020

StephenWakely Nov 27, 2020

bruceg Nov 27, 2020

bruceg Nov 27, 2020

jszwedko left a comment

jszwedko Nov 27, 2020

bruceg Nov 27, 2020

jszwedko Nov 27, 2020

jszwedko Nov 27, 2020

jszwedko Nov 27, 2020

feat(new source): New prometheus_remote_write source #5144

feat(new source): New prometheus_remote_write source #5144

Conversation

bruceg commented Nov 20, 2020

ktff left a comment • edited Loading

Choose a reason for hiding this comment

binarylogic commented Nov 25, 2020

bruceg commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jszwedko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktff left a comment •

edited

Loading