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

More generic docs for use in APM Server #6184

Merged
merged 24 commits into from
Feb 22, 2018

Conversation

roncohen
Copy link
Contributor

  • Use specific beat name name instead of "the Beat"
  • Make Logstash and other outputs optional for the beat
  • Decouple the index pattern from the beat name

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

One minor typo; otherwise, LGTM with one caveat: I only tested that the docs build without errors. I didn't verify that the new conditional sections render as expected.

@@ -20,7 +20,7 @@ To load the dashboards, you can either enable dashboard loading in the
run the `setup` command. Dashboard loading is disabled by default.

When dashboard loading is enabled, {beatname_uc} uses the Kibana API to load the
sample dashboards. Dashboard loading is only attempted at Beat startup.
sample dashboards. Dashboard loading is only attempted when {beatname_uc} startups up.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: startups up

@ruflin
Copy link
Member

ruflin commented Jan 27, 2018

@roncohen Seeing you pushed some more change, I'm curious if it's ready for merge? As Dede is off this week, please ping me if yes.

@roncohen
Copy link
Contributor Author

there's more incoming @ruflin

@roncohen
Copy link
Contributor Author

quite a few things changed so this probably needs another round from @dedemorton

@@ -63,11 +63,11 @@ errors, there will be no log file in the directory specified for logs.
[[level]]
==== `logging.level`

Minimum log level. One of `debug`, `info`, `warning`, or `error`. The default
Minimum log level. One of `debug`, `info`, `warn`, or `error`. The default
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to change this back to warning, as we went for making warning work again in #6240

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. Thanks for the reminder.

@graphaelli
Copy link
Member

graphaelli commented Feb 14, 2018

@tsg @dedemorton can one or both of you sign off on this one?

@dedemorton
Copy link
Contributor

@graphaelli Oh sorry. I was on vacation when the notification came through and didn't see the notification from GitHub. I'll take a look sometime today.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

Mostly minor stuff.

:setup-command-short-desc: Sets up the initial environment, including the index template, Kibana dashboards (when available), and machine learning jobs (when available)
else::[]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think asciidoc supports this style of conditional coding. When I ran with has_ml_jobs: no, the description was blank.

You might need to wrap the text in its own statement:

ifeval::["{has_ml_jobs}"=="yes"]
:setup-command-short-desc: Sets up the initial environment, including the index template, Kibana dashboards (when available), and machine learning jobs (when available)
endif::[]
ifeval::["{has_ml_jobs}"!="yes"]
:setup-command-short-desc: Sets up the initial environment, including the index template, and Kibana dashboards (when available)
endif::[]

@@ -13,6 +13,12 @@
[[configuring-output]]
== Configure the output

ifdef::only-elasticsearch[]
You configure {beatname_uc} to write to Elasticsearch by setting options in
the `output.elasticsearch` of the +{beatname_lc}.yml+ config file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say either "in the output.elasticsearch section of the +{beatname_lc}.yml+ config file" or "under output.elasticsearch in the +{beatname_lc}.yml+ config file."

@@ -1,5 +1,5 @@
[[configuration-ssl]]
== Specify SSL settings
== SSL settings for outputs
Copy link
Contributor

Choose a reason for hiding this comment

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

The SSL settings are relevant for other stuff (like kibana), too, so I wouldn't change the title here.

You can adjust the following settings to load your own template or overwrite an
existing one.

*`setup.template.enabled`*:: Set to false to disable template loading. If set this to false,
you must <<load-template-manually,load the template manually>>.

*`setup.template.name`*:: The name of the template. The default is
+{beatname_lc}+. The Beat version is always appended to the given
+{beatname_lc}-*+. The {beatname_uc} version is always appended to the given
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this asterisk really belong here?

@roncohen
Copy link
Contributor Author

I've addressed your comments in 5a4de2b. Let me know if there's anything else

@roncohen
Copy link
Contributor Author

@dedemorton OK to merge?

@dedemorton
Copy link
Contributor

@roncohen Yes (as long as the CI failures look OK to you...TBH, I'm never quite sure which CI failures I can ignore).

Do we need to backport these changes to 6.2? Or are they for 6.3 and later?

@roncohen roncohen dismissed dedemorton’s stale review February 22, 2018 12:53

Fixed according to comments.

@roncohen roncohen merged commit 1f0189d into elastic:master Feb 22, 2018
jalvz added a commit to jalvz/apm-server that referenced this pull request Feb 26, 2018
jalvz added a commit to elastic/apm-server that referenced this pull request Mar 8, 2018
simitt pushed a commit to simitt/apm-server that referenced this pull request Mar 22, 2018
simitt pushed a commit to elastic/apm-server that referenced this pull request Mar 22, 2018
simitt pushed a commit to simitt/beats that referenced this pull request Apr 17, 2018
* Use specific beat name's instead of 'the Beat'
* Dont show Logstash info unless it's supported.
* Make the index pattern decoupled from the beat name
* Make it possible to skip the pipeline docs in output.elasticsearch
* Introduce beat_default_index_prefix.
* Use name of beat in shared-kibana-config.asciidoc
* Add .\ for PS instruction and make it possible to remove logstash mention.
* Introduce 'has_ml_jobs'
* Added `html_docs` to .gitignore
* Better outputconfig for Elasticsearch only beats.
* Less blamy wording
* Only talk about Filebeat for filebeat docs
* Update the rest of the max_retries sections to only talk about Filebeat for filebeat
* Special case for apm-server as it was only introduced in 6.0
* expand beat_default_index_prefix use (elastic#2)
ruflin pushed a commit that referenced this pull request Apr 17, 2018
* Use specific beat name's instead of 'the Beat'
* Dont show Logstash info unless it's supported.
* Make the index pattern decoupled from the beat name
* Make it possible to skip the pipeline docs in output.elasticsearch
* Introduce beat_default_index_prefix.
* Use name of beat in shared-kibana-config.asciidoc
* Add .\ for PS instruction and make it possible to remove logstash mention.
* Introduce 'has_ml_jobs'
* Added `html_docs` to .gitignore
* Better outputconfig for Elasticsearch only beats.
* Less blamy wording
* Only talk about Filebeat for filebeat docs
* Update the rest of the max_retries sections to only talk about Filebeat for filebeat
* Special case for apm-server as it was only introduced in 6.0
* expand beat_default_index_prefix use (#2)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Use specific beat name's instead of 'the Beat'
* Dont show Logstash info unless it's supported.
* Make the index pattern decoupled from the beat name
* Make it possible to skip the pipeline docs in output.elasticsearch
* Introduce beat_default_index_prefix.
* Use name of beat in shared-kibana-config.asciidoc
* Add .\ for PS instruction and make it possible to remove logstash mention.
* Introduce 'has_ml_jobs'
* Added `html_docs` to .gitignore
* Better outputconfig for Elasticsearch only beats.
* Less blamy wording
* Only talk about Filebeat for filebeat docs
* Update the rest of the max_retries sections to only talk about Filebeat for filebeat
* Special case for apm-server as it was only introduced in 6.0
* expand beat_default_index_prefix use (elastic#2)
bmorelli25 pushed a commit to bmorelli25/observability-docs that referenced this pull request Dec 18, 2023
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.

5 participants