Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ingest: document fields that support templating #34536

Merged
merged 7 commits into from
Oct 23, 2018

Conversation

jakelandis
Copy link
Contributor

This change also updates many of the examples to use ecs as the example.
Some additional minor improvements are also included.

Part of #33188

This change also updates many of the examples to use ecs as the example.
Some additional minor improvements are also included.

Part of elastic#33188
@@ -751,23 +751,23 @@ pipeline basis. Useful to find out which pipelines are used the most or spent th
Appends one or more values to an existing array if the field already exists and it is an array.
Converts a scalar to an array and appends one or more values to it if the field exists and it is a scalar.
Creates an array containing the provided values if the field doesn't exist.
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -846,7 +846,8 @@ Parses dates from fields, and then uses the date or timestamp as the timestamp f
By default, the date processor adds the parsed date as a new field called `@timestamp`. You can specify a
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -1041,12 +1042,12 @@ understands this to mean `2016-04-01` as is explained in the <<date-math-index-n
|======
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -1365,21 +1366,22 @@ a scalar field to an object field.
=== Fail Processor
Raises an exception. This is useful for when
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -2077,14 +2079,14 @@ Converts a string to its lowercase equivalent.

[[remove-processor]]
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -2115,23 +2117,24 @@ To remove multiple fields, you can use the following query:
[[rename-processor]]
=== Rename Processor
Renames an existing field. If the field doesn't exist or the new name is already used, an exception will be thrown.
Settings that support <<accessing-template-fields,template values>> are annotated with ^T^.

[[rename-options]]
.Rename Options
[options="header"]
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

image

@@ -2243,24 +2246,24 @@ and depending on the progress made, indexed into different indices.
[[set-processor]]
=== Set Processor
Copy link
Contributor Author

@jakelandis jakelandis Oct 16, 2018

Choose a reason for hiding this comment

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

(REJECTED)
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@tomcallahan @debadair ^^ example using description instead of superscript. Thoughts on which you prefer seeing side by side ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this the best of the available options

@jakelandis jakelandis added >docs General docs changes :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v7.0.0 v6.5.0 labels Oct 16, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jakelandis
Copy link
Contributor Author

@debadair - could I get your review on this documentation change ? I am abit uncertain about the superscript approach, however after playing around with a few options it came out as less busy/crowded as other options I tried. I have included screen shots of the rendered output to hopefully make the review quick.

Also, I updated many of the examples to be based from https://github.com/elastic/ecs where applicable.

@tomcallahan
Copy link
Contributor

tomcallahan commented Oct 18, 2018

So you're saying it didn't look good with "templating supported" as another column in the table? I am not a huge fan the superscript.

It also appears that field and target_field are templatable in all cases exception one -- can you verify that is correct?

@jakelandis
Copy link
Contributor Author

jakelandis commented Oct 18, 2018

So you're saying it didn't look good with "templating supported" as another column in the table? I am not a huge fan the superscript.

It's a judgement call for sure ... here is an example of the set processor rendered with the extra column:
(REJECTED)
image

It also appears that field and target_field are templatable in all cases exception one -- can you verify that is correct?

Of the processors changed, Date and Date Index Name do not support templates for fieldor target_field. There are a couple processors not part of the changes here that also do not support templates for field or target_field

Copy link
Contributor

@debadair debadair left a comment

Choose a reason for hiding this comment

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

I probably would have added "Supports template values." to the descriptions, rather than using the "T" annotation, but the annotation works, too. I would change "Settings" to "Options" to be consistent with the option tables, though.

@jakelandis
Copy link
Contributor Author

@debadair @tomcallahan - Thanks for the feedback! I have removed the superscript in favor of a comment in the description. (and updated the screen shots)

Copy link
Contributor

@tomcallahan tomcallahan left a comment

Choose a reason for hiding this comment

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

In this doc, we use the term template values, however when I click through we refer to these as template snippets, is there a reason we're not consistent here?

@jakelandis
Copy link
Contributor Author

@tomcallahan - An oversight on my part, I will update the wording to be consistent.

Copy link
Contributor

@tomcallahan tomcallahan left a comment

Choose a reason for hiding this comment

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

LGTM

@jakelandis jakelandis merged commit a8e1ee3 into elastic:master Oct 23, 2018
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Oct 23, 2018
This change also updates many of the examples to use ecs as the example.
Some additional minor improvements are also included.

Part of elastic#33188
jakelandis added a commit that referenced this pull request Oct 24, 2018
This change also updates many of the examples to use ecs as the example.
Some additional minor improvements are also included.

Part of #33188
@jakelandis
Copy link
Contributor Author

6.5 backport: e9a380c

kcm pushed a commit that referenced this pull request Oct 30, 2018
This change also updates many of the examples to use ecs as the example.
Some additional minor improvements are also included.

Part of #33188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >docs General docs changes v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants