Skip to content

Add Logstash output to functionbeat and refactor Logstash output docs#13818

Merged
dedemorton merged 4 commits intoelastic:masterfrom
dedemorton:refactor_logstash
Sep 27, 2019
Merged

Add Logstash output to functionbeat and refactor Logstash output docs#13818
dedemorton merged 4 commits intoelastic:masterfrom
dedemorton:refactor_logstash

Conversation

@dedemorton
Copy link
Contributor

This PR:

  • Breaks outputconfig.asciidoc into multiple files.
  • Changes the libbeat-dir attribute to use docdir as base to make the paths more absolute (makes it easier to move files around).
  • Moves shared-logstash-config.asciidoc content to a tagged region in outputs/output-logstash.asciidoc
  • Adds a tagged region to outputconfig.asciidoc to make it easier for Beats that don't support all outputs to consume the content.
  • Makes attributes used for conditional coding more specific.

@dedemorton dedemorton added docs review needs_backport PR is waiting to be backported to other branches. labels Sep 27, 2019
@dedemorton dedemorton requested a review from a team as a code owner September 27, 2019 02:52
Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

Good stuff here. A couple questions/recommendations.

Before I give this the green thumb, I need to make sure nothing explodes on the APM side. I'm going to check this out and run our make update-beats-docs command to be sure.

@bmorelli25
Copy link
Member

Pending that include file change, these changes seem to work on the APM Server side!

If you're curious: elastic/apm-server#2744.

@bmorelli25
Copy link
Member

bmorelli25 commented Sep 27, 2019

One final APM Server request: Can you please comment out the setup-command-tag tagged region in command-reference.asciidoc so it doesn't render on the website 🤦‍♂. It's here:

tag::setup-command-tag[]

And here:

end::setup-command-tag[]

These should be changed to:

// tag::setup-command-tag[]

and

// end::setup-command-tag[]

This was my fault.

Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

Pending the changes above, LGTM!

@urso
Copy link

urso commented Sep 27, 2019

+100 on separate files per output. To me it looks like things got mostly copied around.

I wonder if this is also an opportunity to refine symbols used by our docs. Especially with settings like 'timeout', some hierarchy can be import to guarantee we always have the correct anchors.

For reference documentation (like the output), I'd like the symbol naming follow the actual setting names (makes it easier to search). For example: ref-output-elasticsearch-setting-timeout (I use 'setting', so to allow sub-section still be named ref-output-elasticsearch-<section>). Same for 'no' patterns: no-output-logstash. WDYT?

@dedemorton
Copy link
Contributor Author

opportunity to refine symbols used by our docs

@urso That's a good idea, but I'd prefer to defer that level of refactoring because I want to have this PR merged by Monday in preparation for the release. I still have other stuff to do. :-/ I will rename the attributes as you suggest, but other renaming should wait until I have more time.

@dedemorton
Copy link
Contributor Author

@urso I've added your request to the meta issue that I have open to track additional refactoring. We will need to be careful here because we don't want to inadvertently change anchor IDs that affect URLs (setting up redirects is possible, but kind of a pain). In fact, it might make more sense to do this for Beats agent rather than refactoring the existing content.

@dedemorton
Copy link
Contributor Author

Doc changes have passed (no code changes) so I'm merging.

@dedemorton dedemorton merged commit 3b1f9e9 into elastic:master Sep 27, 2019
dedemorton added a commit to dedemorton/beats that referenced this pull request Sep 27, 2019
…elastic#13818)

* Refactor logstash output docs

* Add logstash output to functionbeat

* Fix conditional coding to be more flexible

* Resolve review feedback
dedemorton added a commit that referenced this pull request Sep 27, 2019
…#13818) (#13838)

* Refactor logstash output docs

* Add logstash output to functionbeat

* Fix conditional coding to be more flexible

* Resolve review feedback
@dedemorton dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Sep 27, 2019
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…elastic#13818) (elastic#13838)

* Refactor logstash output docs

* Add logstash output to functionbeat

* Fix conditional coding to be more flexible

* Resolve review feedback
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.

3 participants