giraffi / fluent-plugin-amqp

Use AMQP broker to send or receive messages via FluentD
MIT License
15 stars 31 forks source link

Support header configuration on events - Fixes #35 #40

Closed warmfusion closed 7 years ago

warmfusion commented 7 years ago

Note: This'll have a merge conflict with #39 as Ive refactored tests and changelogs etc.

Plugin now supports multiple headers to be defined on input using the new header configuration section.

See the modified Readme and test cases for how this works in practice.

This fixes #35

elyscape commented 7 years ago

Here's a somewhat simpler version I wrote, which also supports setting the source to nested fields. It might be worthwhile to incorporate that functionality. I didn't submit it as a pull request because I based it on an old version of the plugin for Fluentd 0.12 support.

warmfusion commented 7 years ago

I like the use of a list of headers, first one winning. I'm not sure I see how that supports sub-keys though.

I'll incorporate your use of the loop, but will move the header processing into the HeaderElement class so that it's more decoupled; and may allow for ruby blocks to be passed into the config in the future.

elyscape commented 7 years ago

The part that handles subkeys is this part:

if header.path
  temp_data = data
  temp_path = header.path.dup
  until temp_data.nil? or temp_path.empty?
    temp_data = temp_data[temp_path.shift]
  end
  header_val = temp_data if temp_data
end

The path parameter is a list of key names. This part:

until temp_data.nil? or temp_path.empty?
  temp_data = temp_data[temp_path.shift]
end

removes the first element from temp_path, retrieves the member of temp_data with that key, and replaces temp_data with it, effectively walking the structure. As an example:

Loop iteration temp_data temp_path
0 {'a'=>{'b'=>{'c'=>'d'}}} ['a', 'b', 'c']
1 {'b'=>{'c'=>'d'}} ['b', 'c']
2 {'c'=>'d'} ['c']
3 'd' []

At which point the loop exits and 'd' is returned.

As an aside, the last line should probably be changed to this to avoid potential false negatives:

header_val = temp_data unless temp_data.nil?
warmfusion commented 7 years ago

Oh of course. I saw the shift but was thinking it was a pop. That's fine. 😀

Also means that while the idea of multiple headers is real, it isn't what you're code was doing at all.

Glad I write tests.

warmfusion commented 7 years ago

Righty - I think this'll do the job...

You can set headers, set them multiple times, set defaults, fallbacks, get nested keys, and its all tested through rake - Though I've not actually tried it in anger on a real deployment...

Coverage of the output plugin is at 98% (including a few bits ive chosen to ignore due to untestability)