From 26bbadedc91a888c814c6dd1350583cb4bff6fd8 Mon Sep 17 00:00:00 2001 From: Noemi Lapresta Date: Tue, 15 Sep 2026 13:29:12 +0200 Subject: [PATCH] Name the source of each config option in diagnose The diagnose report prints every configuration option with the value it holds, and nothing else. An option that is not what the application set is the first thing somebody looks at, and the report gives them no way to tell a default apart from a value read from the environment, so the next step is guesswork. The Ruby gem's report has named the source for years. Print the source beside the value, and, for an option more than one source holds, list each source with what it holds in the order they are merged. The order comes from the same constant the merge walks, so the report cannot disagree with the merge about which value wins. --- .../name-the-source-of-each-config-option.md | 6 ++++ src/appsignal/cli/diagnose.py | 30 +++++++++++++++++-- src/appsignal/config.py | 14 ++++++--- tests/cli/test_diagnose.py | 26 ++++++++++++++++ tests/diagnose | 2 +- 5 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 .changesets/name-the-source-of-each-config-option.md 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