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

Failing bootstrap checks should link to reference docs #99614

Closed
DaveCTurner opened this issue Sep 17, 2023 · 9 comments · Fixed by #99644
Closed

Failing bootstrap checks should link to reference docs #99614

DaveCTurner opened this issue Sep 17, 2023 · 9 comments · Fixed by #99644
Labels
:Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown >enhancement good first issue low hanging fruit Supportability Improve our (devs, SREs, support eng, users) ability to troubleshoot/self-service product better. Team:Core/Infra Meta label for core/infra team

Comments

@DaveCTurner
Copy link
Contributor

In #92755 we added a mechanism for safely emitting links to pages of the reference manual to help users understand the context of a particular system condition or error message. I've observed that new users often struggle to understand failing bootstrap checks, and every bootstrap check has its own page in the manual, so it would be very helpful to use the ReferenceDocs mechanism to add a docs link to the log message about each failing bootstrap check.

@DaveCTurner DaveCTurner added >enhancement good first issue low hanging fruit :Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown labels Sep 17, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Sep 17, 2023
@DaveCTurner
Copy link
Contributor Author

Fundamentally the heart of this change would look something like this:

diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java
index 75e5fcfa3fa0..ae4b2bde28cc 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java
@@ -59,4 +59,5 @@ public interface BootstrapCheck {
         return false;
     }

+    ReferenceDocs referenceDocs();
 }
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
index b9610c689f92..d4e687c11d3b 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
@@ -131,10 +131,11 @@ final class BootstrapChecks {
         for (final BootstrapCheck check : checks) {
             final BootstrapCheck.BootstrapCheckResult result = check.check(context);
             if (result.isFailure()) {
+                final var message = result.getMessage() + "; for more information see [" + check.referenceDocs() + "]";
                 if (enforceLimits == false && enforceBootstrapChecks == false && check.alwaysEnforce() == false) {
-                    ignoredErrors.add(result.getMessage());
+                    ignoredErrors.add(message);
                 } else {
-                    errors.add(result.getMessage());
+                    errors.add(message);
                 }
             }
         }
@@ -150,7 +151,9 @@ final class BootstrapChecks {
                     + errors.size()
                     + "] bootstrap checks failed. You must address the points described in the following ["
                     + errors.size()
-                    + "] lines before starting Elasticsearch."
+                    + "] lines before starting Elasticsearch. For more information see ["
+                    + ReferenceDocs.BOOTSTRAP_CHECKS
+                    + "]"
             );
             for (int i = 0; i < errors.size(); i++) {
                 messages.add("bootstrap check failure [" + (i + 1) + "] of [" + errors.size() + "]: " + errors.get(i));

Then it should be just a case of adding the links to reference-docs-links.json and ReferenceDocs.java, fixing the compile errors, and adding some relevant tests.

@V5t5d
Copy link

V5t5d commented Sep 17, 2023

Hey everyone, I'm new to open source but have expertise with Java development.
I'm willing to start contributing, therefore it would be awesome if someone could serve as my mentor.

@DaveCTurner
Copy link
Contributor Author

Hi @V5t5d, thanks, we don't really have a formal mentorship program, but we'll review your contributions such as a PR that resolves this issue. If you follow the contributing guide and start from the suggestion in my previous comment then you hopefully won't need too much further guidance.

@DaveCTurner DaveCTurner added the Supportability Improve our (devs, SREs, support eng, users) ability to troubleshoot/self-service product better. label Sep 18, 2023
@heissanjay
Copy link
Contributor

Hi there, I'm looking to tackle this issue as my inaugural open-source contribution. I'm eager to learn and would greatly appreciate any guidance along the way.

@DaveCTurner
Copy link
Contributor Author

Go for it @heissanjay; all the guidance you need is in my previous comments.

@heissanjay
Copy link
Contributor

Hi @DaveCTurner based on your previous comments, it appears that the primary change in BootstrapChecks.java involves modifying error messages by adding a new message format that includes references to documentation. However, I wanted to confirm if there are any other changes or modifications required in BootstrapChecks.java beyond what has been mentioned.

@DaveCTurner
Copy link
Contributor Author

Nothing springs to mind, let's see how the PR looks and we can iterate there.

@NEUpanning
Copy link
Contributor

@DaveCTurner I've opened a pull request for this issue. Could you please have a look when you have some time? Thanks

elasticsearchmachine pushed a commit that referenced this issue Sep 21, 2023
…#99644)

Add a docs link to the log message about each failing bootstrap check to
help new users to understand failing bootstrap checks.

Closes #99614
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown >enhancement good first issue low hanging fruit Supportability Improve our (devs, SREs, support eng, users) ability to troubleshoot/self-service product better. Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants