logstash-plugins / logstash-output-graphite

Apache License 2.0
9 stars 15 forks source link

Example regarding passing/referencing hash variables in `metrics` seems to be invalid #25

Open zstyblik opened 8 years ago

zstyblik commented 8 years ago

Hello,

it seems to me like the following is either invalid or doesn't work or I got it wrong:

  # When hashes are passed in as values they are broken out into a dotted notation
  # For instance if you configure this plugin with
  # # [source,ruby]
  #     metrics => "mymetrics"
  #
  # and "mymetrics" is a nested hash of '{a => 1, b => { c => 2 }}'
  # this plugin will generate two metrics: a => 1, and b.c => 2 .

because of:

config :metrics, :validate => :hash, :default => {}

Therefore, it's not possible to reference attributes nor pass anything else than hash, eg.

metrics => { "foo" => "bar"}

Thank you for clarification.

zstyblik commented 8 years ago

Is there any way how to tell graphite-output metrics k=>v can be found under eg. metrics?:

{
  '@timestamp': '...'
  'metrics: {
    'foo.bar': 1.0,
    'bar.foo': 2.0,
    [...]
  }
}
wiibaa commented 8 years ago

@zstyblik I'm not used to graphite but toying with the unit tests, the following config

  fields_are_metrics => true
   include_metrics" => "metrics"

seems to do the job, did you try something similar ?

zstyblik commented 8 years ago

@wiibaa, yes and no. I've tried fields_are_metrics, but I couldn't ensure event will have only fields that really are metrics. I'm a bit worried that since include_metrics is a regexp, it will lead to the following...

Now, I should clarify a bit. metrics shouldn't be included, eg. metrics.foo.bar is not wanted. And I don't want to know, in the best case scenario, regexp in advance, because I want generic output. I mean, yeah, such thing makes sense to me. That's why I'd like to point graphite-output to attribute which already has k=>v fields and go get it and that's what I was expecting.

I will give it a try regardless tomorrow. Thanks.

wiibaa commented 8 years ago

Reading again the source code, you can achieve something similar with a different config metrics => { "metrics" => "this value is not read when metrics is a hash"} that could be event driven by a meta field like metrics => {"%{[@meta][metrics_field]}" => "none"} The magic happens because your metrics is a hash in https://github.com/logstash-plugins/logstash-output-graphite/blob/master/lib/logstash/outputs/graphite.rb#L177-L181 By hacking one line to dotify(event[metric], nil).map would allow to remove the prefix as you want, but this is not something achievable by config at the moment. Is it more in line with your initial idea ?

zstyblik commented 8 years ago

@wiibaa, I'm afraid my logstash knowledge is low. I mean, I've tried this and that as you've suggested, but I couldn't get where I want to be :\ The problem might be in ...

filter {
  mutate {
    add_field => {
      "metrics" => { "foo" => "bar" }
    }
  }
}

Which shows up as an array in output { stdout { codec => json } } and not as a hash. I know nested fields should/could be as [metrics][foo], but I'm referencing some of original attributes and I'm worried it won't work out. I'll keep trying :(

EDIT: not even having hash there changed it.

Thank you for your support, though. Appreciate it.

wiibaa commented 8 years ago

I see.. but add_field does not work like this currently, you cannot directly create a hash. A working solution is to write

filter {
  mutate {
    add_field => {
      "[metrics][foo]" => "bar"
    }
  }
}
zstyblik commented 8 years ago

@wiibaa, I just want to let you know that I got it working in the end, sort of. Yes, making sure it really is a hash did the trick. Also:

            fields_are_metrics => true
            exclude_metrics => ["type", "original_type"]
            include_metrics => "[metrics]"

got me as far as metrics.foo.bar .... Note, there are no other fields left in that event, otherwise they'd have to be excluded.

Also,

metrics => { "metrics" => "none" }

works now. I guess that's the best I can get out of it. As for hacking you've mentioned, I will give it a thought/ask about internal possibilities.

Again, thank you very much for support and kicking me in the right direction.

wiibaa commented 8 years ago

It should not be too difficult to turn the hack into a boolean config, the most complex is always naming the config :laughing: Something like skip_prefix_for_metrics_hash with a documentation block explaining your use case going from metrics.foo.bar to simply foo.bar Anyway this plugin documentation need review as you already painfully discovered.

zstyblik commented 8 years ago

@wiibaa, actually, what works is(didn't know, didn't have courage and hope(and time) to try it until now:

metrics => { "%{graphite_prefix}" => "none" }

and I've reached desired state this way. I would say 100%. Because graphite_prefix is usual eg. prod.servers.%{host} ... and it will always be there. If not, if the (internal) requirement is different, that's another story :)