Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(tracer): remove asm properties from the tracer class #11791

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
60 changes: 23 additions & 37 deletions ddtrace/_trace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,21 @@ def _default_span_processors_factory(
trace_writer: TraceWriter,
partial_flush_enabled: bool,
partial_flush_min_spans: int,
appsec_enabled: bool,
iast_enabled: bool,
compute_stats_enabled: bool,
single_span_sampling_rules: List[SpanSamplingRule],
agent_url: str,
trace_sampler: BaseSampler,
profiling_span_processor: EndpointCallCounterProcessor,
apm_opt_out: bool = False,
) -> Tuple[List[SpanProcessor], Optional[Any], List[SpanProcessor]]:
# FIXME: type should be AppsecSpanProcessor but we have a cyclic import here
"""Construct the default list of span processors to use."""
trace_processors: List[TraceProcessor] = []
trace_processors += [
PeerServiceProcessor(_ps_config),
BaseServiceProcessor(),
TraceSamplingProcessor(compute_stats_enabled, trace_sampler, single_span_sampling_rules, apm_opt_out),
TraceSamplingProcessor(
compute_stats_enabled, trace_sampler, single_span_sampling_rules, asm_config._apm_opt_out
),
TraceTagsProcessor(),
]
trace_processors += trace_filters
Expand All @@ -131,7 +130,7 @@ def _default_span_processors_factory(
span_processors += [TopLevelSpanProcessor()]

if asm_config._asm_libddwaf_available:
if appsec_enabled:
if asm_config._asm_enabled:
if asm_config._api_security_enabled:
from ddtrace.appsec._api_security.api_manager import APIManager

Expand All @@ -153,7 +152,7 @@ def _default_span_processors_factory(
else:
appsec_processor = None

if iast_enabled:
if asm_config._iast_enabled:
from ddtrace.appsec._iast.processor import AppSecIastSpanProcessor

span_processors.append(AppSecIastSpanProcessor())
Expand Down Expand Up @@ -232,14 +231,8 @@ def __init__(
self.context_provider = context_provider or DefaultContextProvider()
# _user_sampler is the backup in case we need to revert from remote config to local
self._user_sampler: Optional[BaseSampler] = DatadogSampler()
self._asm_enabled = asm_config._asm_enabled
self._iast_enabled = asm_config._iast_enabled
self._appsec_standalone_enabled = asm_config._appsec_standalone_enabled
self._dogstatsd_url = agent.get_stats_url() if dogstatsd_url is None else dogstatsd_url
self._apm_opt_out = self._appsec_standalone_enabled and (
self._asm_enabled or self._iast_enabled or config._sca_enabled
)
if self._apm_opt_out:
if asm_config._apm_opt_out:
self.enabled = False
# Disable compute stats (neither agent or tracer should compute them)
config._trace_compute_stats = False
Expand All @@ -260,30 +253,28 @@ def __init__(
agent_url=self._agent_url,
dogstatsd=get_dogstatsd_client(self._dogstatsd_url),
sync_mode=self._use_sync_mode(),
headers={"Datadog-Client-Computed-Stats": "yes"} if (self._compute_stats or self._apm_opt_out) else {},
report_metrics=not self._apm_opt_out,
headers={"Datadog-Client-Computed-Stats": "yes"}
if (self._compute_stats or asm_config._apm_opt_out)
else {},
report_metrics=not asm_config._apm_opt_out,
response_callback=self._agent_response_callback,
)
self._single_span_sampling_rules: List[SpanSamplingRule] = get_span_sampling_rules()
self._writer: TraceWriter = writer
self._partial_flush_enabled = config._partial_flush_enabled
self._partial_flush_min_spans = config._partial_flush_min_spans
# Direct link to the appsec processor
self._appsec_processor = None
self._endpoint_call_counter_span_processor = EndpointCallCounterProcessor()
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
self._filters,
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
self._asm_enabled,
self._iast_enabled,
self._compute_stats,
self._single_span_sampling_rules,
self._agent_url,
self._sampler,
self._endpoint_call_counter_span_processor,
self._apm_opt_out,
)
if config._data_streams_enabled:
# Inline the import to avoid pulling in ddsketch or protobuf
Expand Down Expand Up @@ -338,7 +329,7 @@ def sampler(self, value):
https://ddtrace.readthedocs.io/en/stable/configuration.html#DD_TRACE_SAMPLING_RULES""",
category=DDTraceDeprecationWarning,
)
if self._apm_opt_out:
if asm_config._apm_opt_out:
log.warning("Cannot set a custom sampler with Standalone ASM mode")
return
self._sampler = value
Expand Down Expand Up @@ -410,7 +401,7 @@ def get_log_correlation_context(self, active: Optional[Union[Context, Span]] = N
span id of the current active span, as well as the configured service, version, and environment names.
If there is no active span, a dictionary with an empty string for each value will be returned.
"""
if active is None and (self.enabled or self._apm_opt_out):
if active is None and (self.enabled or asm_config._apm_opt_out):
active = self.context_provider.active()

if isinstance(active, Span) and active.service:
Expand Down Expand Up @@ -492,16 +483,15 @@ def configure(
self._partial_flush_min_spans = partial_flush_min_spans

if appsec_enabled is not None:
self._asm_enabled = asm_config._asm_enabled = appsec_enabled
asm_config._asm_enabled = appsec_enabled

if iast_enabled is not None:
self._iast_enabled = asm_config._iast_enabled = iast_enabled
asm_config._iast_enabled = iast_enabled

if appsec_standalone_enabled is not None:
self._appsec_standalone_enabled = asm_config._appsec_standalone_enabled = appsec_standalone_enabled
asm_config._appsec_standalone_enabled = appsec_standalone_enabled

if self._appsec_standalone_enabled and (self._asm_enabled or self._iast_enabled or config._sca_enabled):
self._apm_opt_out = True
if asm_config._apm_opt_out:
self.enabled = False
# Disable compute stats (neither agent or tracer should compute them)
config._trace_compute_stats = False
Expand Down Expand Up @@ -560,7 +550,7 @@ def configure(
if writer is not None:
self._writer = writer
elif any(x is not None for x in [new_url, api_version, sampler, dogstatsd_url, appsec_enabled]):
if self._asm_enabled:
if asm_config._asm_enabled:
api_version = "v0.4"
self._writer = AgentWriter(
self._agent_url,
Expand All @@ -569,9 +559,11 @@ def configure(
api_version=api_version,
# if apm opt out, neither agent or tracer should compute the stats
headers=(
{"Datadog-Client-Computed-Stats": "yes"} if (compute_stats_enabled or self._apm_opt_out) else {}
{"Datadog-Client-Computed-Stats": "yes"}
if (compute_stats_enabled or asm_config._apm_opt_out)
else {}
),
report_metrics=not self._apm_opt_out,
report_metrics=not asm_config._apm_opt_out,
response_callback=self._agent_response_callback,
)
elif writer is None and isinstance(self._writer, LogWriter):
Expand Down Expand Up @@ -604,14 +596,11 @@ def configure(
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
self._asm_enabled,
self._iast_enabled,
self._compute_stats,
self._single_span_sampling_rules,
self._agent_url,
self._sampler,
self._endpoint_call_counter_span_processor,
self._apm_opt_out,
)

if context_provider is not None:
Expand Down Expand Up @@ -670,14 +659,11 @@ def _child_after_fork(self):
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
self._asm_enabled,
self._iast_enabled,
self._compute_stats,
self._single_span_sampling_rules,
self._agent_url,
self._sampler,
self._endpoint_call_counter_span_processor,
self._apm_opt_out,
)

self._new_process = True
Expand Down Expand Up @@ -862,7 +848,7 @@ def _start_span(
self._services.add(service)

# Only call span processors if the tracer is enabled (even if APM opted out)
if self.enabled or self._apm_opt_out:
if self.enabled or asm_config._apm_opt_out:
for p in chain(self._span_processors, SpanProcessor.__processors__, self._deferred_processors):
p.on_span_start(span)
self._hooks.emit(self.__class__.start_span, span)
Expand All @@ -879,7 +865,7 @@ def _on_span_finish(self, span: Span) -> None:
log.debug("span %r closing after its parent %r, this is an error when not using async", span, span._parent)

# Only call span processors if the tracer is enabled (even if APM opted out)
if self.enabled or self._apm_opt_out:
if self.enabled or asm_config._apm_opt_out:
for p in chain(self._span_processors, SpanProcessor.__processors__, self._deferred_processors):
p.on_span_finish(span)

Expand Down
12 changes: 3 additions & 9 deletions ddtrace/appsec/_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ def enable_appsec_rc(test_tracer: Optional[Tracer] = None) -> None:

Parameters `test_tracer` and `start_subscribers` are needed for testing purposes
"""
# Import tracer here to avoid a circular import
if test_tracer is None:
from ddtrace import tracer
else:
tracer = test_tracer

log.debug("[%s][P: %s] Register ASM Remote Config Callback", os.getpid(), os.getppid())
asm_callback = (
remoteconfig_poller.get_registered(PRODUCTS.ASM_FEATURES)
Expand All @@ -68,7 +62,7 @@ def enable_appsec_rc(test_tracer: Optional[Tracer] = None) -> None:
if _asm_feature_is_required():
remoteconfig_poller.register(PRODUCTS.ASM_FEATURES, asm_callback)

if tracer._asm_enabled and asm_config._asm_static_rule_file is None:
if asm_config._asm_enabled and asm_config._asm_static_rule_file is None:
remoteconfig_poller.register(PRODUCTS.ASM_DATA, asm_callback) # IP Blocking
remoteconfig_poller.register(PRODUCTS.ASM, asm_callback) # Exclusion Filters & Custom Rules
remoteconfig_poller.register(PRODUCTS.ASM_DD, asm_callback) # DD Rules
Expand Down Expand Up @@ -226,12 +220,12 @@ def _appsec_1click_activation(features: Mapping[str, Any], test_tracer: Optional
)

if rc_asm_enabled:
if not tracer._asm_enabled:
if not asm_config._asm_enabled:
tracer.configure(appsec_enabled=True)
else:
asm_config._asm_enabled = True
else:
if tracer._asm_enabled:
if asm_config._asm_enabled:
tracer.configure(appsec_enabled=False)
else:
asm_config._asm_enabled = False
Expand Down
3 changes: 2 additions & 1 deletion ddtrace/contrib/internal/requests/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from ddtrace.internal.schema.span_attribute_schema import SpanDirection
from ddtrace.internal.utils import get_argument_value
from ddtrace.propagation.http import HTTPPropagator
from ddtrace.settings.asm import config as asm_config


log = get_logger(__name__)
Expand Down Expand Up @@ -59,7 +60,7 @@ def _wrap_send(func, instance, args, kwargs):
tracer = getattr(instance, "datadog_tracer", ddtrace.tracer)

# skip if tracing is not enabled
if not tracer.enabled and not tracer._apm_opt_out:
if not tracer.enabled and not asm_config._apm_opt_out:
return func(*args, **kwargs)

request = get_argument_value(args, kwargs, 0, "request")
Expand Down
5 changes: 4 additions & 1 deletion ddtrace/pin.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ def override(
def enabled(self):
# type: () -> bool
"""Return true if this pin's tracer is enabled."""
return bool(self.tracer) and (self.tracer.enabled or self.tracer._apm_opt_out)
# inline to avoid circular imports
from ddtrace.settings.asm import config as asm_config

return bool(self.tracer) and (self.tracer.enabled or asm_config._apm_opt_out)
mabdinur marked this conversation as resolved.
Show resolved Hide resolved

def onto(self, obj, send=True):
# type: (Any, bool) -> None
Expand Down
6 changes: 6 additions & 0 deletions ddtrace/settings/asm.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@ def _eval_asm_can_be_enabled(self):
def _api_security_feature_active(self) -> bool:
return self._asm_libddwaf_available and self._asm_enabled and self._api_security_enabled

@property
def _apm_opt_out(self) -> bool:
return (
self._asm_enabled or self._iast_enabled or tracer_config._sca_enabled is True
) and self._appsec_standalone_enabled

@property
def _user_event_mode(self) -> str:
if self._asm_enabled and self._auto_user_instrumentation_enabled:
Expand Down
2 changes: 0 additions & 2 deletions tests/appsec/appsec/test_asm_standalone.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@ def test_appsec_standalone_apm_enabled_metric(tracer_appsec_standalone):
or args.get("iast_enabled", None)
or args.get("DD_APPSEC_SCA_ENABLED", "0") == "1"
):
assert tracer._apm_opt_out is True
assert span.get_metric("_dd.apm.enabled") == 0.0
else:
assert tracer._apm_opt_out is False
assert span.get_metric("_dd.apm.enabled") is None
2 changes: 0 additions & 2 deletions tests/appsec/contrib_appsec/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ def check_rules_triggered(self, rule_id: List[str], root_span):
assert result == rule_id, f"result={result}, expected={rule_id}"

def update_tracer(self, interface):
interface.tracer._asm_enabled = asm_config._asm_enabled
interface.tracer._iast_enabled = asm_config._iast_enabled
interface.tracer.configure(api_version="v0.4")
assert asm_config._asm_libddwaf_available
# Only for tests diagnostics
Expand Down
3 changes: 0 additions & 3 deletions tests/appsec/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from ddtrace._trace.span import Span
from ddtrace.ext import SpanTypes
import ddtrace.internal.core as core
from ddtrace.settings.asm import config as asm_config
from tests.utils import override_global_config


Expand Down Expand Up @@ -35,8 +34,6 @@ def asm_context(
with override_global_config(config) if config else contextlib.nullcontext():
if tracer is None:
tracer = default_tracer
if asm_config._asm_enabled:
tracer._asm_enabled = True
if config:
tracer.configure(api_version="v0.4")

Expand Down
2 changes: 0 additions & 2 deletions tests/contrib/django/test_django_appsec.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ def _aux_appsec_get_root_span(
):
if cookies is None:
cookies = {}
tracer._asm_enabled = asm_config._asm_enabled
tracer._iast_enabled = asm_config._iast_enabled
# Hack: need to pass an argument to configure so that the processors are recreated
tracer.configure(api_version="v0.4")
# Set cookies
Expand Down
3 changes: 0 additions & 3 deletions tests/contrib/django/test_django_appsec_iast.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from ddtrace.appsec._iast.constants import VULN_INSECURE_COOKIE
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
from ddtrace.internal.compat import urlencode
from ddtrace.settings.asm import config as asm_config
from tests.appsec.iast.iast_utils import get_line_and_hash
from tests.utils import override_env
from tests.utils import override_global_config
Expand Down Expand Up @@ -66,8 +65,6 @@ def _aux_appsec_get_root_span(
):
if cookies is None:
cookies = {}
tracer._asm_enabled = asm_config._asm_enabled
tracer._iast_enabled = asm_config._iast_enabled
# Hack: need to pass an argument to configure so that the processors are recreated
tracer.configure(api_version="v0.4")
# Set cookies
Expand Down
1 change: 0 additions & 1 deletion tests/contrib/djangorestframework/test_appsec.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
@pytest.mark.skipif(django.VERSION < (1, 10), reason="requires django version >= 1.10")
def test_djangorest_request_body_urlencoded(client, test_spans, tracer):
with override_global_config(dict(_asm_enabled=True)):
tracer._asm_enabled = True
# Hack: need to pass an argument to configure so that the processors are recreated
tracer.configure(api_version="v0.4")
payload = urlencode({"mytestingbody_key": "mytestingbody_value"})
Expand Down
1 change: 0 additions & 1 deletion tests/contrib/fastapi/test_fastapi_appsec.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@


def _aux_appsec_prepare_tracer(tracer, asm_enabled=True):
tracer._asm_enabled = asm_enabled
# Hack: need to pass an argument to configure so that the processors are recreated
tracer.configure(api_version="v0.4")

Expand Down
1 change: 0 additions & 1 deletion tests/contrib/fastapi/test_fastapi_appsec_iast.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ def _aux_appsec_prepare_tracer(tracer):
patch_sqlite_sqli()
oce.reconfigure()

tracer._iast_enabled = True
# Hack: need to pass an argument to configure so that the processors are recreated
tracer.configure(api_version="v0.4")

Expand Down
1 change: 0 additions & 1 deletion tests/contrib/flask/test_flask_appsec.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def setUp(self):
patch()

def _aux_appsec_prepare_tracer(self, appsec_enabled=True):
self.tracer._asm_enabled = appsec_enabled
# Hack: need to pass an argument to configure so that the processors are recreated
self.tracer.configure(api_version="v0.4")

Expand Down
Loading
Loading