Report detected/configured_hostname and fix tests (#1891)

* Report detected/configured_hostname and fix tests

hostname is deprecated

* CHANGELOG

* Add server version check for detected_hostname

* Use only the first part of the detected hostname as the pod name

* Remove detected_hostname config option

* Add tests for getfqdn()

CHANGELOG.asciidoc

@@ -29,6 +29,19 @@ endif::[]
 //===== Bug fixes
 //
 
+=== Unreleased
+
+// Unreleased changes go here
+// When the next release happens, nest these changes under the "Python Agent version 6.x" heading
+[float]
+===== Features
+
+* Collect the `configured_hostname` and `detected_hostname` separately, and switch to FQDN for the `detected_hostname`. {pull}1891[#1891]
+
+//[float]
+//===== Bug fixes
+//
+
 
 
 [[release-notes-6.x]]

elasticapm/base.py

@@ -381,12 +381,18 @@ def get_process_info(self):
 
     def get_system_info(self):
         system_data = {
-            "hostname": keyword_field(self.config.hostname),
+            "detected_hostname": keyword_field(elasticapm.utils.getfqdn()),
             "architecture": platform.machine(),
             "platform": platform.system().lower(),
         }
+        if self.config.hostname:
+            system_data["configured_hostname"] = keyword_field(self.config.hostname)
+        if not self.check_server_version(gte=(7, 4, 0)):
+            system_data["hostname"] = system_data.get("configured_hostname", system_data["detected_hostname"])
         system_data.update(cgroup.get_cgroup_container_metadata())
-        pod_name = os.environ.get("KUBERNETES_POD_NAME") or system_data["hostname"]
+        pod_name = os.environ.get("KUBERNETES_POD_NAME") or keyword_field(
+            self.config.hostname or elasticapm.utils.getfqdn().split(".")[0]
+        )
         changed = False
         if "kubernetes" in system_data:
             k8s = system_data["kubernetes"]

elasticapm/conf/__init__.py

@@ -34,7 +34,6 @@
 import math
 import os
 import re
-import socket
 import threading
 from datetime import timedelta
 
@@ -572,7 +571,7 @@ class Config(_ConfigBase):
         ],
         default=5,
     )
