diff --git a/.changesets/name-the-source-of-each-config-option.md b/.changesets/name-the-source-of-each-config-option.md new file mode 100644 index 00000000..e4d147ba --- /dev/null +++ b/.changesets/name-the-source-of-each-config-option.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: change +--- + +The `appsignal diagnose` report now names where each configuration option's value came from, so an option that is not what you set says what set it instead. An option holding a value from more than one source lists each one with the value it holds, in the order they are merged. diff --git a/src/appsignal/cli/diagnose.py b/src/appsignal/cli/diagnose.py index 9b24f9ee..eba60faf 100644 --- a/src/appsignal/cli/diagnose.py +++ b/src/appsignal/cli/diagnose.py @@ -8,11 +8,11 @@ from argparse import ArgumentParser from pathlib import Path from sys import stderr -from typing import Any +from typing import Any, cast from ..__about__ import __version__ from ..agent import Agent -from ..config import Config +from ..config import SOURCE_ORDER, Config from ..push_api_key_validator import PushApiKeyValidator from ..transmitter import transmit from .command import AppsignalCLICommand @@ -311,12 +311,36 @@ def _configuration_information(self) -> None: print("Configuration") for key in self.config.options: - print(f" {key}: {self.config.options[key]!r}") # type: ignore + value = self.config.options[key] # type: ignore + print(f" {key}: {value!r}{self._config_sources_label(key)}") print() print("Read more about how the diagnose config output is rendered") print("https://docs.appsignal.com/python/command-line/diagnose.html") + # Names where an option's value came from, so that somebody looking at an + # option that is not what they set can see what set it instead. An option + # left at its default says nothing, because there is no source to point at. + def _config_sources_label(self, option: str) -> str: + sources = cast(dict, self.config.sources) + names = [name for name in SOURCE_ORDER if option in sources[name]] + + if names == ["default"]: + return "" + + if len(names) == 1: + return f" (Loaded from: {names[0]})" + + # More than one source holds the option, so each is listed with the + # value it holds, in the order they are merged. The last one wins. + width = max(len(name) for name in names) + 1 + lines = ["", " Sources:"] + for name in names: + label = f"{name}:".ljust(width) + lines.append(f" {label} {sources[name][option]!r}") + + return "\n".join(lines) + def _validation_information(self) -> None: validation_report: Any = self.report["validation"] print("Validation") diff --git a/src/appsignal/config.py b/src/appsignal/config.py index 5ffe2971..dd9b1cc3 100644 --- a/src/appsignal/config.py +++ b/src/appsignal/config.py @@ -73,6 +73,13 @@ class Sources(TypedDict): environment: Options +# The configuration sources, in the order they are merged, so the last one +# holding an option is the one whose value it takes. Both the merge and the +# diagnose report walk this, so neither can disagree with the other about +# where a value came from. +SOURCE_ORDER: list[str] = ["default", "system", "environment", "initial"] + + class Config: valid: bool sources: Sources @@ -142,11 +149,10 @@ def __init__(self, options: Options | None = None) -> None: initial=without_none_overrides(options or Options(), system), environment=Config.load_from_environment(), ) + sources = cast(dict, self.sources) final_options = Options() - final_options.update(self.sources["default"]) - final_options.update(self.sources["system"]) - final_options.update(self.sources["environment"]) - final_options.update(self.sources["initial"]) + for source in SOURCE_ORDER: + final_options.update(sources[source]) self.options = final_options self._validate() diff --git a/tests/cli/test_diagnose.py b/tests/cli/test_diagnose.py index 1224ae17..be06b985 100644 --- a/tests/cli/test_diagnose.py +++ b/tests/cli/test_diagnose.py @@ -116,3 +116,29 @@ def test_diagnose_with_missing_paths(mocker, capfd): out, err = capfd.readouterr() assert "Exists?: False" in out + + +def test_diagnose_names_where_a_value_came_from(mocker, capfd): + os.environ["APPSIGNAL_APP_ENV"] = "production" + os.environ["APPSIGNAL_HOST_ROLE"] = "worker" + + main(["diagnose", "--no-send-report"]) + + out, err = capfd.readouterr() + # One source holds it, so the option just says which. + assert " host_role: 'worker' (Loaded from: environment)" in out + # The default holds a value too, so both are listed with what they hold. + assert ( + " environment: 'production'\n" + " Sources:\n" + " default: 'development'\n" + " environment: 'production'\n" + ) in out + + +def test_diagnose_says_nothing_about_an_option_left_at_its_default(mocker, capfd): + main(["diagnose", "--no-send-report"]) + + out, err = capfd.readouterr() + assert " log: 'file'\n" in out + assert " log: 'file' (Loaded from" not in out diff --git a/tests/diagnose b/tests/diagnose index 30f1c121..5a83ffb4 160000 --- a/tests/diagnose +++ b/tests/diagnose @@ -1 +1 @@ -Subproject commit 30f1c121a4960999fccf02d18317e99edeb0d320 +Subproject commit 5a83ffb4b1e605d8d848cda7ed718de9e0ea057f