Skip to content

Conversation

@dedemorton
Copy link
Contributor

@dedemorton dedemorton commented Jan 13, 2018

These changes have a dependency on elastic/logstash#8937.

@jsvd I've added comments here so you can tell what needs to be fixed in the script vs bad asciidoc that's in older versions of plugins.

The script definitely needs to be changed to:

  • Convert the format of all links that point to the Logstash Reference for info about types. For example: Change <<{version}-plugins-{type}s-{plugin}-name>> |<<string,string>> to {logstash-ref}/configuration-file-structure.html#string[string] This needs to be done for every type.

  • add {version} to all links that are internal. It probably makes sense to fix the format of the external links before we try to add {version} to internal links. Once we get all of the external links tagged correctly, we can assume that any links beginning with << and ending with >> are internal.

Open issues:

  • We need to decide how we want to handle the common files in the include directory. The current paths point to the files in logstash/docs/inclue. I've created a parallel PR in Update shared files to support versioned plugin docs logstash#8937 that (mostly) fixes those files to work with versioned plugins. However, we need to think about what happens when we add common options in the future. We don't want the old plugins docs to show new options. We might want to take a snapshot of the files in logstash/docs/include and create versioned directories (or something like that) in logstash-docs. Those files probably won't change often, but we should plan for it. For example, when we remove the type option, we don't want it to disappear from all the v5 docs right?
  • Do we want to have multiple versions of the versioned plugin reference? The files will end up in later 6.x branches whether we want them there or not. The question is whether we want to build those files as part of the docs, or just ignore them. If we build the files, then users will be able to select branches from the drop-down list in the docs. So basically they would select the version based on which version of Logtash they have installed, and then they would see all the available plugin docs for that version. I'm leaning towards the idea of only having the docs in master.

| <<{version}-plugins-{type}s-{plugin}-signature>> |<<string,string>>|No
| <<{version}-plugins-{type}s-{plugin}-vendor>> |<<string,string>>|No
| <<{version}-plugins-{type}s-{plugin}-version>> |<<string,string>>|No
| <<{version}-plugins-{type}s-{plugin}-delimiter>> |{logstash-ref}/configuration-file-structure.html#string[string]|No
Copy link
Contributor Author

@dedemorton dedemorton Jan 13, 2018

Choose a reason for hiding this comment

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

The script needs to replace all type links so that they point to the Logstash Reference.

@dedemorton dedemorton force-pushed the jsvd-versioned_plugin_docs branch 2 times, most recently from 88046f9 to 87c1543 Compare January 13, 2018 01:42
@dedemorton dedemorton force-pushed the jsvd-versioned_plugin_docs branch 3 times, most recently from e9975dd to af38a05 Compare January 13, 2018 03:05
@dedemorton dedemorton changed the title [Work in progrss] Versioned plugin doc fixes [Work in progress] Versioned plugin doc fixes Jan 13, 2018
///////////////////////////////////////////

[id="plugins-{type}-{plugin}"]
[id="{version}-plugins-{type}s-{plugin}"]
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin.

:release_date: 2017-05-10
:changelog_url: https://github.com/logstash-plugins/logstash-filter-grok/blob/v3.4.1/CHANGELOG.md
:include_path: ../../../logstash/docs/include
:include_path: ../../../../logstash/docs/include
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

The path is wrong here. This problem is caused by bad asciidoc in older versions of the plugin.

==== Grok Filter Configuration Options

This plugin supports the following configuration options plus the <<plugins-{type}s-common-options>> described later.
This plugin supports the following configuration options plus the <<{version}-plugins-{type}s-{plugin}-common-options>> described later.
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin.

We can now test that the ruby script we're using is implemented correctly:

[source]
[source,shell]
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin.

A key/value hash with parameters that are passed to the register method
of your ruby script file defined in `path`.

[id="{version}-plugins-{type}s-{plugin}-tag_on_exception"]
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin.

----

====== Testing the ruby script
===== Testing the ruby script
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin.