-    hostname = _ConfigValue("HOSTNAME", default=socket.gethostname())
+    hostname = _ConfigValue("HOSTNAME", default=None)
     auto_log_stacks = _BoolConfigValue("AUTO_LOG_STACKS", default=True)
     transport_class = _ConfigValue("TRANSPORT_CLASS", default="elasticapm.transport.http.Transport", required=True)
     processors = _ListConfigValue(

elasticapm/utils/__init__.py

@@ -31,6 +31,7 @@
 import base64
 import os
 import re
+import socket
 import urllib.parse
 from functools import partial
 from types import FunctionType
@@ -49,6 +50,7 @@
 
 
 default_ports = {"https": 443, "http": 80, "postgresql": 5432, "mysql": 3306, "mssql": 1433}
+fqdn = None
 
 
 def varmap(func, var, context=None, name=None, **kwargs):
@@ -221,3 +223,29 @@ def nested_key(d: dict, *args):
             d = None
             break
     return d
+
+
+def getfqdn() -> str:
+    """
+    socket.getfqdn() has some issues. For one, it's slow (may do a DNS lookup).
+    For another, it can return `localhost.localdomain`[1], which is less useful
+    than socket.gethostname().
+
+    This function handles the fallbacks and also ensures we don't try to lookup
+    the fqdn more than once.
+
+    [1]: https://stackoverflow.com/a/43330159
+    """
+    global fqdn
+    if not fqdn:
+        fqdn = socket.getfqdn()
+        if fqdn == "localhost.localdomain":
+            fqdn = socket.gethostname()
+        if not fqdn:
+            fqdn = os.environ.get("HOSTNAME")
+        if not fqdn:
+            fqdn = os.environ.get("HOST")
+        if fqdn is None:
+            fqdn = ""
+        fqdn = fqdn.lower().strip()
+    return fqdn

tests/client/client_tests.py

@@ -94,15 +94,15 @@ def test_system_info(elasticapm_client):
     with mock.patch("elasticapm.utils.cgroup.get_cgroup_container_metadata") as mocked:
         mocked.return_value = {}
         system_info = elasticapm_client.get_system_info()
-    assert {"hostname", "architecture", "platform"} == set(system_info.keys())
-    assert system_info["hostname"] == socket.gethostname()
+    assert {"detected_hostname", "architecture", "platform"} == set(system_info.keys())
+    assert system_info["detected_hostname"] == elasticapm.utils.getfqdn()
 
 
 @pytest.mark.parametrize("elasticapm_client", [{"hostname": "my_custom_hostname"}], indirect=True)
 def test_system_info_hostname_configurable(elasticapm_client):
     # mock docker/kubernetes data here to get consistent behavior if test is run in docker
     system_info = elasticapm_client.get_system_info()
-    assert system_info["hostname"] == "my_custom_hostname"
+    assert system_info["configured_hostname"] == "my_custom_hostname"
 
 
 @pytest.mark.parametrize("elasticapm_client", [{"global_labels": "az=us-east-1,az.rack=8"}], indirect=True)
@@ -117,7 +117,7 @@ def test_docker_kubernetes_system_info(elasticapm_client):
         mock_metadata.return_value = {"container": {"id": "123"}, "kubernetes": {"pod": {"uid": "456"}}}
         system_info = elasticapm_client.get_system_info()
     assert system_info["container"] == {"id": "123"}
-    assert system_info["kubernetes"] == {"pod": {"uid": "456", "name": socket.gethostname()}}
+    assert system_info["kubernetes"] == {"pod": {"uid": "456", "name": elasticapm.utils.getfqdn().split(".")[0]}}
 
 
 @mock.patch.dict(
@@ -185,7 +185,10 @@ def test_docker_kubernetes_system_info_except_hostname_from_environ():
         mock_gethostname.return_value = "foo"
         system_info = elasticapm_client.get_system_info()
     assert "kubernetes" in system_info
-    assert system_info["kubernetes"] == {"pod": {"name": socket.gethostname()}, "namespace": "namespace"}
+    assert system_info["kubernetes"] == {
+        "pod": {"name": elasticapm.utils.getfqdn().split(".")[0]},
+        "namespace": "namespace",
+    }
 
 
 def test_config_by_environment():

tests/config/tests.py

@@ -128,7 +128,6 @@ def test_config_inline_dict():
             "secret_token": "bar",
             "server_url": "http://example.com:1234",
             "service_version": "1",
-            "hostname": "localhost",
             "api_request_time": "5s",
         }
     )
@@ -137,7 +136,7 @@ def test_config_inline_dict():
     assert config.secret_token == "bar"
     assert config.server_url == "http://example.com:1234"
     assert config.service_version == "1"
-    assert config.hostname == "localhost"
+    assert config.hostname is None
     assert config.api_request_time.total_seconds() == 5
 
 

tests/fixtures.py

@@ -485,3 +485,11 @@ def always_uninstrument_and_close():
                 client.close()
         except Exception:
             pass
+
+
+@pytest.fixture()
+def invalidate_fqdn_cache():
+    fqdn = elasticapm.utils.fqdn
+    elasticapm.utils.fqdn = None
+    yield
+    elasticapm.utils.fqdn = fqdn

tests/utils/tests.py

@@ -29,14 +29,17 @@
 #  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import os
+import socket
 from functools import partial
 
 import pytest
 
+import elasticapm.utils
 from elasticapm.conf import constants
 from elasticapm.utils import (
     get_name_from_func,
     get_url_dict,
+    getfqdn,
     nested_key,
     read_pem_file,
     sanitize_url,
@@ -259,3 +262,12 @@ def test_nested_key(data, key, expected):
         assert r is expected
     else:
         assert r == expected
+
+
+def test_getfqdn(invalidate_fqdn_cache):
+    assert getfqdn() == socket.getfqdn()
+
+
+def test_getfqdn_caches(invalidate_fqdn_cache):
+    elasticapm.utils.fqdn = "foo"
+    assert getfqdn() == "foo"