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

Rewrite Riemann output plugin #1878

Closed
sparrc opened this issue Oct 11, 2016 · 5 comments
Closed

Rewrite Riemann output plugin #1878

sparrc opened this issue Oct 11, 2016 · 5 comments

Comments

@sparrc
Copy link
Contributor

sparrc commented Oct 11, 2016

The Riemann plugin is currently in a bad state, to name a few problems:

  1. there are no unit tests.
  2. there is no README.
  3. it concatenates all tags into the riemann "service" name.
  4. it sends all string metrics as riemann "state" metrics, which probably doesn't make sense to do.
  5. it doesn't use Riemann attributes.
  6. it doesn't set a TTL for any metrics, and doesn't provide the option to.

From a cursory reading of http://riemann.io/concepts.html, I believe that the Riemann output plugin should do the following:

  1. Never set the "state" field, telegraf doesn't have a concept of this so we shouldn't jerry-rig support (string metrics should be skipped).
  2. Riemann Tags should be able to be set explicitly, and also we should be able to reference telegraf tag keys to use the value as a Riemann tag.
  3. The service name should be configurable, and should default to "measurement/field", where measurement and field are keywords that insert the measurement and field name.
  4. All Tags (except host) should be set as attributes.
  5. TTL should be configurable.
  6. Description should be set to something like "A Metric collected from Telegraf."

There should also be unit tests which verify this metric structure. A unit test testing writing the metric to a riemann docker container is probably not necessary in this scenario, since the client library event structs are relatively simple.

PR #1845 fixed some of the problems, but I'd like to instead have a general rewrite that makes the Riemann plugin better and more usable. From what I can tell of it's current state, I don't imagine that it has very many users. We will have to announce this as a breaking change and can continue to support the old riemann plugin by renaming it to riemann_legacy

@sparrc
Copy link
Contributor Author

sparrc commented Oct 11, 2016

see also the collectd output plugin for a good example of the write protocol: https://collectd.org/wiki/index.php/Plugin:Write_Riemann

@JamesClonk
Copy link
Contributor

JamesClonk commented Oct 15, 2016

@sparrc I started working on new version of the riemann plugin. See #1900
Still work-in-progress. Have to write some actual unit & integration tests, and a proper README.md.

It's mostly a cleaned-up version of the old plugin, though it's behaviour should be more obvious and predictable now.
I agree with all of your points except 1) about using state. I need to be able to send strings metrics to Riemann, exactly for handling such state transitions. failing, running, initializing, etc..
I couldn't think of a way how else this could be handled. Any ideas? 😮

@sparrc
Copy link
Contributor Author

sparrc commented Oct 15, 2016

thanks @JamesClonk for the PR

Re: state How are you doing that exactly? are you sending custom metrics through telegraf?

Mostly I'm concerned that it's not going to make sense for the majority of string metrics, which could be anything (but could also be a state). Maybe we could make it a config option? send_strings_as_states?

@JamesClonk
Copy link
Contributor

@sparrc Hmm OK, yes I guess it makes sense to only send string metrics if explicitly requested. I've added a string_as_state configuration option to the plugin (plus some more unit & integration tests, also had to change the riemann docker image to one that allows querying for the integration tests).

Currently I'm sending string/state metrics into Telegraf via the http_listener, the data I'm sending looks similar to this:

monit,process=unbound status="running",monitoring_status="monitored",memory_kilobytes=34612 ...

The input is coming from a simple shellscript that queries the local Monit daemon.
I was thinking about writing a proper input plugin for Monit at some point in the future, but just did not yet find the time.
(That would close this issue: #230)

@JamesClonk
Copy link
Contributor

@sparrc Could you have a look at the Riemann output plugin at #1900, when you have time?
I think it's finished now. We've been using a telegraf built with it for 2 weeks now in our environments and it seems to work well. 😃

@sparrc sparrc closed this as completed in 3fa37a9 Jan 27, 2017
njwhite pushed a commit to njwhite/telegraf that referenced this issue Jan 31, 2017
* rename to riemann_legacy

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* initial draft for Riemann output plugin rewrite

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add unit tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add option to send string metrics as states

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add integration tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add plugin README.md

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* bump riemann library

* clarify settings description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* update Readme.md with updated description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add Riemann event examples

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* use full URL for Riemann server address

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

closes influxdata#1878
mlindes pushed a commit to Comcast/telegraf that referenced this issue Feb 6, 2017
* rename to riemann_legacy

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* initial draft for Riemann output plugin rewrite

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add unit tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add option to send string metrics as states

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add integration tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add plugin README.md

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* bump riemann library

* clarify settings description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* update Readme.md with updated description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add Riemann event examples

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* use full URL for Riemann server address

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

closes influxdata#1878
maxunt pushed a commit that referenced this issue Jun 26, 2018
* rename to riemann_legacy

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* initial draft for Riemann output plugin rewrite

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add unit tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add option to send string metrics as states

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add integration tests

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add plugin README.md

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* bump riemann library

* clarify settings description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* update Readme.md with updated description

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* add Riemann event examples

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

* use full URL for Riemann server address

Signed-off-by: Fabio Berchtold <fabio.berchtold@swisscom.com>

closes #1878
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

No branches or pull requests

2 participants