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

[health] bootstrap HealthObserver from agent to API #16141

Merged

Conversation

yaauie
Copy link
Member

@yaauie yaauie commented May 3, 2024

Release notes

[rn:skip]

What does this PR do?

  • Introduces org.logstash.health.HealthObserver that is accessible via Agent#health_observer
  • Introduces org.logstash.health.Status with facilities for combining statuses
  • Populates API-common top-level status with HealthObserver#status. In bootstrap form, the value is controlled by system property logstash.apiStatus, but will be derived from indicators once they are added in a subsequent PR.

Why is it important/What is the impact to the user?

These are steps toward delivering the health report API as RFC'd in #16056

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

To prove that the value of API response's status is no longer hard-coded to green,
this bootstrap contains a hidden feature flag java property logstash.apiStatus which can be any of unknown, green, yellow, red, or random (it defaults to green).

Start Logstash running a long-lived pipeline:

LS_JAVA_OPTS="-Dlogstash.apiStatus=random" bin/logstash -e 'input { heartbeat {} }'

Get the API response for GET /_node and/or GET /_node_stats and observe that the status is no longer hard-coded to green:

╭─{ rye@perhaps:~/src/elastic/logstash@main (health-api-bootstrap ✔) }
╰─● curl --silent -XGET http://localhost:9600/_node | jq .status
"yellow"
[success]                                                                                                                                                                                                                                                                                                                                                                                                                        

╭─{ rye@perhaps:~/src/elastic/logstash@main (health-api-bootstrap ✔) }
╰─● curl --silent -XGET http://localhost:9600/_node | jq .status
"yellow"
[success]                                                                                                                                                                                                                                                                                                                                                                                                                        

╭─{ rye@perhaps:~/src/elastic/logstash@main (health-api-bootstrap ✔) }
╰─● curl --silent -XGET http://localhost:9600/_node | jq .status
"red"
[success]                                                                                                                                                                                                                                                                                                                                                                                                                        

╭─{ rye@perhaps:~/src/elastic/logstash@main (health-api-bootstrap ✔) }
╰─● curl --silent -XGET http://localhost:9600/_node | jq .status
"green"
[success]                                                                                                                                                                                                                                                                                                                                                                                                                        

╭─{ rye@perhaps:~/src/elastic/logstash@main (health-api-bootstrap ✔) }
╰─● curl --silent -XGET http://localhost:9600/_node | jq .status
"unknown"
[success] 

Related issues

  • Closes elastic/ingest-dev#3241
  • Closes elastic/ingest-dev#3242

Copy link
Contributor

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

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

Thank you for making this happen. Finally, always-false-green will be really Logstash status. I have added minor suggestion and asked a question to clarify but changes are reasonable.
Can you also please take a look CI failure? It doesn't seem to me relative but not sure why failing.

@@ -28,7 +28,7 @@ def all
:id => service.agent.id,
:name => service.agent.name,
:ephemeral_id => service.agent.ephemeral_id,
:status => "green", # This is hard-coded to mirror x-pack behavior
:status => service.agent.health_observer.status,
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 finally...

@@ -0,0 +1,21 @@
package org.logstash.health;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not to forget adding Elastic License headers.

Suggested change
package org.logstash.health;
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.logstash.health;

* @param status the other status
* @return the more-degraded of the two statuses.
*/
public Status reduce(Status status) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't get that why we need reduce here?!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's part of future work, and slightly out of scope but a required part of getting this endpoint bootstrapped.

Once we have many indicator reports, we will need to reduce their individual statuses into a summary status. Similar with an indicator report from many probes, each of which can degrade status.

Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Contributor

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

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

lgtm.

@yaauie yaauie merged commit 1f5ba9c into elastic:feature/health-report-api May 8, 2024
6 checks passed
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.

None yet

4 participants