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

Create NGINX onboarding tutorial #3826

Merged
merged 6 commits into from
May 3, 2024

Conversation

mdbirnstiehl
Copy link
Contributor

@mdbirnstiehl mdbirnstiehl commented Apr 23, 2024

This PR closes #3411.

Creates an onboarding tutorial for NGINX using the integration. Includes information on exploring and visualizing data using the built-in dashboard.

Preview available here

@mdbirnstiehl mdbirnstiehl self-assigned this Apr 23, 2024
@mdbirnstiehl mdbirnstiehl added backport-8.13 Automated backport with mergify backport-8.14 Automated backport with mergify labels Apr 23, 2024
Copy link
Contributor

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

@mdbirnstiehl mdbirnstiehl requested a review from muthu-mps April 23, 2024 21:12
@muthu-mps
Copy link

Please find my observations,

1. Nginx naming

The Nginx docs page has the naming convention as NGINX. Our integration docs page represents with the naming convention as Nginx. This makes different representation's in the docs page. see below,

From Integration page:

Nginx-naming

From the docs page:

Nginx-Naming-1
  • Can we make the naming of Nginx consistent in both the places. I would suggest to update the docs to replace NGINX with Nginx.

2. Alignment

  • Please fix the alignment issues in the logs to make it similar to metrics.
Alignment

3. Suggestions on Add Agent section

  • When looking at the Fleet enrolment section. I was noticing different representations in other pages. Can we consolidate the Add Agent as a separate section and reference from the documentation page here. This would also help keeping the length of the documentation short.

Azure

Agent-Instructions-1

Kubernetes

Agent-Instructions-2

Nginx

Agent-Instructions-3

@mdbirnstiehl mdbirnstiehl marked this pull request as ready for review April 26, 2024 15:25
@mdbirnstiehl mdbirnstiehl requested a review from a team as a code owner April 26, 2024 15:25
bmorelli25
bmorelli25 previously approved these changes May 1, 2024
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.

Looks good. Just a few thoughts.

docs/en/observability/monitor-nginx.asciidoc Outdated Show resolved Hide resolved
docs/en/observability/monitor-nginx.asciidoc Outdated Show resolved Hide resolved
docs/en/observability/monitor-nginx.asciidoc Show resolved Hide resolved
docs/en/observability/monitor-nginx.asciidoc Outdated Show resolved Hide resolved
bmorelli25
bmorelli25 previously approved these changes May 1, 2024
@mdbirnstiehl
Copy link
Contributor Author

Please find my observations,

1. Nginx naming

The Nginx docs page has the naming convention as NGINX. Our integration docs page represents with the naming convention as Nginx. This makes different representation's in the docs page. see below,

NGINX mostly stylizes their name in all caps, but they have trademarked both NGINX and nginx. We should really be using one of those in our integration, but I've now switched instances of NGINX to nginx except when referencing a feature or section of the integration. There I've used Nginx.

2. Alignment

  • Please fix the alignment issues in the logs to make it similar to metrics.

Unfortunately, I think this is a bug in our styles due to the horizontal list having more than one line.

3. Suggestions on Add Agent section

  • When looking at the Fleet enrolment section. I was noticing different representations in other pages. Can we consolidate the Add Agent as a separate section and reference from the documentation page here. This would also help keeping the length of the documentation short.

Thanks for pointing this out. I think there may be a case for some reuse between these docs. There are some slight differences (mostly in the K8s integration) that may make things a little unclear for users. For now, I would like to keep these instructions here to keep users in the flow, but maintaining all of these separately may become too big of a task so I will look into reuse here and updating the necessary pages.

muthu-mps
muthu-mps previously approved these changes May 3, 2024
alaudazzi
alaudazzi previously approved these changes May 3, 2024
Copy link
Contributor

@alaudazzi alaudazzi left a comment

Choose a reason for hiding this comment

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

Left a few editing suggestions, otherwise LGTM.

docs/en/observability/monitor-nginx.asciidoc Outdated Show resolved Hide resolved
docs/en/observability/monitor-nginx.asciidoc Outdated Show resolved Hide resolved
@mdbirnstiehl mdbirnstiehl dismissed stale reviews from alaudazzi, muthu-mps, and bmorelli25 via fa3ffd8 May 3, 2024 13:59
Copy link
Contributor

@alaudazzi alaudazzi left a comment

Choose a reason for hiding this comment

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

🚀

@mdbirnstiehl mdbirnstiehl merged commit 7697da2 into elastic:main May 3, 2024
3 checks passed
@mdbirnstiehl mdbirnstiehl deleted the monitor-nginx branch May 3, 2024 14:35
mergify bot pushed a commit that referenced this pull request May 3, 2024
mergify bot pushed a commit that referenced this pull request May 3, 2024
mdbirnstiehl added a commit that referenced this pull request May 3, 2024
(cherry picked from commit 7697da2)

Co-authored-by: Mike Birnstiehl <[email protected]>
mdbirnstiehl added a commit that referenced this pull request May 3, 2024
(cherry picked from commit 7697da2)

Co-authored-by: Mike Birnstiehl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.13 Automated backport with mergify backport-8.14 Automated backport with mergify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Onboard NGINX using integrations (part one)
4 participants