The code to execute for every event.
You will have an `event` variable available that is the event itself. See the <<event-api,Event API>> for more information.
You will have an `event` variable available that is the event itself. See the {logstash-ref}/event-api.html[Event API] for more information.
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This is an external link, so we need to convert it to use the format for external links. We'll need to fix this in the asciidoc in the plugin repo.

= Versioned Plugin Reference

include::inputs.asciidoc[]
include::inputs-index.asciidoc[]
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

The filenames used in the index file are not correct.

NOTE: The Beats shipper automatically sets the `type` field on the event.
You cannot override this setting in the Logstash config. If you specify
a setting for the <<plugins-inputs-beats-type,`type`>> config option in
a setting for the <<{version}-plugins-inputs-beats-type,`type`>> config option in
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This is an internal link, so it needs to include {version}. We'll need to fix the script to add {version} to all internal links.

use the https://www.elastic.co/guide/en/beats/filebeat/current/multiline-examples.html[configuration options available in Filebeat] to handle multiline events
before sending the event data to Logstash. You cannot use the
<<plugins-codecs-multiline>> codec to handle multiline events. Doing so will
{logstash-ref}/plugins-codecs-multiline.html[Multiline codec plugin] codec to handle multiline events. Doing so will
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This is an external link, so we need to convert it to use the format for external links. We'll need to fix this in the asciidoc in the plugin repo.




[id="{version}-plugins-{type}s-{plugin}-common-options"]
Copy link
Contributor Author

@dedemorton dedemorton Jan 14, 2018

Choose a reason for hiding this comment

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

This problem is caused by bad asciidoc in older versions of the plugin where this line is missing.


==== Description

Sends email when an output is received. Alternatively, you may include or
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 description is missing from the older plugins. It must be added, or the build fails.

@jsvd
Copy link
Member

jsvd commented Jan 18, 2018

@dedemorton I have updated #444 with a few fixes for the things pointed out in this PR.

Things missing:

  • detecting missing description contents and id tags
  • converting external links

One question, I see the links <<{version}-plugins-{type}s-{plugin}-common-options>>

Should the common options link to a specific plugin (and specific plugin version)?

@dedemorton
Copy link
Contributor Author

dedemorton commented Jan 18, 2018

@jsvd Yes. The version info is required in the links because the section with common options gets pulled into every plugin file through include statements. If you don't include the version info, then the IDs for those sections will not be unique in each file, and the doc build will fail with errors.

You'll notice in my latest commit that I point to the include folder in the logstash repo rather than the one under logstash-docs/docs/versioned-plugins. I think we probably want one source of truth for the common files, so I updated the common files in the logstash repo so that they work with both the versioned plugin doc build and the Logstash Reference build: elastic/logstash#8937. But as I mention in my comments earlier, we need to decide how we will handle the common files over time. I want to maintain one source so we don't have duplicated content, but we might want to take the files, copy them to logstash-docs, and version them somehow (see the open issues I list above).

@jsvd
Copy link
Member

jsvd commented Jan 18, 2018

I'm pretty close to being able to generate the changes of this PR. here's the current diff:

diff --git a/docs/versioned-plugins/filters/ruby-v3.1.0.asciidoc b/docs/versioned-plugins/filters/ruby-v3.1.0.asciidoc
index 8b72ba7..dd6681a 100644
--- a/docs/versioned-plugins/filters/ruby-v3.1.0.asciidoc
+++ b/docs/versioned-plugins/filters/ruby-v3.1.0.asciidoc
@@ -188,14 +188,5 @@ The path of the ruby script file that implements the `filter` method.
 A key/value hash with parameters that are passed to the register method
 of your ruby script file defined in `path`.
 
-[id="{version}-plugins-{type}s-{plugin}-tag_on_exception"]
-===== `tag_on_exception`
-
-  * Value type is {logstash-ref}/configuration-file-structure.html#string[string]
-  * Default value is `_rubyexception`
-
-Tag to add to events in case the ruby code (either inline or file based)
-causes an exception.
-
 [id="{version}-plugins-{type}s-{plugin}-common-options"]
 include::{include_path}/{type}.asciidoc[]
