Skip to content

Add Prometheus remote write output plugin.#5003

Closed
tomwilkie wants to merge 8 commits into
influxdata:masterfrom
tomwilkie:prometheus-remote-write
Closed

Add Prometheus remote write output plugin.#5003
tomwilkie wants to merge 8 commits into
influxdata:masterfrom
tomwilkie:prometheus-remote-write

Conversation

@tomwilkie

@tomwilkie tomwilkie commented Nov 18, 2018

Copy link
Copy Markdown

Add a output plugin that allows Telegraf to send metrics to systems which support Prometheus remote write format (such as Cortex).

Doesn't handle histograms or summaries right now. Tested and it works.

Signed-off-by: Tom Wilkie tom.wilkie@gmail.com

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@tomwilkie tomwilkie changed the title [WIP] Add Prometheus remote write output plugin. Add Prometheus remote write output plugin. Nov 19, 2018
@danielnelson

Copy link
Copy Markdown
Contributor

Thanks for the pull request, this sounds useful. This would work with any of these integrations that are marked as write?

@tomwilkie

Copy link
Copy Markdown
Author

@danielnelson thats right!

@tomwilkie

Copy link
Copy Markdown
Author

@danielnelson anything I can do to help this along?

@glinton
glinton self-requested a review December 6, 2018 21:00
@danielnelson danielnelson added this to the 1.10.0 milestone Dec 17, 2018
Comment thread plugins/outputs/prometheus_remote_write/README.md Outdated
Comment thread plugins/outputs/prometheus_remote_write/prometheus_remote_write.go
Comment thread plugins/outputs/prometheus_remote_write/prometheus_remote_write.go Outdated
}
defer resp.Body.Close()

if resp.StatusCode/100 != 2 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer matching the exact status codes or range, to avoid any potential issue where an error can be missed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The protocol doesn't specify the exact code, and I know some implementation return 200 and 204 (no content).

Comment thread plugins/outputs/prometheus_remote_write/prometheus_remote_write.go
Comment thread plugins/outputs/prometheus_remote_write/README.md Outdated
Comment thread plugins/outputs/prometheus_remote_write/prometheus_remote_write.go Outdated
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like it copied from the prometheus library, is there a package would we need to import to avoid duplicating this file? I would only want to define this again here if it would be a hit on binary size.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The biggest problem is Prometheus imports quite a few projects (for service discovery), including kubernetes, so eventually we end up importing the world and conflicting on versions. I gave it a go and it wasn't as bad as I expected.

copy(labels, commonLabels)
labels = append(labels, &Label{
Name: "__name__",
Value: metric.Name() + "_" + field.Key,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We may wish to mirror this code to special case a few input formats, this is useful for round tripping data collected by the prometheus input.

switch point.Type() {
case telegraf.Counter:
if fn == "counter" {
mname = sanitize(point.Name())
}
case telegraf.Gauge:
if fn == "gauge" {
mname = sanitize(point.Name())
}
}
if mname == "" {
if fn == "value" {
mname = sanitize(point.Name())
} else {
mname = sanitize(fmt.Sprintf("%s_%s", point.Name(), fn))
}
}

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
@tomwilkie
tomwilkie force-pushed the prometheus-remote-write branch from 4feaa00 to 35b28de Compare February 1, 2019 14:55
Required pinning / updating a couple of dependancies, but wasn't as bad as I first throught.

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>
@tomwilkie

Copy link
Copy Markdown
Author

Most of the feedback done; I'd like to refactor and dedupe some of the code with the existing prometheus output format first though, and add support for histograms. WDYT so far?

@danielnelson danielnelson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, feel free to refactor the prometheus output code so long as the output format isn't changed. You can create a package internal/prometheus if you need a place for shared code.

Comment thread Gopkg.toml
version = "0.2.3"

[[constraint]]
name = "github.com/golang/protobuf"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a direct dependency, as is genproto, so they should both stay in the Gopkg.toml. I think perhaps you ran into grpc-ecosystem/grpc-gateway#829, if so we can use the override to workaround the issue:

[[constraint]]
  name = "github.com/golang/protobuf"
  version = "1.1.0"
  version = "1.2.0"

[[override]]
  name = "github.com/golang/protobuf"
  version = "1.2.0"

@danielnelson

Copy link
Copy Markdown
Contributor

@tomwilkie Just a heads up in case you need this to go out with the next release, we are hoping to do an release candidate for 1.10 on the 12th.

Praveen Shukla and others added 2 commits April 25, 2019 21:53
…mp order mismatch

1. Add bearer Token support
2. Sanitize the metrics to follow prometheus format i.e having underscrore.
3. Sorted returns a copy of the metrics in time ascending order.  A copy is
made to avoid modifying the input metric slice since doing so causing
issue of timestamp order mismatch.
Add support of Bearer Token and handle timestamp order mismatch issue
@achilles42

Copy link
Copy Markdown

@tomwilkie @danielnelson What else is pending on this PR? I am happy to work on this if there any work required.

@danielnelson danielnelson modified the milestones: 1.12.0, 1.13.0 Aug 19, 2019
@kushwiz

kushwiz commented Oct 19, 2019

Copy link
Copy Markdown

Please let me know if i can help :)
This is a missing piece to a scalable monitoring solution.

@sjwang90 sjwang90 modified the milestones: 1.13.0, 1.14.0 Nov 15, 2019
@danielnelson

Copy link
Copy Markdown
Contributor

Continued on #5942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants