Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions datadog_lambda/__init__.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import datadog_lambda.config # noqa: F401 needs to be imported before `ddtrace`
from datadog_lambda.cold_start import initialize_cold_start_tracing
import os


if os.environ.get("DD_INSTRUMENTATION_TELEMETRY_ENABLED") is None:
# Telemetry is required for Appsec Software Composition Analysis
os.environ["DD_INSTRUMENTATION_TELEMETRY_ENABLED"] = os.environ.get(
"DD_APPSEC_ENABLED", "false"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason this code was added here was we must ensure the DD_INSTRUMENTATION_TELEMETRY_ENABLED is set before any ddtrace code is imported. Can we ensure that we the change you're proposing? How can we enforce it?

Copy link
Contributor Author

@florentinl florentinl Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason this code was added here was we must ensure the DD_INSTRUMENTATION_TELEMETRY_ENABLED is set before any ddtrace code is imported.

I agree. As explained in the PR notes, this is the reason why we need the config to be resolved before the from datadog_lambda.patch import patch_all line of __init__.py.

Can we ensure that we the change you're proposing? How can we enforce it?

We can eagerly import the config in __init__.py and add a test to ensure that importing config.py does not import ddtrace. Would that work for you or does it feel like an unnecessary constraint ? Let me suggest something.

The idea was to reuse the parsing of environment variables done in config if possible. This also looked like some configuration and it felt natural to put it in "config.py".
I can keep it in __init__.py if it make more sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think any of those options are fine. As long as we can ensure it's being set before importing ddtrace. I like your idea of adding a test to check for this.



initialize_cold_start_tracing()
Expand Down
15 changes: 9 additions & 6 deletions datadog_lambda/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,6 @@ def _resolve_env(self, key, default=None, cast=None, depends_on_tracing=False):
logs_injection = _get_env("DD_LOGS_INJECTION", "true", as_bool)
merge_xray_traces = _get_env("DD_MERGE_XRAY_TRACES", "false", as_bool)

telemetry_enabled = _get_env(
"DD_INSTRUMENTATION_TELEMETRY_ENABLED",
"false",
as_bool,
depends_on_tracing=True,
)
otel_enabled = _get_env("DD_TRACE_OTEL_ENABLED", "false", as_bool)
profiling_enabled = _get_env("DD_PROFILING_ENABLED", "false", as_bool)
llmobs_enabled = _get_env("DD_LLMOBS_ENABLED", "false", as_bool)
Expand All @@ -96,6 +90,7 @@ def _resolve_env(self, key, default=None, cast=None, depends_on_tracing=False):
"DD_DATA_STREAMS_ENABLED", "false", as_bool, depends_on_tracing=True
)
appsec_enabled = _get_env("DD_APPSEC_ENABLED", "false", as_bool)
sca_enabled = _get_env("DD_APPSEC_SCA_ENABLED", "false", as_bool)

is_gov_region = _get_env("AWS_REGION", "", lambda x: x.startswith("us-gov-"))

Expand Down Expand Up @@ -144,3 +139,11 @@ def _reset(self):
"Python Lambda Layer FIPS mode is %s.",
"enabled" if config.fips_mode_enabled else "not enabled",
)


if (
"DD_INSTRUMENTATION_TELEMETRY_ENABLED" not in os.environ
and not config.sca_enabled
and not config.appsec_enabled
):
os.environ["DD_INSTRUMENTATION_TELEMETRY_ENABLED"] = "false"
5 changes: 0 additions & 5 deletions datadog_lambda/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@
logger = logging.getLogger(__name__)

dd_trace_context = None
if config.telemetry_enabled:
# Enable the telemetry client if the user has opted in
from ddtrace.internal.telemetry import telemetry_writer

telemetry_writer.enable()

propagator = HTTPPropagator()

Expand Down
34 changes: 28 additions & 6 deletions tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import importlib
import sys

import pytest

from datadog_lambda.config import config, _get_env, Config
Expand All @@ -14,6 +17,29 @@ def set_env(key, value):
return set_env


def test_config_import_does_not_import_ddtrace(monkeypatch):
import datadog_lambda

with monkeypatch.context() as mp:
for name in list(sys.modules):
if name == "ddtrace" or name.startswith("ddtrace."):
mp.delitem(sys.modules, name, raising=False)

class _BlockDdtrace(importlib.abc.MetaPathFinder):
def find_spec(self, fullname, path=None, target=None):
if fullname == "ddtrace" or fullname.startswith("ddtrace."):
raise ImportError("ddtrace must not be imported during this test")
return None

blocker = _BlockDdtrace()
mp.setattr(sys, "meta_path", [blocker] + sys.meta_path, raising=False)

mp.delattr(datadog_lambda, "config", raising=False)
mp.delitem(sys.modules, "datadog_lambda.config", raising=False)
importlib.invalidate_caches()
importlib.import_module("datadog_lambda.config")


def _test_as_bool(env_key, conf_key, default):
return (
(env_key, conf_key, None, default),
Expand Down Expand Up @@ -72,9 +98,6 @@ def _test_as_list(env_key, conf_key, default):
*_test_as_bool("DD_INTEGRATION_TEST", "integration_test", default=False),
*_test_as_bool("DD_BOTOCORE_ADD_SPAN_POINTERS", "add_span_pointers", default=True),
*_test_as_bool("DD_TRACE_OTEL_ENABLED", "otel_enabled", default=False),
*_test_as_bool(
"DD_INSTRUMENTATION_TELEMETRY_ENABLED", "telemetry_enabled", default=False
),
*_test_as_bool("DD_MERGE_XRAY_TRACES", "merge_xray_traces", default=False),
*_test_as_bool("DD_PROFILING_ENABLED", "profiling_enabled", default=False),
*_test_as_bool("DD_LLMOBS_ENABLED", "llmobs_enabled", default=False),
Expand All @@ -86,6 +109,8 @@ def _test_as_list(env_key, conf_key, default):
),
*_test_as_bool("DD_LOCAL_TEST", "local_test", default=False),
*_test_as_bool("DD_DATA_STREAMS_ENABLED", "data_streams_enabled", default=False),
*_test_as_bool("DD_APPSEC_ENABLED", "appsec_enabled", default=False),
*_test_as_bool("DD_APPSEC_SCA_ENABLED", "sca_enabled", default=False),
*_test_int(
"DD_CAPTURE_LAMBDA_PAYLOAD_MAX_DEPTH", "capture_payload_max_depth", default=10
),
Expand Down Expand Up @@ -143,9 +168,6 @@ def test_config_from_environ(env_key, conf_key, env_val, conf_val, setenv):
"DD_DECODE_AUTHORIZER_CONTEXT", "decode_authorizer_context", default=True
),
*_test_as_bool("DD_DATA_STREAMS_ENABLED", "data_streams_enabled", default=False),
*_test_as_bool(
"DD_INSTRUMENTATION_TELEMETRY_ENABLED", "telemetry_enabled", default=False
),
)


Expand Down
Loading