diff --git a/docs/versioned-plugins/filters/ruby-v3.1.1.asciidoc b/docs/versioned-plugins/filters/ruby-v3.1.1.asciidoc
index 41d9699..6b7e44c 100644
--- a/docs/versioned-plugins/filters/ruby-v3.1.1.asciidoc
+++ b/docs/versioned-plugins/filters/ruby-v3.1.1.asciidoc
@@ -188,14 +188,5 @@ The path of the ruby script file that implements the `filter` method.
 A key/value hash with parameters that are passed to the register method
 of your ruby script file defined in `path`.
 
-[id="{version}-plugins-{type}s-{plugin}-tag_on_exception"]
-===== `tag_on_exception`
-
-  * Value type is {logstash-ref}/configuration-file-structure.html#string[string]
-  * Default value is `_rubyexception`
-
-Tag to add to events in case the ruby code (either inline or file based)
-causes an exception.
-
 [id="{version}-plugins-{type}s-{plugin}-common-options"]
 include::{include_path}/{type}.asciidoc[]
diff --git a/docs/versioned-plugins/filters/ruby-v3.1.2.asciidoc b/docs/versioned-plugins/filters/ruby-v3.1.2.asciidoc
index 375e5a4..12e9377 100644
--- a/docs/versioned-plugins/filters/ruby-v3.1.2.asciidoc
+++ b/docs/versioned-plugins/filters/ruby-v3.1.2.asciidoc
@@ -188,14 +188,5 @@ The path of the ruby script file that implements the `filter` method.
 A key/value hash with parameters that are passed to the register method
 of your ruby script file defined in `path`.
 
-[id="{version}-plugins-{type}s-{plugin}-tag_on_exception"]
-===== `tag_on_exception`
-
-  * Value type is {logstash-ref}/configuration-file-structure.html#string[string]
-  * Default value is `_rubyexception`
-
-Tag to add to events in case the ruby code (either inline or file based)
-causes an exception.
-
 [id="{version}-plugins-{type}s-{plugin}-common-options"]
 include::{include_path}/{type}.asciidoc[]
diff --git a/docs/versioned-plugins/outputs/email-v4.0.4.asciidoc b/docs/versioned-plugins/outputs/email-v4.0.4.asciidoc
index 2ac93cc..c9b5d9c 100644
--- a/docs/versioned-plugins/outputs/email-v4.0.4.asciidoc
+++ b/docs/versioned-plugins/outputs/email-v4.0.4.asciidoc
@@ -20,8 +20,6 @@ include::{include_path}/plugin_header.asciidoc[]
 
 ==== Description
 
-Sends email when an output is received. Alternatively, you may include or
-exclude the email output execution using conditionals.
 
 ==== Usage Example
 [source,ruby]

So I'm missing:

  • Filling in missing descriptions
  • Manual interventions that required adding content like missing documentation for a setting
  • The script still performs some hardcoded corrections such as replacing the internal multiline codec link with an external one.

As for the point you mentioned about fixing things only once I completely agree. This sample of 15 allowed me to encode common fixes that we'll see on the 200 plugins, so that if we generate the docs for the 200 plugins you won't have to fix the same thing a million times.
Once docs are generated, fixed and commited I already have a clause that skips a version of a plugin if the asciidoc file exists for it.

@dedemorton
Copy link
Contributor Author

Cool. The first two items in your list (missing descriptions and missing documentation) should be fixed in the latest doc source. We are likely to find other similar problems as we iterate, but it's exciting to see this project coming together!

@jsvd
Copy link
Member

jsvd commented Jan 18, 2018

@dedemorton I pushed this PR into a branch: https://github.com/elastic/logstash-docs/tree/versioned_plugin_docs

We can use this branch as the base branch to build the versioned docs. I created #449, which adds the docs of 14 more codecs on top of this.

@dedemorton
Copy link
Contributor Author

Awesome. I'll close this PR to avoid confusion in the future.

@dedemorton dedemorton closed this Jan 19, 2018
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.

2 participants