Skip to content

Clarify when to define multiple prospectors#1091

Merged
ruflin merged 1 commit intoelastic:1.1from
dedemorton:filebeat_#685
Mar 3, 2016
Merged

Clarify when to define multiple prospectors#1091
ruflin merged 1 commit intoelastic:1.1from
dedemorton:filebeat_#685

Conversation

@dedemorton
Copy link
Copy Markdown
Contributor

Remaining changes to close #685

@urso Can you review this and check the example? I took the patterns in the example from the doc, but I am not sure if they are realistic for the files that you might find in the apache2 directory. If they aren't, please suggest a different pattern...I just think we need to show more configuration for the example to demonstrate why someone might want to specify multiple prospectors.

@urso
Copy link
Copy Markdown

urso commented Mar 2, 2016

@ruflin can you have a look, too

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

another use-case is adding 'fields' to a prospector.

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 actually add fields as the first thing to mention. I think currently this is the most common reason people use multiple prospectors as the assign fields to the files

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

one nice use-case I've seen was:

fields:
  json: true
fields_under_root: true

Then in logstash you can filter and do json parsing like:

if [json] {
    json {...}
}

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Mar 2, 2016

@dedemorton I was thinking about this again. We should definitively have the page you created here. The part I'm not sure about is how we can make sure, people don't get the impression only the configurations mentioned above are reasons to create and additional prospector. Every single configuration in the prospector is a reason for it. Just to mention a few: document_type, scan_frequency, multiline, force_close_files. Should we perhaps show all existing config options (without comments) to make sure people understand that each of this config option they can set per prospector?

@dedemorton
Copy link
Copy Markdown
Contributor Author

@ruflin I don't think we should add all config options because we would need to worry about updating the list every time we add a new option. Plus it might actually make the text harder to follow. Instead, I've tried to make the text a bit clearer so users understand that there are a bunch of prospector-specific config options.

@urso I like the use case of adding fields to the output, so I've used that in the example instead of include_lines and exclude_lines.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not sure I like this phrase, as it pretty much sounds like magic:

(each prospector begins with a -)

maybe we should stress the fact config files are YAML and prospectors: requires a YAML array which normally is written by having every element beginning with -

@ruflin changed configs to have input_type: log right after the - (no empty new line). The docs should do the same (for some reason people like to remove the - itself)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@urso OK, I'll make this sound less magical. :-) But I think the change for input_type to come after the dash is for master, right, and not 1.1? If so, I'll change the example after I've cherry-picked the commit into master.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oh right. I think it's in master.

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Mar 3, 2016

LGTM. @dedemorton Ready to be merged? We can still improve small things later.

@dedemorton
Copy link
Copy Markdown
Contributor Author

@ruflin @urso Made a minor change based on urso's comment. If the change looks OK, this is ready to merge.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

change 'json' to 'apache'. Setting 'json: true' is kinda unrealistic for apache logs

@dedemorton
Copy link
Copy Markdown
Contributor Author

Changed json to apache, as urso requested

ruflin added a commit that referenced this pull request Mar 3, 2016
Clarify when to define multiple prospectors
@ruflin ruflin merged commit 2a95c8e into elastic:1.1 Mar 3, 2016
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.

4 participants