Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

added encoder for OpenTSDB #1751

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

timurb
Copy link
Contributor

@timurb timurb commented Sep 29, 2015

Hi,
Here is an encoder for OpenTSDB.
It is loosely based on https://github.com/hynd/heka-tsutils-plugins/blob/master/lua_encoders/opentsdb_http.lua but is able to process full-formed JSON with depth of 1 level.
You'll probably have some suggestions on improvement prior to merging this which I'll be happy to fix.

@trixpan
Copy link

trixpan commented Oct 4, 2015

@timurb great work. Looking forward for this to be merged. 👍


- add_hostname_if_missing (boolean, optional, default false)
If no 'host' tag has been seen, append one with the value of the
Hostname field. Deprecated in favour of using the 'fieldfix' filter.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no fieldfix filter, this must be a reference to a custom filter that you're using internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was left from @hynd's implementation. I'll remove the reference.

@rafrombrc
Copy link
Contributor

Overall this looks great, thanks! I made a bunch of comments, but they're pretty much all for small stylistic issues.

In addition to the in-line comments, we'll want to have an entry added to the CHANGES.txt changelog, and the lovely documentation comment you've provided will need to be wired up to the config and sandbox sections of the Sphinx documentation.

@timurb
Copy link
Contributor Author

timurb commented Oct 12, 2015

The code is updated according to your feedback.
I've just realized that to avoid breaking @hynd's setup (whose original plugin I've taken to rework) I have to rename the lua file so it is now called opentsdb_batch.lua.
By the way should I put any credits for his implementation (from which I think only docs and flush function is left in place) and how to do that?

I've updated the docs but have never worked with RST so not very confident all is fine there, please let me know if it looks correct.

@huhongbo
Copy link

+1

@timurb
Copy link
Contributor Author

timurb commented Oct 13, 2015

Should I rewrite the code using read_next_field() in favor of read_raw() or read_raw() should be ok too?

I'm referring to this discussion: https://mail.mozilla.org/pipermail/heka/2015-October/000831.html
We were discussing the decoders and I wonder if anything from here is relevant for encoders too.

@rafrombrc
Copy link
Contributor

@timurb As I said in the mailing list thread, read_message("raw") is safe to use once the message has hit the router. Filter, output, and encode plugins all happen after the router, so read_message("raw") is what you want to use.

Also, using tonumber is fine, although if you break the not skip_fields[name] part into a separate if statement you can put that one first, so tonumber doesn't get called when the field is being skipped.

@timurb
Copy link
Contributor Author

timurb commented Oct 16, 2015

Ok, I'll leave read_message("raw") here then.

Regarding tonumber -- if performance is fine let's leave this as is then. Checking for not skip_fields[name] first will add one more layer of ifs which I believe makes the code harder to read.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants