Detect the revision, hostname and platform in the package

.changesets/detect-the-revision-from-the-platform.md

@@ -0,0 +1,8 @@
+---
+bump: patch
+type: add
+---
+
+Detect the revision that is being deployed from the environment variables set by Heroku, Render, Kamal and Scalingo: `HEROKU_SLUG_COMMIT`, `RENDER_GIT_COMMIT`, `KAMAL_VERSION` and `CONTAINER_VERSION`. Applications deployed on those platforms now report their revision without setting the `revision` configuration option.
+
+This affects collector mode, where deploys were reported as `unknown` when the revision was not configured.

.changesets/keep-detected-values-when-an-option-is-none.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: fix
+---
+
+An option set to `None` when initializing the `Appsignal` client no longer replaces a value that AppSignal detected itself. For example, `Appsignal(hostname=None)` now reports the detected hostname, instead of reporting no hostname at all. Options that AppSignal does not detect are unchanged: setting `request_headers` to `None`, for example, still turns off request header collection.

.changesets/prefer-the-dyno-name-for-the-hostname.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: change
+---
+
+On Heroku, report the name of the dyno as the hostname. Before, the hostname of the container that the dyno runs in was reported, so applications running on Heroku will see their data reported under a new host name.

src/appsignal/config.py

@@ -49,6 +49,7 @@ class Options(TypedDict, total=False):
     nginx_port: str | int | None
     opentelemetry_port: str | int | None
     name: str | None
+    platform: str | None
     push_api_key: str | None
     revision: str | None
     request_headers: list[str] | None
@@ -134,10 +135,11 @@ class Config:
 
     def __init__(self, options: Options | None = None) -> None:
         self.valid = False
+        system = Config.load_from_system()
         self.sources = Sources(
             default=self.DEFAULT_CONFIG,
-            system=Config.load_from_system(),
-            initial=options or Options(),
+            system=system,
+            initial=without_none_overrides(options or Options(), system),
             environment=Config.load_from_environment(),
         )
         final_options = Options()
@@ -171,13 +173,63 @@ def should_use_collector(self) -> bool:
     def should_use_external_collector(self) -> bool:
         return self.option("collector_endpoint") is not None
 
+    # Environment variables that deployment platforms set to the revision that
+    # is being deployed, in the order the agent reads them. The agent detects
+    # the revision this way as well, but only for the data it reports itself,
+    # so the package has to do it for collector mode.
+    PLATFORM_REVISION_ENVIRONMENT_VARIABLES: ClassVar[list[str]] = [
+        "HEROKU_SLUG_COMMIT",
+        "RENDER_GIT_COMMIT",
+        "KAMAL_VERSION",
+        "CONTAINER_VERSION",  # Scalingo
+    ]
+
     @staticmethod
     def load_from_system() -> Options:
