Skip to content

Commit

Permalink
fix(asm): fix config in api_manager (#7678)
Browse files Browse the repository at this point in the history
- fix config use in api_manager for api security.
- ensure api security global callback is only called once in all
supported frameworks.

No release note necessary as the feature is private and experimental for
now.

This will be covered later by system-tests

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [ ] Title is accurate.
- [ ] No unnecessary changes are introduced.
- [ ] Description motivates each change.
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [ ] Testing strategy adequately addresses listed risk(s).
- [ ] Change is maintainable (easy to change, telemetry, documentation).
- [ ] Release note makes sense to a user of the library.
- [ ] Reviewer has explicitly 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)
- [ ] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [ ] This PR doesn't touch any of that.

---------

Co-authored-by: Alberto Vara <alberto.vara@datadoghq.com>
Co-authored-by: Eric Navarro <eric.navarro@datadoghq.com>
Co-authored-by: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
  • Loading branch information
5 people authored Nov 20, 2023
1 parent 1c9765f commit 80bf6a9
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
4 changes: 2 additions & 2 deletions ddtrace/appsec/_api_security/api_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import sys
from typing import TYPE_CHECKING

from ddtrace import config
from ddtrace._tracing._limits import MAX_SPAN_META_VALUE_LEN
from ddtrace.appsec import _processor as appsec_processor
from ddtrace.appsec._asm_request_context import add_context_callback
Expand All @@ -15,6 +14,7 @@
from ddtrace.internal.logger import get_logger
from ddtrace.internal.metrics import Metrics
from ddtrace.internal.service import Service
from ddtrace.settings.asm import config as asm_config


if TYPE_CHECKING:
Expand Down Expand Up @@ -90,7 +90,7 @@ def _start_service(self):
def _should_collect_schema(self, env):
method = env.waf_addresses.get(SPAN_DATA_NAMES.REQUEST_METHOD)
route = env.waf_addresses.get(SPAN_DATA_NAMES.REQUEST_ROUTE)
sample_rate = config._api_security_sample_rate
sample_rate = asm_config._api_security_sample_rate
# Framework is not fully supported
if not method or not route:
log.debug("unsupported groupkey for api security [method %s] [route %s]", bool(method), bool(route))
Expand Down
7 changes: 5 additions & 2 deletions ddtrace/appsec/_asm_request_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def __init__(self, active: bool = False):
self.callbacks: Dict[str, Any] = {}
self.telemetry: Dict[str, Any] = {}
self.addresses_sent: Set[str] = set()
self.must_call_globals: bool = True


def _get_asm_context() -> ASM_Environment:
Expand Down Expand Up @@ -96,10 +97,11 @@ def unregister(span: Span) -> None:
env = _get_asm_context()
if env.span_asm_context is not None and env.span is span:
env.span_asm_context.__exit__(None, None, None)
elif env.span is span:
elif env.span is span and env.must_call_globals:
# needed for api security flushing information before end of the span
for function in GLOBAL_CALLBACKS.get(_CONTEXT_CALL, []):
function(env)
env.must_call_globals = False


class _DataHandler:
Expand All @@ -125,7 +127,8 @@ def __init__(self):
def finalise(self):
if self.active:
env = self.execution_context.get_item("asm_env")
callbacks = GLOBAL_CALLBACKS.get(_CONTEXT_CALL, [])
callbacks = GLOBAL_CALLBACKS.get(_CONTEXT_CALL, []) if env.must_call_globals else []
env.must_call_globals = False
if env is not None and env.callbacks is not None and env.callbacks.get(_CONTEXT_CALL):
callbacks += env.callbacks.get(_CONTEXT_CALL)
if callbacks:
Expand Down

0 comments on commit 80bf6a9

Please sign in to comment.