chore(telemetry): update config names and values (#10987)

- Removes the name argument from `_ConfigItem.__init__(..)`. The name of a configuration will be set to the first environment variable in the envs field (ie the primary environment variable). - Avoids sending arbitrary configuration names to instrumentation telemetry. When possible, the configuration name should match a well documented name (ex: a public environment variable). - Avoids reporting integers, floats, booleans and None types as strings in instrumentation telemetry payloads ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
DataDog · Oct 17, 2024 · c2135e7 · c2135e7
1 parent f505953
commit c2135e7
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 83 deletions.
diff --git a/ddtrace/internal/telemetry/writer.py b/ddtrace/internal/telemetry/writer.py
@@ -465,7 +465,11 @@ def remove_configuration(self, configuration_name):
     def add_configuration(self, configuration_name, configuration_value, origin="unknown"):
         # type: (str, Any, str) -> None
         """Creates and queues the name, origin, value of a configuration"""
-        if not isinstance(configuration_value, (bool, str, int, float, type(None))):
+        if isinstance(configuration_value, dict):
+            configuration_value = ",".join(":".join((k, str(v))) for k, v in configuration_value.items())
+        elif isinstance(configuration_value, (list, tuple)):
+            configuration_value = ",".join(str(v) for v in configuration_value)
+        elif not isinstance(configuration_value, (bool, str, int, float, type(None))):
             # convert unsupported types to strings
             configuration_value = str(configuration_value)
 

diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py
@@ -198,10 +198,10 @@ def get_error_ranges(error_range_str):
 class _ConfigItem:
     """Configuration item that tracks the value of a setting, and where it came from."""
 
-    def __init__(self, name, default, envs):
-        # type: (str, Union[_JSONType, Callable[[], _JSONType]], List[Tuple[str, Callable[[str], Any]]]) -> None
+    def __init__(self, default, envs):
+        # type: (Union[_JSONType, Callable[[], _JSONType]], List[Tuple[str, Callable[[str], Any]]]) -> None
         # _ConfigItem._name is only used in __repr__ and instrumentation telemetry
-        self._name = name
+        self._name = envs[0][0]
         self._env_value: _JSONType = None
         self._code_value: _JSONType = None
         self._rc_value: _JSONType = None
@@ -214,7 +214,7 @@ def __init__(self, name, default, envs):
             if env_var in os.environ:
                 self._env_value = parser(os.environ[env_var])
                 break
-        telemetry_writer.add_configuration(name, self._telemetry_value(), self.source())
+        telemetry_writer.add_configuration(self._name, self.value(), self.source())
 
     def set_value_source(self, value: Any, source: _ConfigSource) -> None:
         if source == "code":
@@ -248,16 +248,6 @@ def source(self) -> _ConfigSource:
             return "env_var"
         return "default"
 
-    def _telemetry_value(self) -> str:
-        val = self.value()
-        if val is None:
-            return ""
-        elif isinstance(val, (bool, int, float)):
-            return str(val).lower()
-        elif isinstance(val, dict):
-            return ",".join(":".join((k, str(v))) for k, v in val.items())
-        return str(val)
-
     def __repr__(self):
         return "<{} name={} default={} env_value={} user_value={} remote_config_value={}>".format(
             self.__class__.__name__,
@@ -277,52 +267,42 @@ def _parse_global_tags(s):
 def _default_config() -> Dict[str, _ConfigItem]:
     return {
         "_trace_sample_rate": _ConfigItem(
-            name="trace_sample_rate",
             default=1.0,
             envs=[("DD_TRACE_SAMPLE_RATE", float)],
         ),
         "_trace_sampling_rules": _ConfigItem(
-            name="trace_sampling_rules",
             default=lambda: "",
             envs=[("DD_TRACE_SAMPLING_RULES", str)],
         ),
         "_logs_injection": _ConfigItem(
-            name="logs_injection_enabled",
             default=False,
             envs=[("DD_LOGS_INJECTION", asbool)],
         ),
         "_trace_http_header_tags": _ConfigItem(
-            name="trace_header_tags",
             default=lambda: {},
             envs=[("DD_TRACE_HEADER_TAGS", parse_tags_str)],
         ),
         "tags": _ConfigItem(
-            name="trace_tags",
             default=lambda: {},
             envs=[("DD_TAGS", _parse_global_tags)],
         ),
         "_tracing_enabled": _ConfigItem(
-            name="trace_enabled",
             default=True,
             envs=[("DD_TRACE_ENABLED", asbool)],
         ),
         "_profiling_enabled": _ConfigItem(
-            name="profiling_enabled",
             default=False,
             envs=[("DD_PROFILING_ENABLED", asbool)],
         ),
         "_asm_enabled": _ConfigItem(
-            name="appsec_enabled",
             default=False,
             envs=[("DD_APPSEC_ENABLED", asbool)],
         ),
         "_sca_enabled": _ConfigItem(
-            name="DD_APPSEC_SCA_ENABLED",
             default=None,
             envs=[("DD_APPSEC_SCA_ENABLED", asbool)],
         ),
         "_dsm_enabled": _ConfigItem(
-            name="data_streams_enabled",
             default=False,
             envs=[("DD_DATA_STREAMS_ENABLED", asbool)],
         ),
@@ -405,12 +385,6 @@ def __init__(self):
         # Must come before _integration_configs due to __setattr__
         self._config = _default_config()
 
-        # Remove the SCA configuration from instrumentation telemetry if it is not set
-        # this behavior is validated by system tests
-        # FIXME(munir): Is this really needed? Should report the default value (None) instead?
-        if self._config["_sca_enabled"].value() is None:
-            telemetry_writer.remove_configuration("DD_APPSEC_SCA_ENABLED")
-
         sample_rate = os.getenv("DD_TRACE_SAMPLE_RATE")
         if sample_rate is not None:
             deprecate(
@@ -815,7 +789,7 @@ def _set_config_items(self, items):
             item = self._config[key]
             item.set_value_source(value, origin)
             if self._telemetry_enabled:
-                telemetry_writer.add_configuration(item._name, item._telemetry_value(), item.source())
+                telemetry_writer.add_configuration(item._name, item.value(), item.source())
         self._notify_subscribers(item_names)
 
     def _reset(self):

diff --git a/tests/integration/test_settings.py b/tests/integration/test_settings.py
@@ -39,29 +39,29 @@ def test_setting_origin_environment(test_agent_session, run_python_code_in_subpr
     assert status == 0, err
 
     events = test_agent_session.get_events()
-    events_trace_sample_rate = _get_telemetry_config_items(events, "trace_sample_rate")
+    events_trace_sample_rate = _get_telemetry_config_items(events, "DD_TRACE_SAMPLE_RATE")
 
     assert {
-        "name": "trace_sample_rate",
-        "value": "0.1",
+        "name": "DD_TRACE_SAMPLE_RATE",
+        "value": 0.1,
         "origin": "env_var",
     } in events_trace_sample_rate
 
-    events_logs_injection_enabled = _get_telemetry_config_items(events, "logs_injection_enabled")
-    assert {"name": "logs_injection_enabled", "value": "true", "origin": "env_var"} in events_logs_injection_enabled
+    events_logs_injection_enabled = _get_telemetry_config_items(events, "DD_LOGS_INJECTION")
+    assert {"name": "DD_LOGS_INJECTION", "value": True, "origin": "env_var"} in events_logs_injection_enabled
 
-    events_trace_header_tags = _get_telemetry_config_items(events, "trace_header_tags")
+    events_trace_header_tags = _get_telemetry_config_items(events, "DD_TRACE_HEADER_TAGS")
     assert {
-        "name": "trace_header_tags",
+        "name": "DD_TRACE_HEADER_TAGS",
         "value": "X-Header-Tag-1:header_tag_1,X-Header-Tag-2:header_tag_2",
         "origin": "env_var",
     } in events_trace_header_tags
 
-    events_trace_tags = _get_telemetry_config_items(events, "trace_tags")
-    assert {"name": "trace_tags", "value": "team:apm,component:web", "origin": "env_var"} in events_trace_tags
+    events_trace_tags = _get_telemetry_config_items(events, "DD_TAGS")
+    assert {"name": "DD_TAGS", "value": "team:apm,component:web", "origin": "env_var"} in events_trace_tags
 
-    events_tracing_enabled = _get_telemetry_config_items(events, "trace_enabled")
-    assert {"name": "trace_enabled", "value": "true", "origin": "env_var"} in events_tracing_enabled
+    events_tracing_enabled = _get_telemetry_config_items(events, "DD_TRACE_ENABLED")
+    assert {"name": "DD_TRACE_ENABLED", "value": True, "origin": "env_var"} in events_tracing_enabled
 
 
 @pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent")
@@ -96,38 +96,38 @@ def test_setting_origin_code(test_agent_session, run_python_code_in_subprocess):
     assert status == 0, err
 
     events = test_agent_session.get_events()
-    events_trace_sample_rate = _get_telemetry_config_items(events, "trace_sample_rate")
+    events_trace_sample_rate = _get_telemetry_config_items(events, "DD_TRACE_SAMPLE_RATE")
     assert {
-        "name": "trace_sample_rate",
-        "value": "0.2",
+        "name": "DD_TRACE_SAMPLE_RATE",
+        "value": 0.2,
         "origin": "code",
     } in events_trace_sample_rate
 
-    events_logs_injection_enabled = _get_telemetry_config_items(events, "logs_injection_enabled")
+    events_logs_injection_enabled = _get_telemetry_config_items(events, "DD_LOGS_INJECTION")
     assert {
-        "name": "logs_injection_enabled",
-        "value": "false",
+        "name": "DD_LOGS_INJECTION",
+        "value": False,
         "origin": "code",
     } in events_logs_injection_enabled
 
-    events_trace_header_tags = _get_telemetry_config_items(events, "trace_header_tags")
+    events_trace_header_tags = _get_telemetry_config_items(events, "DD_TRACE_HEADER_TAGS")
     assert {
-        "name": "trace_header_tags",
+        "name": "DD_TRACE_HEADER_TAGS",
         "value": "header:value",
         "origin": "code",
     } in events_trace_header_tags
 
-    events_trace_tags = _get_telemetry_config_items(events, "trace_tags")
+    events_trace_tags = _get_telemetry_config_items(events, "DD_TAGS")
     assert {
-        "name": "trace_tags",
+        "name": "DD_TAGS",
         "value": "header:value",
         "origin": "code",
     } in events_trace_tags
 
-    events_tracing_enabled = _get_telemetry_config_items(events, "trace_enabled")
+    events_tracing_enabled = _get_telemetry_config_items(events, "DD_TRACE_ENABLED")
     assert {
-        "name": "trace_enabled",
-        "value": "false",
+        "name": "DD_TRACE_ENABLED",
+        "value": False,
         "origin": "code",
     } in events_tracing_enabled
 
@@ -174,8 +174,8 @@ def test_remoteconfig_sampling_rate_default(test_agent_session, run_python_code_
     assert status == 0, err
 
     events = test_agent_session.get_events()
-    events_trace_sample_rate = _get_telemetry_config_items(events, "trace_sample_rate")
-    assert {"name": "trace_sample_rate", "value": "1.0", "origin": "default"} in events_trace_sample_rate
+    events_trace_sample_rate = _get_telemetry_config_items(events, "DD_TRACE_SAMPLE_RATE")
+    assert {"name": "DD_TRACE_SAMPLE_RATE", "value": 1.0, "origin": "default"} in events_trace_sample_rate
 
 
 @pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent")
@@ -201,8 +201,8 @@ def test_remoteconfig_sampling_rate_telemetry(test_agent_session, run_python_cod
     assert status == 0, err
 
     events = test_agent_session.get_events()
-    events_trace_sample_rate = _get_telemetry_config_items(events, "trace_sample_rate")
-    assert {"name": "trace_sample_rate", "value": "0.5", "origin": "remote_config"} in events_trace_sample_rate
+    events_trace_sample_rate = _get_telemetry_config_items(events, "DD_TRACE_SAMPLE_RATE")
+    assert {"name": "DD_TRACE_SAMPLE_RATE", "value": 0.5, "origin": "remote_config"} in events_trace_sample_rate
 
 
 @pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent")
@@ -238,9 +238,9 @@ def test_remoteconfig_header_tags_telemetry(test_agent_session, run_python_code_
     assert status == 0, err
 
     events = test_agent_session.get_events()
-    events_trace_header_tags = _get_telemetry_config_items(events, "trace_header_tags")
+    events_trace_header_tags = _get_telemetry_config_items(events, "DD_TRACE_HEADER_TAGS")
     assert {
-        "name": "trace_header_tags",
+        "name": "DD_TRACE_HEADER_TAGS",
         "value": "used:header_tag_69,unused:header_tag_70,used-with-default:",
         "origin": "remote_config",
     } in events_trace_header_tags
diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py
@@ -55,12 +55,12 @@ def test_add_event_disabled_writer(telemetry_writer, test_agent_session):
 @pytest.mark.parametrize(
     "env_var,value,expected_value",
     [
-        ("DD_APPSEC_SCA_ENABLED", "true", "true"),
-        ("DD_APPSEC_SCA_ENABLED", "True", "true"),
-        ("DD_APPSEC_SCA_ENABLED", "1", "true"),
-        ("DD_APPSEC_SCA_ENABLED", "false", "false"),
-        ("DD_APPSEC_SCA_ENABLED", "False", "false"),
-        ("DD_APPSEC_SCA_ENABLED", "0", "false"),
+        ("DD_APPSEC_SCA_ENABLED", "true", True),
+        ("DD_APPSEC_SCA_ENABLED", "True", True),
+        ("DD_APPSEC_SCA_ENABLED", "1", True),
+        ("DD_APPSEC_SCA_ENABLED", "false", False),
+        ("DD_APPSEC_SCA_ENABLED", "False", False),
+        ("DD_APPSEC_SCA_ENABLED", "0", False),
     ],
 )
 def test_app_started_event_configuration_override_asm(
@@ -312,6 +312,11 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         },
         {"name": "DD_APPSEC_RASP_ENABLED", "origin": "default", "value": True},
         {"name": "DD_APPSEC_RULES", "origin": "default", "value": None},
+        {
+            "name": "DD_APPSEC_SCA_ENABLED",
+            "origin": "default",
+            "value": None,
+        },
         {"name": "DD_APPSEC_STACK_TRACE_ENABLED", "origin": "default", "value": True},
         {"name": "DD_APPSEC_WAF_TIMEOUT", "origin": "default", "value": 5.0},
         {"name": "DD_CIVISIBILITY_AGENTLESS_ENABLED", "origin": "env_var", "value": False},
@@ -325,7 +330,7 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "DD_CRASHTRACKING_STACKTRACE_RESOLVER", "origin": "default", "value": "full"},
         {"name": "DD_CRASHTRACKING_STDERR_FILENAME", "origin": "default", "value": None},
         {"name": "DD_CRASHTRACKING_STDOUT_FILENAME", "origin": "default", "value": None},
-        {"name": "DD_CRASHTRACKING_TAGS", "origin": "default", "value": "{}"},
+        {"name": "DD_CRASHTRACKING_TAGS", "origin": "default", "value": ""},
         {"name": "DD_CRASHTRACKING_WAIT_FOR_RECEIVER", "origin": "default", "value": True},
         {"name": "DD_DATA_STREAMS_ENABLED", "origin": "env_var", "value": True},
         {"name": "DD_DOGSTATSD_PORT", "origin": "default", "value": None},
@@ -372,6 +377,7 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "DD_LLMOBS_ENABLED", "origin": "default", "value": False},
         {"name": "DD_LLMOBS_ML_APP", "origin": "default", "value": None},
         {"name": "DD_LLMOBS_SAMPLE_RATE", "origin": "default", "value": 1.0},
+        {"name": "DD_LOGS_INJECTION", "origin": "env_var", "value": True},
         {"name": "DD_PROFILING_AGENTLESS", "origin": "default", "value": False},
         {"name": "DD_PROFILING_API_TIMEOUT", "origin": "default", "value": 10.0},
         {"name": "DD_PROFILING_CAPTURE_PCT", "origin": "env_var", "value": 5.0},
@@ -385,7 +391,7 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "DD_PROFILING_MAX_TIME_USAGE_PCT", "origin": "default", "value": 1.0},
         {"name": "DD_PROFILING_OUTPUT_PPROF", "origin": "default", "value": None},
         {"name": "DD_PROFILING_SAMPLE_POOL_CAPACITY", "origin": "default", "value": 4},
-        {"name": "DD_PROFILING_TAGS", "origin": "default", "value": "{}"},
+        {"name": "DD_PROFILING_TAGS", "origin": "default", "value": ""},
         {"name": "DD_PROFILING_TIMELINE_ENABLED", "origin": "default", "value": False},
         {"name": "DD_PROFILING_UPLOAD_INTERVAL", "origin": "env_var", "value": 10.0},
         {"name": "DD_PROFILING__FORCE_LEGACY_EXPORTER", "origin": "env_var", "value": True},
@@ -404,6 +410,7 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         },
         {"name": "DD_SYMBOL_DATABASE_INCLUDES", "origin": "default", "value": "set()"},
         {"name": "DD_SYMBOL_DATABASE_UPLOAD_ENABLED", "origin": "default", "value": False},
+        {"name": "DD_TAGS", "origin": "env_var", "value": "team:apm,component:web"},
         {"name": "DD_TELEMETRY_DEPENDENCY_COLLECTION_ENABLED", "origin": "default", "value": True},
         {"name": "DD_TELEMETRY_HEARTBEAT_INTERVAL", "origin": "default", "value": 60},
         {"name": "DD_TESTING_RAISE", "origin": "env_var", "value": True},
@@ -417,6 +424,8 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "DD_TRACE_CLIENT_IP_HEADER", "origin": "default", "value": None},
         {"name": "DD_TRACE_COMPUTE_STATS", "origin": "env_var", "value": True},
         {"name": "DD_TRACE_DEBUG", "origin": "env_var", "value": True},
+        {"name": "DD_TRACE_ENABLED", "origin": "env_var", "value": False},
+        {"name": "DD_TRACE_HEADER_TAGS", "origin": "default", "value": ""},
         {"name": "DD_TRACE_HEALTH_METRICS_ENABLED", "origin": "env_var", "value": True},
         {"name": "DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING", "origin": "default", "value": "true"},
         {"name": "DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "origin": "default", "value": "500-599"},
@@ -431,6 +440,12 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "DD_TRACE_PROPAGATION_STYLE_INJECT", "origin": "env_var", "value": "tracecontext"},
         {"name": "DD_TRACE_RATE_LIMIT", "origin": "env_var", "value": 50},
         {"name": "DD_TRACE_REPORT_HOSTNAME", "origin": "default", "value": False},
+        {"name": "DD_TRACE_SAMPLE_RATE", "origin": "env_var", "value": 0.5},
+        {
+            "name": "DD_TRACE_SAMPLING_RULES",
+            "origin": "env_var",
+            "value": '[{"sample_rate":1.0,"service":"xyz","name":"abc"}]',
+        },
         {"name": "DD_TRACE_SPAN_AGGREGATOR_RLOCK", "origin": "default", "value": True},
         {"name": "DD_TRACE_SPAN_TRACEBACK_MAX_SIZE", "origin": "default", "value": 30},
         {"name": "DD_TRACE_STARTUP_LOGS", "origin": "env_var", "value": True},
@@ -447,24 +462,11 @@ def test_app_started_event_configuration_override(test_agent_session, run_python
         {"name": "_DD_IAST_LAZY_TAINT", "origin": "default", "value": False},
         {"name": "_DD_INJECT_WAS_ATTEMPTED", "origin": "default", "value": False},
         {"name": "_DD_TRACE_WRITER_LOG_ERROR_PAYLOADS", "origin": "default", "value": False},
-        {"name": "appsec_enabled", "origin": "env_var", "value": "true"},
-        {"name": "data_streams_enabled", "origin": "env_var", "value": "true"},
         {"name": "ddtrace_auto_used", "origin": "unknown", "value": True},
         {"name": "ddtrace_bootstrapped", "origin": "unknown", "value": True},
-        {"name": "logs_injection_enabled", "origin": "env_var", "value": "true"},
-        {"name": "profiling_enabled", "origin": "env_var", "value": "true"},
         {"name": "python_build_gnu_type", "origin": "unknown", "value": sysconfig.get_config_var("BUILD_GNU_TYPE")},
         {"name": "python_host_gnu_type", "origin": "unknown", "value": sysconfig.get_config_var("HOST_GNU_TYPE")},
         {"name": "python_soabi", "origin": "unknown", "value": sysconfig.get_config_var("SOABI")},
-        {"name": "trace_enabled", "origin": "env_var", "value": "false"},
-        {"name": "trace_header_tags", "origin": "default", "value": ""},
-        {"name": "trace_sample_rate", "origin": "env_var", "value": "0.5"},
-        {
-            "name": "trace_sampling_rules",
-            "origin": "env_var",
-            "value": '[{"sample_rate":1.0,"service":"xyz","name":"abc"}]',
-        },
-        {"name": "trace_tags", "origin": "env_var", "value": "team:apm,component:web"},
     ]
     assert configurations == expected, configurations