-        return Options(
+        options = Options(
             app_path=os.getcwd(),
-            hostname=os.environ.get("HOSTNAME") or socket.gethostname(),
+            # The Heroku dyno name comes first, the way the agent detects the
+            # hostname. Heroku sets the container hostname as well, and the
+            # dyno name is the more useful of the two.
+            hostname=os.environ.get("DYNO")
+            or os.environ.get("HOSTNAME")
+            or socket.gethostname(),
         )
 
+        revision = Config.detect_revision()
+        if revision is not None:
+            options["revision"] = revision
+
+        detected_platform = Config.detect_platform()
+        if detected_platform is not None:
+            options["platform"] = detected_platform
+
+        return options
+
+    @staticmethod
+    def detect_revision() -> str | None:
+        for variable in Config.PLATFORM_REVISION_ENVIRONMENT_VARIABLES:
+            revision = os.environ.get(variable)
+            if revision:
+                return revision
+
+        return None
+
+    # Detect the platform the application is deployed on, the way the agent
+    # does. The agent only detects it for the data it reports itself, so the
+    # package has to do it for collector mode. It is detected rather than
+    # configured: it has no environment variable of its own, and it is not
+    # documented as an option.
+    @staticmethod
+    def detect_platform() -> str | None:
+        if os.environ.get("DOKKU_ROOT"):
+            return "dokku"
+
+        if os.environ.get("DYNO"):
+            return "heroku"
+
+        return None
+
     @staticmethod
     def load_from_environment() -> Options:
         options = Options(
@@ -313,6 +365,7 @@ def set_private_environ(self) -> None:
             "_APPSIGNAL_LOGGING_ENDPOINT": options.get("logging_endpoint"),
             "_APPSIGNAL_NGINX_PORT": options.get("nginx_port"),
             "_APPSIGNAL_OPENTELEMETRY_PORT": options.get("opentelemetry_port"),
+            "_APPSIGNAL_PLATFORM": options.get("platform"),
             "_APPSIGNAL_PUSH_API_KEY": options.get("push_api_key"),
             "_APPSIGNAL_PUSH_API_ENDPOINT": options.get("endpoint"),
             "_APPSIGNAL_RUNNING_IN_CONTAINER": bool_to_env_str(
@@ -501,6 +554,20 @@ def parse_bool(value: str | None) -> bool | None:
     return None
 
 
+# An option passed to the client as None carries no value, so it must not erase
+# one that was detected from the system. It does still override a default: that
+# is how `request_headers=None` turns off request header collection.
+def without_none_overrides(options: Options, system: Options) -> Options:
+    return cast(
+        Options,
+        {
+            key: value
+            for key, value in options.items()
+            if value is not None or key not in system
+        },
+    )
+
+
 def parse_list(value: str | None) -> list[str] | None:
     if value is None:
         return None

src/appsignal/opentelemetry.py

@@ -290,6 +290,7 @@ def _resource(config: Config) -> Resource:
             "appsignal.config.push_api_key": config.options.get("push_api_key"),
             "appsignal.config.revision": config.options.get("revision") or "unknown",
             "appsignal.config.app_path": config.options.get("app_path"),
+            "appsignal.config.platform": config.options.get("platform"),
             "appsignal.config.language_integration": "python",
             "service.name": config.options.get("service_name") or "app",
             "host.name": config.options.get("hostname") or "unknown",

tests/test_config.py

@@ -1,11 +1,34 @@
 from __future__ import annotations
 
 import os
+import socket
+
+import pytest
 
 from appsignal.__about__ import __version__
 from appsignal.config import Config, Options
 
 
+# The system source detects configuration from these environment variables, so
+# a value that happens to be set where the tests run changes what it detects.
+DETECTED_ENVIRONMENT_VARIABLES = [
+    "APP_REVISION",
+    "CONTAINER_VERSION",
+    "DOKKU_ROOT",
+    "DYNO",
+    "HEROKU_SLUG_COMMIT",
+    "HOSTNAME",
+    "KAMAL_VERSION",
+    "RENDER_GIT_COMMIT",
+]
+
+
+@pytest.fixture(autouse=True)
+def clear_detected_environment_variables():
+    for variable in DETECTED_ENVIRONMENT_VARIABLES:
+        os.environ.pop(variable, None)
+
+
 def test_option():
     config = Config(Options(active=False, enable_host_metrics=True))
 
@@ -43,6 +66,115 @@ def test_system_source():
     assert "hostname" in list(config.options.keys())
 
 
+def test_system_source_hostname():
+    config = Config()
+
+    assert config.option("hostname") == socket.gethostname()
+
+
+def test_system_source_hostname_from_hostname_variable():
+    os.environ["HOSTNAME"] = "from-hostname"
+    config = Config()
+
+    assert config.option("hostname") == "from-hostname"
+
+
+def test_system_source_hostname_prefers_the_dyno_name():
+    os.environ["DYNO"] = "web.1"
+    os.environ["HOSTNAME"] = "from-hostname"
+    config = Config()
+
+    assert config.option("hostname") == "web.1"
+
+
+def test_initial_options_none_does_not_override_detected_values():
+    os.environ["RENDER_GIT_COMMIT"] = "abc123"
+    config = Config(Options(hostname=None, revision=None))
+
+    assert config.option("hostname") == socket.gethostname()
+    assert config.option("revision") == "abc123"
+
+
+def test_initial_options_none_still_overrides_defaults():
+    config = Config(Options(request_headers=None))
+
+    assert config.option("request_headers") is None
+
+
+def test_system_source_platform():
+    config = Config()
+
+    assert "platform" not in config.sources["system"]
+
+
+def test_system_source_platform_dokku():
+    os.environ["DOKKU_ROOT"] = "~dokku"
+    config = Config()
+
+    assert config.option("platform") == "dokku"
+
+
+def test_system_source_platform_heroku():
+    os.environ["DYNO"] = "web.1"
+    config = Config()
+
+    assert config.option("platform") == "heroku"
+
+
+def test_system_source_platform_prefers_dokku():
+    os.environ["DOKKU_ROOT"] = "~dokku"
+    os.environ["DYNO"] = "web.1"
+    config = Config()
+
+    assert config.option("platform") == "dokku"
+
+
+def test_system_source_revision():
+    config = Config()
+
+    assert "revision" not in config.sources["system"]
+
+
+def test_system_source_revision_from_platform():
+    for variable in [
+        "HEROKU_SLUG_COMMIT",
+        "RENDER_GIT_COMMIT",
+        "KAMAL_VERSION",
+        "CONTAINER_VERSION",
+    ]:
+        os.environ[variable] = "abc123"
+        config = Config()
+
+        assert config.sources["system"]["revision"] == "abc123"
+        assert config.option("revision") == "abc123"
+
+        del os.environ[variable]
+
+
+def test_system_source_revision_reads_variables_in_order():
+    os.environ["RENDER_GIT_COMMIT"] = "from-render"
+    os.environ["KAMAL_VERSION"] = "from-kamal"
+    config = Config()
+
+    assert config.option("revision") == "from-render"
+
+
+def test_system_source_revision_ignores_empty_variables():
+    os.environ["HEROKU_SLUG_COMMIT"] = ""
+    os.environ["RENDER_GIT_COMMIT"] = "from-render"
+    config = Config()
+
+    assert config.option("revision") == "from-render"
+
+
+def test_system_source_revision_is_overridden_by_app_revision():
+    os.environ["RENDER_GIT_COMMIT"] = "from-render"
+    os.environ["APP_REVISION"] = "from-app-revision"
+    config = Config()
+
+    assert config.option("revision") == "from-app-revision"
+
+
 def test_environ_source():
     os.environ["APPSIGNAL_ACTIVE"] = "true"
     os.environ["APPSIGNAL_APP_ENV"] = "development"
@@ -204,6 +336,7 @@ def test_set_private_environ():
             nginx_port=8080,
             opentelemetry_port=9002,
             name="MyApp",
+            platform="heroku",
             push_api_key="some-api-key",
             revision="abc123",
             running_in_container=True,
@@ -232,6 +365,7 @@ def test_set_private_environ():
     assert os.environ["_APPSIGNAL_FILTER_PARAMETERS"] == "password,secret"
     assert os.environ["_APPSIGNAL_FILTER_SESSION_DATA"] == "key1,key2"
     assert os.environ["_APPSIGNAL_HOSTNAME"] == "Test hostname"
+    assert os.environ["_APPSIGNAL_PLATFORM"] == "heroku"
     assert os.environ["_APPSIGNAL_HOST_ROLE"] == "a role"
     assert os.environ["_APPSIGNAL_HTTP_PROXY"] == "http://proxy.local:9999"
     assert os.environ["_APPSIGNAL_IGNORE_ACTIONS"] == "action1,action2"
@@ -267,6 +401,7 @@ def test_opentelemetry_resource():
             push_api_key="test-key",
             revision="abc123",
             app_path="/path/to/app",
+            platform="heroku",
             service_name="test-service",
             hostname="test-host",
             filter_attributes=["password", "secret"],
@@ -295,6 +430,7 @@ def test_opentelemetry_resource():
     assert resource.attributes["appsignal.config.push_api_key"] == "test-key"
     assert resource.attributes["appsignal.config.revision"] == "abc123"
     assert resource.attributes["appsignal.config.app_path"] == "/path/to/app"
+    assert resource.attributes["appsignal.config.platform"] == "heroku"
     assert resource.attributes["appsignal.config.language_integration"] == "python"
     assert resource.attributes["service.name"] == "test-service"
     assert resource.attributes["host.name"] == "test-host"
@@ -363,7 +499,7 @@ def test_opentelemetry_resource_with_none_values():
 
     assert resource.attributes["appsignal.config.revision"] == "unknown"
     assert resource.attributes["service.name"] == "app"
-    assert resource.attributes["host.name"] == "unknown"
+    assert resource.attributes["host.name"] == socket.gethostname()
 
 
 def test_set_private_environ_valid_log_path():