Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions .buildkite/scripts/health-report-tests/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def __resolve_latest_stack_version_for(self, major_version: str) -> str:
return release_version

def install_plugin(self, plugin_path: str) -> None:
print("Installing logstash-integration-failure_injector plugin")
Copy link
Member

Choose a reason for hiding this comment

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

Is this a generalized method? Does this print statement belong here specific to one plugin?

Copy link
Contributor Author

@mashhurs mashhurs Aug 15, 2025

Choose a reason for hiding this comment

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

specific to one plugin?

Yup! The logstash-integration-failure_injector isn't embedded in the core by default and we install while health-report integration tests to simulate various scenarios (failure, backpressure, etc...). This log statement is very useful because a) plugin-manager (logstash-plugin {operation}) silently processes its operations. If any failure we get while installing the plugin w/o this log statement that b) might be confusing if it came from LS build.

util.run_or_raise_error(
["bin/logstash-plugin", "install", plugin_path],
f"Failed to install {plugin_path}")
Expand All @@ -71,27 +72,23 @@ def run_logstash(self, full_start_required: bool) -> subprocess.Popen:
# --config.reload.automatic is to make instance active
# it is helpful when testing crash pipeline cases
config_path = os.getcwd() + "/.buildkite/scripts/health-report-tests/config"
process = subprocess.Popen(["bin/logstash", "--config.reload.automatic", "--path.settings", config_path,
"-w 1"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, shell=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

review note: in pipeline.yaml config files (e.g here), we set worker count, this is not necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Confirmed, this flag is not necessary and arguably was not appropriate for a generalized run_logstash method anyway. I like removal 👍

process = subprocess.Popen(["bin/logstash", "--config.reload.automatic", "--path.settings", config_path],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, shell=False)
if process.poll() is not None:
print(f"Logstash failed to run, check the the config and logs, then rerun.")
return None

print(f"Logstash started running with PID: {process.pid}.")

# Read stdout and stderr in real-time
logs = []
for stdout_line in iter(process.stdout.readline, ""):
logs.append(stdout_line.strip())
print(stdout_line.strip())
# we don't wait for Logstash fully start as we also test slow pipeline start scenarios
if full_start_required is False and "Starting pipeline" in stdout_line:
break
if full_start_required is True and "Pipeline started" in stdout_line:
if "Pipeline started" in stdout_line:
break
if "Logstash shut down" in stdout_line or "Logstash stopped" in stdout_line:
print(f"Logstash couldn't spin up.")
print(logs)
return None

print(f"Logstash is running with PID: {process.pid}.")
return process

def stop_logstash(self, process: subprocess.Popen):
Expand All @@ -102,7 +99,7 @@ def stop_logstash(self, process: subprocess.Popen):
if "Logstash shut down" in stdout_line or "Logstash stopped" in stdout_line:
print(f"Logstash stopped.")
return None
# shudown watcher keep running, we should be good with considering time spent
# shutdown watcher keep running, we should be good with considering time spent
if time.time() - start_time > 60:
print(f"Logstash didn't stop in 1min, sending SIGTERM signal.")
process.kill()
Expand Down
2 changes: 1 addition & 1 deletion .buildkite/scripts/health-report-tests/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def __enter__(self):
raise ValueError(f"Could not find logstash-integration-failure_injector plugin.")

self.bootstrap.install_plugin(matching_files[0])
print(f"logstash-integration-failure_injector successfully installed.")
print("logstash-integration-failure_injector successfully installed.")
return self.bootstrap

def __exit__(self, exc_type, exc_value, exc_traceback):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,4 @@ def on(self, scenario_name: str, expectations: dict) -> None:
if attempts == 0:
raise Exception(f"{scenario_name} failed.")
else:
print(f"Scenario `{scenario_name}` expectaion meets the health report stats.")
print(f"Scenario `{scenario_name}` expectation meets the health report stats.")
5 changes: 3 additions & 2 deletions .buildkite/scripts/health-report-tests/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ def git_check_out_branch(branch_name: str) -> None:

def run_or_raise_error(commands: list, error_message):
f"""
Executes the {list} commands and raises an {Exception} if opration fails.
Executes the {list} commands and raises an {Exception} if operation fails.
"""
result = subprocess.run(commands, env=os.environ.copy(), universal_newlines=True, stdout=subprocess.PIPE)
if result.returncode != 0:
full_error_message = (error_message + ", output: " + result.stdout.decode('utf-8')) \
full_error_message = (error_message + ", output: " + result.stdout) \
if result.stdout else error_message
raise Exception(f"{full_error_message}")
print(result.stdout)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

review note: regardless of failure status, prints log for better visibility.

Copy link
Member

Choose a reason for hiding this comment

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

Nice. Looks like the majority of the logging that is beneficial will happen at https://github.com/elastic/logstash/pull/17991/files/acc787b0aeac14acc37704857dccddd75661990f#diff-cabfb7b7a18cf3ea856fc69cec74a74345761db2625d41b10ceb7ec13c447252R85 but this is good too i think.