From 36d1e17db7c169f9c54c574c6aff77164bce2a94 Mon Sep 17 00:00:00 2001 From: Noemi Lapresta Date: Thu, 1 Oct 2026 17:25:42 +0200 Subject: [PATCH] Ask for a collector in the installer After the push API key, `appsignal install` asks for a collector endpoint and links to the page where a hosted collector is created. Logging and distributed tracing need a collector, so an empty answer asks for confirmation before going on without one. With an endpoint, it also asks for a service name, and writes both to `__appsignal__.py`. --- .../ask-for-a-collector-in-the-installer.md | 6 ++ src/appsignal/cli/install.py | 75 +++++++++++--- tests/cli/test_demo.py | 13 ++- tests/cli/test_diagnose.py | 8 +- tests/cli/test_install.py | 97 ++++++++++++++++--- 5 files changed, 164 insertions(+), 35 deletions(-) create mode 100644 .changesets/ask-for-a-collector-in-the-installer.md diff --git a/.changesets/ask-for-a-collector-in-the-installer.md b/.changesets/ask-for-a-collector-in-the-installer.md new file mode 100644 index 00000000..68d92cf1 --- /dev/null +++ b/.changesets/ask-for-a-collector-in-the-installer.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: add +--- + +The `appsignal install` command asks for a collector endpoint and a service name, and adds them to the `__appsignal__.py` file it writes. diff --git a/src/appsignal/cli/install.py b/src/appsignal/cli/install.py index 56ca219e..4e88d5c1 100644 --- a/src/appsignal/cli/install.py +++ b/src/appsignal/cli/install.py @@ -13,19 +13,38 @@ appsignal = Appsignal( active=True, - name="{name}", - # Please do not commit this key to your source control management system. - # Move this to your app's security credentials or environment variables. - # https://docs.appsignal.com/python/configuration/options.html#option-push_api_key - push_api_key="{push_api_key}", -) +{options}) """ +INSTALL_FILE_OPTION_COMMENTS = { + "push_api_key": [ + "Please do not commit this key to your source control management system.", + "Move this to your app's security credentials or environment variables.", + "https://docs.appsignal.com/python/configuration/options.html" + "#option-push_api_key", + ], +} + +COLLECTOR_URL = ( + "https://appsignal.com/redirect-to/organization?to=admin/hosted_collectors" +) + INSTALL_FILE_NAME = "__appsignal__.py" WARNING_EMOJI = "\u26A0\ufe0f" +def install_file_contents(options: Options) -> str: + lines: list[str] = [] + for key, value in options.items(): + lines.extend( + f" # {comment}\n" + for comment in INSTALL_FILE_OPTION_COMMENTS.get(key, []) + ) + lines.append(f' {key}="{value}",\n') + return INSTALL_FILE_TEMPLATE.format(options="".join(lines)) + + class InstallCommand(AppsignalCLICommand): """Generate Appsignal client integration code.""" @@ -48,6 +67,12 @@ def run(self) -> int: print() + collector_endpoint = self._collector_endpoint() + if collector_endpoint: + options["collector_endpoint"] = collector_endpoint + options["service_name"] = self._service_name() + print() + if self._should_write_file(): print(f"Writing the {INSTALL_FILE_NAME} configuration file...") self._write_file(options) @@ -73,6 +98,37 @@ def run(self) -> int: return 0 + def _collector_endpoint(self) -> str | None: + while True: + endpoint = input( + f"Please enter your collector endpoint (create one at {COLLECTOR_URL}): " + ) + if endpoint: + return endpoint + if self._input_should_continue_without_collector(): + return None + + def _input_should_continue_without_collector(self) -> bool: + response = input( + "Are you sure? Without a collector, logging and distributed tracing" + " won't work. Continue without one? (y/N): " + ) + if len(response) == 0 or response[0].lower() == "n": + return False + if response[0].lower() == "y": + return True + print('Please answer "y" (yes) or "n" (no)') + return self._input_should_continue_without_collector() + + def _service_name(self) -> str: + name = "" + while not name: + name = input( + "Please enter the name of this service" + " (such as web-server or background-worker): " + ) + return name + def _should_write_file(self) -> bool: if os.path.exists(INSTALL_FILE_NAME): return self._input_should_overwrite_file() @@ -92,12 +148,7 @@ def _input_should_overwrite_file(self) -> bool: def _write_file(self, options: Options) -> None: with open(INSTALL_FILE_NAME, "w") as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name=options["name"], - push_api_key=options["push_api_key"], - ) - - f.write(file_contents) + f.write(install_file_contents(options)) def _requirements_file(self) -> str | None: current_dir = os.getcwd() diff --git a/tests/cli/test_demo.py b/tests/cli/test_demo.py index 1f234b06..d6fa9561 100644 --- a/tests/cli/test_demo.py +++ b/tests/cli/test_demo.py @@ -4,7 +4,8 @@ import shutil from appsignal.cli.base import main -from appsignal.cli.install import INSTALL_FILE_TEMPLATE +from appsignal.cli.install import install_file_contents +from appsignal.config import Options from .utils import mock_input @@ -67,9 +68,8 @@ def test_demo_with_config_file(request, mocker, capfd): os.chdir(test_dir) # Add client file with open(os.path.join(test_dir, "__appsignal__.py"), "w") as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name="My app name", - push_api_key="000", + file_contents = install_file_contents( + Options(name="My app name", push_api_key="000") ) f.write(file_contents) @@ -92,9 +92,8 @@ def test_demo_with_config_file_and_cli_options(request, mocker, capfd): os.chdir(test_dir) # Add client file with open(os.path.join(test_dir, "__appsignal__.py"), "w") as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name="My app name", - push_api_key="000", + file_contents = install_file_contents( + Options(name="My app name", push_api_key="000") ) f.write(file_contents) diff --git a/tests/cli/test_diagnose.py b/tests/cli/test_diagnose.py index 9903a40a..c49c659c 100644 --- a/tests/cli/test_diagnose.py +++ b/tests/cli/test_diagnose.py @@ -4,7 +4,8 @@ import shutil from appsignal.cli.base import main -from appsignal.cli.install import INSTALL_FILE_TEMPLATE +from appsignal.cli.install import install_file_contents +from appsignal.config import Options from .utils import mock_input @@ -65,9 +66,8 @@ def test_diagnose_with_config_file(request, mocker, capfd): os.chdir(test_dir) # Add client file with open(os.path.join(test_dir, "__appsignal__.py"), "w") as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name="My app name", - push_api_key="000", + file_contents = install_file_contents( + Options(name="My app name", push_api_key="000") ) f.write(file_contents) diff --git a/tests/cli/test_install.py b/tests/cli/test_install.py index b4d1a946..95a7cec1 100644 --- a/tests/cli/test_install.py +++ b/tests/cli/test_install.py @@ -5,7 +5,8 @@ from unittest.mock import MagicMock from appsignal.cli.base import main -from appsignal.cli.install import INSTALL_FILE_TEMPLATE +from appsignal.cli.install import install_file_contents +from appsignal.config import Options from .utils import mock_input @@ -22,21 +23,52 @@ ) """ +EXPECTED_COLLECTOR_FILE_CONTENTS = """from appsignal import Appsignal + +appsignal = Appsignal( + active=True, + name="My app name", + # Please do not commit this key to your source control management system. + # Move this to your app's security credentials or environment variables. + # https://docs.appsignal.com/python/configuration/options.html#option-push_api_key + push_api_key="My push API key", + collector_endpoint="https://collector.example", + service_name="web-server", +) +""" + +COLLECTOR_ENDPOINT_PROMPT = ( + "Please enter your collector endpoint (create one at" + " https://appsignal.com/redirect-to/organization?to=admin/hosted_collectors): " +) + +CONTINUE_WITHOUT_COLLECTOR_PROMPT = ( + "Are you sure? Without a collector, logging and distributed tracing" + " won't work. Continue without one? (y/N): " +) + +SERVICE_NAME_PROMPT = ( + "Please enter the name of this service" + " (such as web-server or background-worker): " +) + +NO_COLLECTOR = ( + (COLLECTOR_ENDPOINT_PROMPT, ""), + (CONTINUE_WITHOUT_COLLECTOR_PROMPT, "y"), +) + def mock_file_operations(mocker, file_exists: bool = False): mocker.patch("os.path.exists", return_value=file_exists) mocker.patch("appsignal.cli.install.open") -def assert_wrote_file_contents(mocker): +def assert_wrote_file_contents(mocker, contents=EXPECTED_FILE_CONTENTS): from appsignal.cli import install builtins_open: MagicMock = install.open # type: ignore[attr-defined] assert mocker.call("__appsignal__.py", "w") in builtins_open.mock_calls - assert ( - mocker.call().__enter__().write(EXPECTED_FILE_CONTENTS) - in builtins_open.mock_calls - ) + assert mocker.call().__enter__().write(contents) in builtins_open.mock_calls def assert_did_not_write_file_contents(mocker): @@ -52,9 +84,8 @@ def assert_did_not_write_file_contents(mocker): def assert_wrote_real_file_contents(test_dir, name, push_api_key): with open(os.path.join(test_dir, "__appsignal__.py")) as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name=name, - push_api_key=push_api_key, + file_contents = install_file_contents( + Options(name=name, push_api_key=push_api_key) ) assert f.read() == file_contents @@ -72,6 +103,7 @@ def test_install_command_run(mocker): mocker, ("Please enter the name of your application: ", "My app name"), ("Please enter your Push API key: ", "My push API key"), + *NO_COLLECTOR, ): main(["install"]) @@ -88,6 +120,7 @@ def test_install_command_when_empty_value_ask_again(mocker): ("Please enter the name of your application: ", "My app name"), ("Please enter your Push API key: ", ""), ("Please enter your Push API key: ", "My push API key"), + *NO_COLLECTOR, ): main(["install"]) @@ -101,6 +134,7 @@ def test_install_command_when_push_api_key_given(mocker): with mock_input( mocker, ("Please enter the name of your application: ", "My app name"), + *NO_COLLECTOR, ): main(["install", "--push-api-key", "My push API key"]) @@ -115,6 +149,7 @@ def test_install_command_when_file_exists_overwrite(mocker, request): mocker, ("Please enter the name of your application: ", "My app name"), ("Please enter your Push API key: ", "My push API key"), + *NO_COLLECTOR, ( "The __appsignal__.py file already exists." " Should it be overwritten? (y/N): ", @@ -126,9 +161,8 @@ def test_install_command_when_file_exists_overwrite(mocker, request): os.makedirs(test_dir) # Add client file with open(os.path.join(test_dir, "__appsignal__.py"), "w") as f: - file_contents = INSTALL_FILE_TEMPLATE.format( - name="Existing app name", - push_api_key="Existing Push API key", + file_contents = install_file_contents( + Options(name="Existing app name", push_api_key="Existing Push API key") ) f.write(file_contents) os.chdir(test_dir) @@ -149,6 +183,7 @@ def test_install_command_when_file_exists_no_overwrite(mocker): mocker, ("Please enter the name of your application: ", "My app name"), ("Please enter your Push API key: ", "My push API key"), + *NO_COLLECTOR, ( "The __appsignal__.py file already exists." " Should it be overwritten? (y/N): ", @@ -175,3 +210,41 @@ def test_install_command_when_invalid_api_key_ask_again(mocker): main(["install"]) assert_did_not_write_file_contents(mocker) + + +def test_install_command_with_collector(mocker): + mock_file_operations(mocker) + mock_validate_push_api_key_request(mocker) + + with mock_input( + mocker, + ("Please enter the name of your application: ", "My app name"), + ("Please enter your Push API key: ", "My push API key"), + (COLLECTOR_ENDPOINT_PROMPT, "https://collector.example"), + (SERVICE_NAME_PROMPT, "web-server"), + ): + main(["install"]) + + assert_wrote_file_contents(mocker, EXPECTED_COLLECTOR_FILE_CONTENTS) + + +def test_install_command_when_empty_collector_endpoint_ask_again(mocker): + mock_file_operations(mocker) + mock_validate_push_api_key_request(mocker) + + with mock_input( + mocker, + ("Please enter the name of your application: ", "My app name"), + ("Please enter your Push API key: ", "My push API key"), + (COLLECTOR_ENDPOINT_PROMPT, ""), + (CONTINUE_WITHOUT_COLLECTOR_PROMPT, ""), + (COLLECTOR_ENDPOINT_PROMPT, ""), + (CONTINUE_WITHOUT_COLLECTOR_PROMPT, "maybe"), + (CONTINUE_WITHOUT_COLLECTOR_PROMPT, "n"), + (COLLECTOR_ENDPOINT_PROMPT, "https://collector.example"), + (SERVICE_NAME_PROMPT, ""), + (SERVICE_NAME_PROMPT, "web-server"), + ): + main(["install"]) + + assert_wrote_file_contents(mocker, EXPECTED_COLLECTOR_FILE_CONTENTS)