diff --git a/.gitignore b/.gitignore index 149fc092..588c6b0a 100644 --- a/.gitignore +++ b/.gitignore @@ -106,3 +106,4 @@ uv.lock # Sandbox sandbox/ .bob/ +.serena/ \ No newline at end of file diff --git a/src/instana/instrumentation/aiohttp/client.py b/src/instana/instrumentation/aiohttp/client.py index 7cb3f3ab..10a18cab 100644 --- a/src/instana/instrumentation/aiohttp/client.py +++ b/src/instana/instrumentation/aiohttp/client.py @@ -2,21 +2,23 @@ # (c) Copyright Instana Inc. 2019 -from types import SimpleNamespace -from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Tuple - -import wrapt -from opentelemetry.semconv.trace import SpanAttributes - -from instana.log import logger -from instana.propagators.format import Format -from instana.singletons import agent -from instana.util.secrets import strip_secrets_from_query -from instana.util.traceutils import extract_custom_headers, get_tracer_tuple - try: + import http.client + from collections.abc import Awaitable, Callable + from types import SimpleNamespace + from typing import TYPE_CHECKING, Any + import aiohttp + import wrapt from opentelemetry.context import get_current + from opentelemetry.semconv.trace import SpanAttributes + + from instana.log import logger + from instana.propagators.format import Format + from instana.singletons import agent + from instana.util.http import should_mark_http_exit_as_error + from instana.util.secrets import strip_secrets_from_query + from instana.util.traceutils import extract_custom_headers, get_tracer_tuple if TYPE_CHECKING: from aiohttp.client import ClientSession @@ -65,8 +67,10 @@ async def stan_request_end( extract_custom_headers(span, params.response.headers) - if params.response.status >= 500: - span.mark_as_errored({"http.error": params.response.reason}) + if should_mark_http_exit_as_error(params.response.status, agent.options): + status = params.response.status + reason = http.client.responses.get(status, params.response.reason) + span.mark_as_errored({"http.error": f"{status} {reason}"}) if span.is_recording(): span.end() @@ -92,8 +96,8 @@ async def stan_request_exception( def init_with_instana( wrapped: Callable[..., Awaitable["ClientSession"]], instance: aiohttp.client.ClientSession, - args: Tuple[int, str, Tuple[object, ...]], - kwargs: Dict[str, Any], + args: tuple[int, str, tuple[object, ...]], + kwargs: dict[str, Any], ) -> object: instana_trace_config = aiohttp.TraceConfig() instana_trace_config.on_request_start.append(stan_request_start) diff --git a/src/instana/instrumentation/httpx.py b/src/instana/instrumentation/httpx.py index 96beaeeb..675b10a9 100644 --- a/src/instana/instrumentation/httpx.py +++ b/src/instana/instrumentation/httpx.py @@ -1,7 +1,7 @@ # (c) Copyright IBM Corp. 2025 try: - from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, Tuple + from typing import TYPE_CHECKING, Any, Callable, Optional import httpx import wrapt @@ -12,6 +12,7 @@ from instana.log import logger from instana.propagators.format import Format from instana.singletons import agent + from instana.util.http import should_mark_http_exit_as_error from instana.util.secrets import strip_secrets_from_query from instana.util.traceutils import extract_custom_headers, get_tracer_tuple @@ -50,7 +51,7 @@ def _set_request_span_attributes( def _set_response_span_attributes( span: "InstanaSpan", - response: Optional[httpx.Response] = None, + response: "Optional[httpx.Response]" = None, ) -> None: try: if response.headers: @@ -58,8 +59,9 @@ def _set_response_span_attributes( status_code = response.status_code span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - if status_code >= 500: - span.mark_as_errored() + if should_mark_http_exit_as_error(status_code, agent.options): + error_msg = f"{status_code} {response.reason_phrase}" + span.mark_as_errored({"http.error": error_msg}) except Exception: logger.debug("httpx _set_request_span_attributes error: ", exc_info=True) @@ -67,8 +69,8 @@ def _set_response_span_attributes( def handle_request_with_instana( wrapped: Callable[..., "httpx.HTTPTransport.handle_request"], instance: httpx.HTTPTransport, - args: Tuple[int, str, Tuple[Any, ...]], - kwargs: Dict[str, Any], + args: tuple[int, str, tuple[Any, ...]], + kwargs: dict[str, Any], ) -> httpx.Response: tracer, _, _ = get_tracer_tuple() # If we're not tracing, just return @@ -102,8 +104,8 @@ def handle_request_with_instana( async def handle_async_request_with_instana( wrapped: Callable[..., "httpx.AsyncHTTPTransport.handle_async_request"], instance: httpx.AsyncHTTPTransport, - args: Tuple[int, str, Tuple[Any, ...]], - kwargs: Dict[str, Any], + args: tuple[int, str, tuple[Any, ...]], + kwargs: dict[str, Any], ) -> httpx.Response: tracer, _, _ = get_tracer_tuple() # If we're not tracing, just return diff --git a/src/instana/instrumentation/tornado/client.py b/src/instana/instrumentation/tornado/client.py index 5a1cc314..1251529f 100644 --- a/src/instana/instrumentation/tornado/client.py +++ b/src/instana/instrumentation/tornado/client.py @@ -4,7 +4,7 @@ try: import functools - from typing import TYPE_CHECKING, Any, Callable, Dict, Tuple + from typing import TYPE_CHECKING, Any, Callable import tornado import wrapt @@ -23,6 +23,7 @@ from instana.propagators.format import Format from instana.singletons import agent, get_tracer from instana.span.span import get_current_span + from instana.util.http import should_mark_http_exit_as_error from instana.util.secrets import strip_secrets_from_query from instana.util.traceutils import extract_custom_headers @@ -30,8 +31,8 @@ def fetch_with_instana( wrapped: Callable[..., object], instance: "AsyncHTTPClient", - argv: Tuple[object, ...], - kwargs: Dict[str, Any], + argv: tuple[object, ...], + kwargs: dict[str, Any], ) -> "Future": try: parent_span = get_current_span() @@ -47,13 +48,12 @@ def fetch_with_instana( # To modify request headers, we have to preemptively create an HTTPRequest object if a # URL string was passed. if not isinstance(request, tornado.httpclient.HTTPRequest): + # "callback" and "raise_error" are fetch()-level kwargs, not + # HTTPRequest constructor params — extract them first so they + # are not forwarded to HTTPRequest.__init__. + fetch_only_params = ("callback", "raise_error") + new_kwargs = {p: kwargs.pop(p) for p in fetch_only_params if p in kwargs} request = tornado.httpclient.HTTPRequest(url=request, **kwargs) - - new_kwargs = {} - for param in ("callback", "raise_error"): - # if not in instead and pop - if param in kwargs: - new_kwargs[param] = kwargs.pop(param) kwargs = new_kwargs parent_context = get_current() @@ -89,8 +89,10 @@ def finish_tracing(future: "Future", span: "InstanaSpan") -> None: try: response = future.result() span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, response.code) - extract_custom_headers(span, response.headers) + if should_mark_http_exit_as_error(response.code, agent.options): + error_msg = f"{response.code} {response.reason}" + span.mark_as_errored({"http.error": error_msg}) except tornado.httpclient.HTTPClientError as e: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, e.code) span.record_exception(e) diff --git a/src/instana/instrumentation/tornado/server.py b/src/instana/instrumentation/tornado/server.py index 55ce828a..773178f7 100644 --- a/src/instana/instrumentation/tornado/server.py +++ b/src/instana/instrumentation/tornado/server.py @@ -77,6 +77,7 @@ def set_default_headers_with_instana( span = instance.request._instana tracer = get_tracer() tracer.inject(span.context, Format.HTTP_HEADERS, instance._headers) + return wrapped(*argv, **kwargs) @wrapt.patch_function_wrapper("tornado.web", "RequestHandler.on_finish") def on_finish_with_instana( diff --git a/src/instana/instrumentation/twisted/client.py b/src/instana/instrumentation/twisted/client.py index 802239e0..1669b875 100644 --- a/src/instana/instrumentation/twisted/client.py +++ b/src/instana/instrumentation/twisted/client.py @@ -19,6 +19,7 @@ from instana.propagators.format import Format from instana.singletons import agent, get_tracer from instana.span.span import get_current_span + from instana.util.http import should_mark_http_exit_as_error from instana.util.secrets import strip_secrets_from_query from instana.util.traceutils import extract_custom_headers @@ -133,9 +134,10 @@ def finish_tracing( } extract_custom_headers(span, headers_dict) - if status_code >= 500: + if should_mark_http_exit_as_error(status_code, agent.options): + phrase = result.phrase.decode("latin-1") span.mark_as_errored({ - "http.error": result.phrase.decode("latin-1") + "http.error": f"{status_code} {phrase}" }) except Exception: logger.debug("twisted client finish_tracing", exc_info=True) diff --git a/src/instana/instrumentation/twisted/server.py b/src/instana/instrumentation/twisted/server.py index 2c8d2d70..f70fc00d 100644 --- a/src/instana/instrumentation/twisted/server.py +++ b/src/instana/instrumentation/twisted/server.py @@ -62,10 +62,20 @@ def render_with_instana( span = tracer.start_span( "twisted-server", context=parent_context) - # Set span as current so downstream code - # (e.g. twisted-client) can find it during the synchronous - # wrapped() call. We detach unconditionally in the finally - # block below once wrapped() has returned. + # Set span as current so that any async work started during + # wrapped() (e.g. outgoing Agent.request Deferreds) can find + # this span as their parent after the event loop resumes. + # + # IMPORTANT: we do NOT detach the token here in the `finally` + # block. Twisted's render() returns NOT_DONE_YET for async + # handlers, and the event loop only fires pending Deferreds + # after render() has returned — by which point a `finally` + # detach would have already removed the context, leaving + # downstream spans (e.g. twisted-client) with no active parent. + # + # Instead the token is stored on the request object and detached + # inside finish_tracing(), which is called by notifyFinish() only + # after the full async response lifecycle has completed. ctx = trace.set_span_in_context(span) token = context.attach(ctx) @@ -119,8 +129,10 @@ def render_with_instana( for key, value in response_headers.items(): request.setHeader(key.encode("latin-1"), value.encode("utf-8")) - # Store span on request for later retrieval + # Store span and context token on the request so finish_tracing + # can detach the token after the full async lifecycle completes. request._instana = span + request._instana_token = token request._instana_finished = False finish_deferred = request.notifyFinish() @@ -128,12 +140,13 @@ def render_with_instana( return wrapped(*argv, **kwargs) except Exception: + # On instrumentation error detach immediately (we never reach + # finish_tracing in this path) and fall through to the bare call. + if token is not None: + context.detach(token) if span is not None and span.is_recording(): span.end() logger.debug("twisted server render_with_instana", exc_info=True) - finally: - if token is not None: - context.detach(token) return wrapped(*argv, **kwargs) @@ -146,6 +159,7 @@ def finish_tracing( request._instana_finished = True span = request._instana + token = getattr(request, "_instana_token", None) try: status_code = request.code if isinstance(status_code, int): @@ -159,12 +173,19 @@ def finish_tracing( extract_custom_headers(span, response_hdrs) if isinstance(status_code, int) and status_code >= 500: + phrase = request.code_message.decode("latin-1") span.mark_as_errored({ - "http.error": request.code_message.decode("latin-1") + "http.error": f"{status_code} {phrase}" }) except Exception: logger.debug("twisted server finish_tracing", exc_info=True) finally: + # Detach the OTel context token here — after the full async + # response lifecycle — instead of in render_with_instana's + # finally block. This ensures the server span remains the + # active context for any Deferreds started during render(). + if token is not None: + context.detach(token) if span.is_recording(): span.end() diff --git a/src/instana/instrumentation/urllib3.py b/src/instana/instrumentation/urllib3.py index 8ed9a976..1f57521c 100644 --- a/src/instana/instrumentation/urllib3.py +++ b/src/instana/instrumentation/urllib3.py @@ -2,22 +2,23 @@ # (c) Copyright Instana Inc. 2017 -from typing import TYPE_CHECKING, Any, Callable, Dict, Tuple, Union +try: + from typing import TYPE_CHECKING, Any, Callable, Union -import wrapt -from opentelemetry.context import get_current -from opentelemetry.semconv.trace import SpanAttributes + import wrapt + from opentelemetry.context import get_current + from opentelemetry.semconv.trace import SpanAttributes -from instana.log import logger -from instana.propagators.format import Format -from instana.singletons import agent -from instana.util.secrets import strip_secrets_from_query -from instana.util.traceutils import extract_custom_headers, get_tracer_tuple + from instana.log import logger + from instana.propagators.format import Format + from instana.singletons import agent + from instana.util.http import should_mark_http_exit_as_error + from instana.util.secrets import strip_secrets_from_query + from instana.util.traceutils import extract_custom_headers, get_tracer_tuple -if TYPE_CHECKING: - from instana.span.span import InstanaSpan + if TYPE_CHECKING: + from instana.span.span import InstanaSpan -try: import urllib3 def _collect_kvs( @@ -25,9 +26,9 @@ def _collect_kvs( urllib3.connectionpool.HTTPConnectionPool, urllib3.connectionpool.HTTPSConnectionPool, ], - args: Tuple[int, str, Tuple[Any, ...]], - kwargs: Dict[str, Any], - ) -> Dict[str, Any]: + args: tuple[int, str, tuple[Any, ...]], + kwargs: dict[str, Any], + ) -> dict[str, Any]: kvs = dict() try: kvs["host"] = instance.host @@ -74,8 +75,9 @@ def collect_response( extract_custom_headers(span, response.headers) - if response.status >= 500: - span.mark_as_errored() + if should_mark_http_exit_as_error(response.status, agent.options): + error_msg = f"{response.status} {response.reason}" + span.mark_as_errored({"http.error": error_msg}) except Exception: logger.debug("urllib3 collect_response error: ", exc_info=True) @@ -88,8 +90,8 @@ def urlopen_with_instana( urllib3.connectionpool.HTTPConnectionPool, urllib3.connectionpool.HTTPSConnectionPool, ], - args: Tuple[int, str, Tuple[Any, ...]], - kwargs: Dict[str, Any], + args: tuple[int, str, tuple[Any, ...]], + kwargs: dict[str, Any], ) -> urllib3.response.HTTPResponse: tracer, _, span_name = get_tracer_tuple() diff --git a/src/instana/options.py b/src/instana/options.py index 6085cd9f..2ca532b8 100644 --- a/src/instana/options.py +++ b/src/instana/options.py @@ -25,6 +25,7 @@ get_disable_trace_configurations_from_env, get_disable_trace_configurations_from_local, get_disable_trace_configurations_from_yaml, + get_http_exit_classification_from_yaml, get_stack_trace_config_from_yaml, is_truthy, parse_filter_rules, @@ -55,6 +56,10 @@ def __init__(self, **kwds: dict[str, Any]) -> None: # enabled_spans lists all categories and types that should be enabled, preceding disabled_spans self.enabled_spans = [] + # HTTP exit span 4xx error classification (opt-in) + self.http_exit_classify_all_4xx_as_errors = False + self.http_exit_classify_as_errors = [] + # Stack trace configuration - global defaults self.stack_trace_level = "all" # Options: "all", "error", "none" self.stack_trace_length = 30 # Default: 30, recommended range: 10-40 @@ -120,10 +125,47 @@ def set_trace_configurations(self) -> None: os.environ["INSTANA_ASYNCIO_TASK_CONTEXT_PROPAGATION"] ) + self.set_http_exit_classification_configurations() self.set_disable_trace_configurations() self.set_stack_trace_configurations() self.set_span_filter_configurations() + def set_http_exit_classification_configurations(self) -> None: + """Set HTTP exit span 4xx classification from environment variables or config file.""" + env_classify_as_errors = os.environ.get( + "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS", None + ) + if env_classify_as_errors is not None: + codes = [] + for part in env_classify_as_errors.split(","): + part = part.strip() + if part.isdigit(): + code = int(part) + if 400 <= code <= 499: + codes.append(code) + else: + logger.warning( + "Ignoring out-of-range value in" + " INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS:" + f" {code}, must be 400-499" + ) + elif part: + logger.warning( + "Ignoring non-integer value in" + f" INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS: {part}" + ) + if codes: + self.http_exit_classify_as_errors = codes + elif "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS" in os.environ: + self.http_exit_classify_all_4xx_as_errors = is_truthy( + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] + ) + logger.debug( + "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS = True" + ) + elif "INSTANA_CONFIG_PATH" in os.environ: + self.http_exit_classify_all_4xx_as_errors, self.http_exit_classify_as_errors = get_http_exit_classification_from_yaml() + def _add_instana_agent_span_filter(self) -> None: """Add Instana agent span filter to exclude internal spans.""" if "exclude" not in self.span_filters: @@ -417,6 +459,10 @@ def set_tracing(self, tracing: dict[str, Any]) -> None: # Handle stack trace configuration from agent config self.set_stack_trace_from_agent(tracing) + # HTTP exit 4xx classification — only apply if env var didn't already set it + if "http" in tracing and not self._has_env_http_exit_classification(): + self._apply_agent_http_exit_classification(tracing["http"]) + def _apply_agent_filter_config(self, filter_config: dict[str, Any]) -> None: """Apply span filter rules from agent config.""" parsed = parse_filter_rules(filter_config) @@ -445,6 +491,46 @@ def _apply_agent_kafka_config( "Binary header format for Kafka is deprecated. Please use string header format." ) + def _has_env_http_exit_classification(self) -> bool: + """Return True if env vars have already configured HTTP exit 4xx classification.""" + return ( + "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS" in os.environ + or "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS" in os.environ + ) + + def _apply_agent_http_exit_classification( + self, http_config: dict[str, Any] + ) -> None: + """Apply HTTP exit 4xx classification from agent config (lowest priority).""" + exit_cfg = http_config.get("exit", {}) + if not isinstance(exit_cfg, dict): + return + + classify_as_errors = exit_cfg.get("classify-as-errors") + if classify_as_errors is not None: + codes = [] + for code in classify_as_errors: + if isinstance(code, int) and 400 <= code <= 499: + codes.append(code) + else: + logger.warning( + "Ignoring invalid value in agent config" + f" tracing.http.exit.classify-as-errors: {code}" + ) + if codes: + self.http_exit_classify_as_errors = codes + return + + classify_all = exit_cfg.get("classify-all-4xx-as-errors") + if classify_all is not None: + if isinstance(classify_all, bool): + self.http_exit_classify_all_4xx_as_errors = classify_all + else: + logger.warning( + "Ignoring non-boolean value in agent config" + f" tracing.http.exit.classify-all-4xx-as-errors: {classify_all}" + ) + def _has_high_priority_span_filter_source(self) -> bool: """Return True if a higher-priority span filter source (env var, YAML, or in-code config) has already been configured, in which case the agent-provided filter should be ignored.""" diff --git a/src/instana/util/config.py b/src/instana/util/config.py index 5377c89c..0bd5b0eb 100644 --- a/src/instana/util/config.py +++ b/src/instana/util/config.py @@ -583,4 +583,50 @@ def get_stack_trace_config_from_yaml() -> Tuple[ return level, length, tech_config +def get_http_exit_classification_from_yaml() -> Tuple[bool, List[int]]: + """ + Get HTTP exit 4xx classification configuration from the YAML file specified + by INSTANA_CONFIG_PATH. + + Returns: + Tuple of (classify_all_4xx, classify_as_errors) where: + - classify_all_4xx: True if all 4xx responses should be errors + - classify_as_errors: List of specific 4xx codes to treat as errors + """ + config_reader = ConfigReader(os.environ.get("INSTANA_CONFIG_PATH", "")) + + root_key = get_tracing_root_key(config_reader.data) + if not root_key: + return False, [] + + http_cfg = config_reader.data[root_key].get("http", {}) + exit_cfg = http_cfg.get("exit", {}) if isinstance(http_cfg, dict) else {} + if not isinstance(exit_cfg, dict): + return False, [] + + classify_as_errors = exit_cfg.get("classify-as-errors") + if classify_as_errors is not None: + codes = [] + for code in classify_as_errors: + if isinstance(code, int) and 400 <= code <= 499: + codes.append(code) + else: + logger.warning( + "Ignoring invalid value in YAML config" + f" tracing.http.exit.classify-as-errors: {code}" + ) + return False, codes + + classify_all = exit_cfg.get("classify-all-4xx-as-errors") + if isinstance(classify_all, bool): + return classify_all, [] + elif classify_all is not None: + logger.warning( + "Ignoring non-boolean value in YAML config" + f" tracing.http.exit.classify-all-4xx-as-errors: {classify_all}" + ) + + return False, [] + + # Made with Bob diff --git a/src/instana/util/http.py b/src/instana/util/http.py new file mode 100644 index 00000000..0a16759d --- /dev/null +++ b/src/instana/util/http.py @@ -0,0 +1,30 @@ +# (C) Copyright IBM Corp. 2026. + +"""HTTP utility helpers for tracer instrumentation.""" + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from instana.options import BaseOptions + + +def should_mark_http_exit_as_error(status_code: int, opts: "BaseOptions") -> bool: + """Return True if an HTTP exit span with *status_code* should be marked as errored. + + Rules (in priority order): + 1. status >= 500 → always an error. + 2. 400 <= status <= 499 and ``opts.http_exit_classify_as_errors`` is non-empty + → error only if *status_code* is in that list. + 3. 400 <= status <= 499 and ``opts.http_exit_classify_all_4xx_as_errors`` is True + → error for every 4xx code. + 4. Otherwise → not an error. + + Entry (server) spans are never passed here; this function is for exit spans only. + """ + if status_code >= 500: + return True + if 400 <= status_code <= 499: + if opts.http_exit_classify_as_errors: + return status_code in opts.http_exit_classify_as_errors + return opts.http_exit_classify_all_4xx_as_errors + return False diff --git a/tests/apps/twisted_server/app.py b/tests/apps/twisted_server/app.py index 1788f67a..6b95c993 100644 --- a/tests/apps/twisted_server/app.py +++ b/tests/apps/twisted_server/app.py @@ -40,6 +40,20 @@ def render_GET(self, request: Request) -> bytes: return b"Not Found" +class R4xxResource(Resource): + """Serves any 4xx status requested as the path segment, e.g. /400, /401.""" + + isLeaf = True + + def __init__(self, code: int) -> None: + super().__init__() + self._code = code + + def render_GET(self, request: Request) -> bytes: + request.setResponseCode(self._code) + return f"{self._code}".encode() + + class R500Resource(Resource): isLeaf = True @@ -101,6 +115,13 @@ def getChild(self, path: bytes, request: Request) -> Resource: return R301Resource() if path == b"404": return R404Resource() + # Generic 4xx endpoints: /400, /401, /403, /405, etc. + try: + code = int(path) + if 400 <= code <= 499: + return R4xxResource(code) + except (ValueError, TypeError): + pass if path == b"500": return R500Resource() if path == b"response_headers": diff --git a/tests/clients/test_4xx_classification.py b/tests/clients/test_4xx_classification.py new file mode 100644 index 00000000..bcc49f97 --- /dev/null +++ b/tests/clients/test_4xx_classification.py @@ -0,0 +1,602 @@ +# (c) Copyright IBM Corp. 2026 + +""" +Integration tests for HTTP exit span 4xx error classification. + +Tests cover the three configuration modes against real server spans for: + - urllib3 + - httpx + - aiohttp client + - tornado client + - twisted client + +For every HTTP client the test matrix is: + + 1. default (opt-in off) → exit ec=0, entry ec=0 for 4xx + 2. classify_all=True → exit ec=1, entry ec=0 for 4xx + 3. classify_codes=[401] → exit ec=1 for 401, ec=0 for 404; entry never changed + +Server apps used: + - Flask (urllib3, httpx tests) + - aiohttp server (aiohttp client tests) + - tornado server (tornado client tests) + - twisted server (twisted client tests) + +All tests clean up options.http_exit_classify_* in teardown. +""" + +import asyncio +import threading +import time +from collections.abc import Generator +from typing import Optional # noqa: UP035 — keep typing import separate for clarity + +import aiohttp +import httpx +import pytest +import tornado +import tornado.ioloop +import urllib3 +from tornado.httpclient import AsyncHTTPClient +from twisted.internet import reactor +from twisted.web.client import Agent +from twisted.web.http_headers import Headers + +import tests.apps.aiohttp_app # noqa: F401 — starts aiohttp server +import tests.apps.flask_app # noqa: F401 — starts Flask server +import tests.apps.tornado_server # noqa: F401 — starts tornado server +import tests.apps.twisted_server # noqa: F401 — starts twisted server +from instana.singletons import agent, get_tracer +from tests.helpers import get_first_span_by_name, testenv + +# --------------------------------------------------------------------------- +# shared fixture +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _reset_4xx_opts() -> Generator[None, None, None]: + """Ensure 4xx classification options are clean before and after every test.""" + agent.options.http_exit_classify_all_4xx_as_errors = False + agent.options.http_exit_classify_as_errors = [] + yield + agent.options.http_exit_classify_all_4xx_as_errors = False + agent.options.http_exit_classify_as_errors = [] + + +# --------------------------------------------------------------------------- +# urllib3 +# --------------------------------------------------------------------------- + + +class TestUrllib34xxClassification: + @pytest.fixture(autouse=True) + def _setup(self) -> Generator[None, None, None]: + self.tracer = get_tracer() + self.recorder = self.tracer.span_processor + self.recorder.clear_spans() + self.http = urllib3.PoolManager() + yield + + def test_default_400_not_error(self) -> None: + """With default config, 400 exit span must NOT be errored.""" + with self.tracer.start_as_current_span("test"): + r = self.http.request("GET", testenv["flask_server"] + "/400") + + assert r.status == 400 + spans = self.recorder.queued_spans() + # 3 spans: sdk(test) → urllib3 → wsgi + exit_span = get_first_span_by_name(spans, "urllib3") + assert exit_span is not None + assert not exit_span.ec, "default: 400 must not be errored on exit span" + + entry_span = get_first_span_by_name(spans, "wsgi") + assert entry_span is not None + assert not entry_span.ec, "entry span must never be errored by 4xx classification" + + def test_classify_all_400_is_error(self) -> None: + """With classify_all=True, 400 exit span MUST be errored.""" + agent.options.http_exit_classify_all_4xx_as_errors = True + + with self.tracer.start_as_current_span("test"): + r = self.http.request("GET", testenv["flask_server"] + "/400") + + assert r.status == 400 + spans = self.recorder.queued_spans() + exit_span = get_first_span_by_name(spans, "urllib3") + assert exit_span is not None + assert exit_span.ec == 1, "classify_all: 400 must be errored on exit span" + + entry_span = get_first_span_by_name(spans, "wsgi") + assert entry_span is not None + assert not entry_span.ec, "entry span must never be errored by 4xx classification" + + def test_classify_codes_400_is_error_404_is_not(self) -> None: + """With classify_codes=[400], 400→ec=1 but 405→ec=0.""" + agent.options.http_exit_classify_as_errors = [400] + + with self.tracer.start_as_current_span("test"): + r400 = self.http.request("GET", testenv["flask_server"] + "/400") + spans_400 = self.recorder.queued_spans() + self.recorder.clear_spans() + + with self.tracer.start_as_current_span("test"): + r405 = self.http.request("GET", testenv["flask_server"] + "/405") + spans_405 = self.recorder.queued_spans() + + assert r400.status == 400 + exit_400 = get_first_span_by_name(spans_400, "urllib3") + assert exit_400 is not None + assert exit_400.ec == 1, "400 in classify list must be errored" + + assert r405.status == 405 + exit_405 = get_first_span_by_name(spans_405, "urllib3") + assert exit_405 is not None + assert not exit_405.ec, "405 NOT in classify list must not be errored" + + +# --------------------------------------------------------------------------- +# httpx +# --------------------------------------------------------------------------- + + +class TestHttpx4xxClassification: + @pytest.fixture(autouse=True) + def _setup(self) -> Generator[None, None, None]: + self.tracer = get_tracer() + self.recorder = self.tracer.span_processor + self.recorder.clear_spans() + yield + + def test_default_400_not_error(self) -> None: + with self.tracer.start_as_current_span("test"): + r = httpx.get(testenv["flask_server"] + "/400") + + assert r.status_code == 400 + spans = self.recorder.queued_spans() + # httpx span name is registered as "http", entry (Flask/wsgi) is "wsgi" + exit_span = get_first_span_by_name(spans, "http") + assert exit_span is not None + assert not exit_span.ec, "default: 400 must not be errored on exit span" + + entry_span = get_first_span_by_name(spans, "wsgi") + assert entry_span is not None + assert not entry_span.ec + + def test_classify_all_400_is_error(self) -> None: + agent.options.http_exit_classify_all_4xx_as_errors = True + + with self.tracer.start_as_current_span("test"): + r = httpx.get(testenv["flask_server"] + "/400") + + assert r.status_code == 400 + spans = self.recorder.queued_spans() + exit_span = get_first_span_by_name(spans, "http") + assert exit_span is not None + assert exit_span.ec == 1, "classify_all: 400 must be errored on exit span" + + entry_span = get_first_span_by_name(spans, "wsgi") + assert entry_span is not None + assert not entry_span.ec + + def test_classify_codes_selective(self) -> None: + agent.options.http_exit_classify_as_errors = [400] + + with self.tracer.start_as_current_span("test"): + httpx.get(testenv["flask_server"] + "/400") + spans_400 = self.recorder.queued_spans() + self.recorder.clear_spans() + + with self.tracer.start_as_current_span("test"): + httpx.get(testenv["flask_server"] + "/405") + spans_405 = self.recorder.queued_spans() + + exit_400 = get_first_span_by_name(spans_400, "http") + assert exit_400 is not None and exit_400.ec == 1 + + exit_405 = get_first_span_by_name(spans_405, "http") + assert exit_405 is not None and not exit_405.ec + + +# --------------------------------------------------------------------------- +# aiohttp client +# --------------------------------------------------------------------------- + + +class TestAiohttpClient4xxClassification: + @pytest.fixture(autouse=True) + def _setup(self) -> Generator[None, None, None]: + self.tracer = get_tracer() + self.recorder = self.tracer.span_processor + self.recorder.clear_spans() + self.loop = asyncio.new_event_loop() + asyncio.set_event_loop(self.loop) + yield + self.loop.close() + + def _run(self, coro) -> object: + return self.loop.run_until_complete(coro) + + async def _fetch(self, url: str) -> int: + async with aiohttp.ClientSession() as session: + try: + async with session.get(url) as resp: + return resp.status + except aiohttp.ClientResponseError as e: + return e.status + + def test_default_401_not_error(self) -> None: + with self.tracer.start_as_current_span("test"): + status = self._run(self._fetch(testenv["aiohttp_server"] + "/401")) + + assert status == 401 + spans = self.recorder.queued_spans() + exit_span = get_first_span_by_name(spans, "aiohttp-client") + assert exit_span is not None + assert not exit_span.ec, "default: 401 must not be errored" + + def test_classify_all_401_is_error(self) -> None: + agent.options.http_exit_classify_all_4xx_as_errors = True + + with self.tracer.start_as_current_span("test"): + status = self._run(self._fetch(testenv["aiohttp_server"] + "/401")) + + assert status == 401 + spans = self.recorder.queued_spans() + exit_span = get_first_span_by_name(spans, "aiohttp-client") + assert exit_span is not None + assert exit_span.ec == 1, "classify_all: 401 must be errored" + + def test_classify_codes_401_is_error(self) -> None: + agent.options.http_exit_classify_as_errors = [401] + + with self.tracer.start_as_current_span("test"): + status = self._run(self._fetch(testenv["aiohttp_server"] + "/401")) + + assert status == 401 + spans = self.recorder.queued_spans() + exit_span = get_first_span_by_name(spans, "aiohttp-client") + assert exit_span is not None + assert exit_span.ec == 1 + + +# --------------------------------------------------------------------------- +# tornado client +# --------------------------------------------------------------------------- + + +class TestTornadoClient4xxClassification: + @pytest.fixture(autouse=True) + def _setup(self) -> Generator[None, None, None]: + self.tracer = get_tracer() + self.recorder = self.tracer.span_processor + self.recorder.clear_spans() + # New event loop for every test — same pattern as test_tornado_client.py + self.loop = asyncio.new_event_loop() + asyncio.set_event_loop(self.loop) + self.http_client = AsyncHTTPClient() + yield + self.http_client.close() + + def test_default_405_client_exception_is_error(self) -> None: + """405 raises HTTPClientError → tornado already marks ec=1 via record_exception.""" + async def test(): + with self.tracer.start_as_current_span("test"): + try: + return await self.http_client.fetch( + testenv["tornado_server"] + "/405" + ) + except tornado.httpclient.HTTPClientError as e: + return e.response + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + server_span = get_first_span_by_name(spans, "tornado-server") + assert client_span is not None + assert client_span.ec == 1, "405 HTTPClientError must always errored via record_exception" + assert server_span is not None + assert not server_span.ec, "entry span must not be errored" + + def test_classify_all_405_is_error(self) -> None: + """With classify_all=True, HTTPClientError path: 405→ec=1 (same as default, + since record_exception already sets ec). More importantly, entry span stays ec=0.""" + agent.options.http_exit_classify_all_4xx_as_errors = True + + async def test(): + with self.tracer.start_as_current_span("test"): + try: + return await self.http_client.fetch( + testenv["tornado_server"] + "/405" + ) + except tornado.httpclient.HTTPClientError as e: + return e.response + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + server_span = get_first_span_by_name(spans, "tornado-server") + assert client_span is not None + assert client_span.ec == 1 + assert server_span is not None + assert not server_span.ec, "entry span must never be errored by 4xx classification" + + def test_classify_codes_405_in_list_is_error(self) -> None: + """With classify_codes=[405], HTTPClientError path still marks ec=1 (record_exception).""" + agent.options.http_exit_classify_as_errors = [405] + + async def test(): + with self.tracer.start_as_current_span("test"): + try: + return await self.http_client.fetch( + testenv["tornado_server"] + "/405" + ) + except tornado.httpclient.HTTPClientError as e: + return e.response + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + assert client_span is not None + assert client_span.ec == 1 + + # ------------------------------------------------------------------ + # raise_error=False tests — exercise should_mark_http_exit_as_error + # happy path in finish_tracing (no HTTPClientError raised) + # ------------------------------------------------------------------ + + def test_raise_error_false_default_405_not_error(self) -> None: + """raise_error=False: 405 response arrives on the happy path of finish_tracing. + With no opt-in active the exit span must NOT be errored.""" + + async def test(): + with self.tracer.start_as_current_span("test"): + return await self.http_client.fetch( + testenv["tornado_server"] + "/405", raise_error=False + ) + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + server_span = get_first_span_by_name(spans, "tornado-server") + assert client_span is not None + assert not client_span.ec, "default: 405 must not be errored on exit span" + assert server_span is not None + assert not server_span.ec, "entry span must not be errored" + + def test_raise_error_false_classify_all_405_is_error(self) -> None: + """raise_error=False + classify_all=True: should_mark_http_exit_as_error() runs + on the happy path and must set ec=1 on the exit span; entry span stays ec=0.""" + agent.options.http_exit_classify_all_4xx_as_errors = True + + async def test(): + with self.tracer.start_as_current_span("test"): + return await self.http_client.fetch( + testenv["tornado_server"] + "/405", raise_error=False + ) + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + server_span = get_first_span_by_name(spans, "tornado-server") + assert client_span is not None + assert client_span.ec == 1, "classify_all: 405 must be errored on exit span" + assert client_span.data["http"]["error"] == "405 Method Not Allowed", "http.error must be set" + assert server_span is not None + assert not server_span.ec, "entry span must never be errored by 4xx classification" + + def test_raise_error_false_classify_codes_405_in_list_is_error(self) -> None: + """raise_error=False + classify_codes=[405]: should_mark_http_exit_as_error() + must return True for 405 and set ec=1; a code not in the list (e.g. 301) stays ec=0.""" + agent.options.http_exit_classify_as_errors = [405] + + async def test(): + with self.tracer.start_as_current_span("test"): + return await self.http_client.fetch( + testenv["tornado_server"] + "/405", raise_error=False + ) + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + assert client_span is not None + assert client_span.ec == 1, "classify_codes=[405]: 405 must be errored on exit span" + assert client_span.data["http"]["error"] == "405 Method Not Allowed", "http.error must be set" + + def test_raise_error_false_classify_codes_405_not_in_list_not_error(self) -> None: + """raise_error=False + classify_codes=[401]: 405 is NOT in the list → ec must stay 0.""" + agent.options.http_exit_classify_as_errors = [401] + + async def test(): + with self.tracer.start_as_current_span("test"): + return await self.http_client.fetch( + testenv["tornado_server"] + "/405", raise_error=False + ) + + tornado.ioloop.IOLoop.current().run_sync(test) + time.sleep(0.3) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "tornado-client") + assert client_span is not None + assert not client_span.ec, "classify_codes=[401]: 405 must NOT be errored" + + +# --------------------------------------------------------------------------- +# twisted client +# --------------------------------------------------------------------------- + + +class TestTwistedClient4xxClassification: + """4xx error classification tests for the Twisted HTTP client. + + Uses ``tests.apps.twisted_server`` (started once per session via module-level + import) as the upstream target. Each test fires a Twisted ``Agent.request`` + from a background thread via ``reactor.callFromThread`` — the same pattern + used by ``TestTwistedClient``. + + Span tree per request: + sdk("test") → twisted-client → twisted-server + """ + + @pytest.fixture(autouse=True) + def _setup(self) -> Generator[None, None, None]: + self.tracer = get_tracer() + self.recorder = self.tracer.span_processor + self.recorder.clear_spans() + yield + + # ------------------------------------------------------------------ + # helpers + # ------------------------------------------------------------------ + + def _make_request(self, path: str) -> Optional[object]: + """Fire GET inside a test span; return the response.""" + result_holder: dict = {} + event = threading.Event() + + def run() -> None: + with self.tracer.start_as_current_span("test"): + agent_obj = Agent(reactor) + url = (testenv["twisted_server"] + path).encode("utf-8") + d = agent_obj.request(b"GET", url, Headers({}), None) + + def on_response(response: object) -> object: + result_holder["response"] = response + return response + + def on_error(failure: object) -> object: + result_holder["failure"] = failure + return failure + + d.addCallbacks(on_response, on_error) + + def done(_: object) -> None: + event.set() + + d.addBoth(done) + + reactor.callFromThread(run) + event.wait(timeout=5) + return result_holder.get("response") + + # ------------------------------------------------------------------ + # tests + # ------------------------------------------------------------------ + + def test_default_401_not_error(self) -> None: + """With default config, 401 exit span must NOT be errored.""" + response = self._make_request("/401") + assert response is not None + assert response.code == 401 + + time.sleep(0.5) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "twisted-client") + assert client_span is not None + assert not client_span.ec, "default: 401 must not be errored on exit span" + assert client_span.data["http"]["status"] == 401 + + server_span = get_first_span_by_name(spans, "twisted-server") + assert server_span is not None + assert not server_span.ec, "entry span must never be errored by 4xx classification" + + def test_classify_all_401_is_error(self) -> None: + """With classify_all=True, 401 exit span MUST be errored; entry span must not.""" + agent.options.http_exit_classify_all_4xx_as_errors = True + + response = self._make_request("/401") + assert response is not None + assert response.code == 401 + + time.sleep(0.5) + spans = self.recorder.queued_spans() + + client_span = get_first_span_by_name(spans, "twisted-client") + assert client_span is not None + assert client_span.ec == 1, "classify_all: 401 must be errored on exit span" + assert client_span.data["http"]["error"] == "401 Unauthorized" + + server_span = get_first_span_by_name(spans, "twisted-server") + assert server_span is not None + assert not server_span.ec, "entry span must never be errored by 4xx classification" + + def test_classify_codes_401_is_error_403_is_not(self) -> None: + """With classify_codes=[401], 401→ec=1 but 403→ec=0; entry spans unaffected.""" + agent.options.http_exit_classify_as_errors = [401] + + response_401 = self._make_request("/401") + assert response_401 is not None + assert response_401.code == 401 + + time.sleep(0.5) + spans_401 = self.recorder.queued_spans() + self.recorder.clear_spans() + + response_403 = self._make_request("/403") + assert response_403 is not None + assert response_403.code == 403 + + time.sleep(0.5) + spans_403 = self.recorder.queued_spans() + + client_401 = get_first_span_by_name(spans_401, "twisted-client") + assert client_401 is not None + assert client_401.ec == 1, "401 in classify list must be errored" + assert client_401.data["http"]["error"] == "401 Unauthorized" + + server_401 = get_first_span_by_name(spans_401, "twisted-server") + assert server_401 is not None + assert not server_401.ec, "entry span must never be errored by 4xx classification" + + client_403 = get_first_span_by_name(spans_403, "twisted-client") + assert client_403 is not None + assert not client_403.ec, "403 NOT in classify list must not be errored" + + server_403 = get_first_span_by_name(spans_403, "twisted-server") + assert server_403 is not None + assert not server_403.ec, "entry span must never be errored by 4xx classification" + + def test_classify_all_overridden_by_classify_codes(self) -> None: + """classify_codes non-empty overrides classify_all per spec precedence rule. + + classify_all=True + classify_codes=[403] → only 403 gets ec=1; 401 stays ec=0. + """ + agent.options.http_exit_classify_all_4xx_as_errors = True + agent.options.http_exit_classify_as_errors = [403] + + response_401 = self._make_request("/401") + assert response_401 is not None + assert response_401.code == 401 + + time.sleep(0.5) + spans_401 = self.recorder.queued_spans() + self.recorder.clear_spans() + + response_403 = self._make_request("/403") + assert response_403 is not None + assert response_403.code == 403 + + time.sleep(0.5) + spans_403 = self.recorder.queued_spans() + + client_401 = get_first_span_by_name(spans_401, "twisted-client") + assert client_401 is not None + assert not client_401.ec, "classify_codes overrides classify_all: 401 must not be errored" + + client_403 = get_first_span_by_name(spans_403, "twisted-client") + assert client_403 is not None + assert client_403.ec == 1, "403 in classify_codes must be errored even with classify_all" diff --git a/tests/frameworks/test_aiohttp_client.py b/tests/frameworks/test_aiohttp_client.py index cac00bd7..90429669 100644 --- a/tests/frameworks/test_aiohttp_client.py +++ b/tests/frameworks/test_aiohttp_client.py @@ -266,7 +266,7 @@ async def test(): assert aiohttp_span.data["http"]["status"] == 500 assert aiohttp_span.data["http"]["url"] == testenv["flask_server"] + "/500" assert aiohttp_span.data["http"]["method"] == "GET" - assert aiohttp_span.data["http"]["error"] == "INTERNAL SERVER ERROR" + assert aiohttp_span.data["http"]["error"] == "500 Internal Server Error" assert aiohttp_span.stack assert isinstance(aiohttp_span.stack, list) assert len(aiohttp_span.stack) > 1 @@ -313,7 +313,7 @@ async def test(): assert aiohttp_span.data["http"]["status"] == 504 assert aiohttp_span.data["http"]["url"] == testenv["flask_server"] + "/504" assert aiohttp_span.data["http"]["method"] == "GET" - assert aiohttp_span.data["http"]["error"] == "GATEWAY TIMEOUT" + assert aiohttp_span.data["http"]["error"] == "504 Gateway Timeout" assert aiohttp_span.stack assert isinstance(aiohttp_span.stack, list) assert len(aiohttp_span.stack) > 1 diff --git a/tests/frameworks/test_aiohttp_server.py b/tests/frameworks/test_aiohttp_server.py index 86781c6b..53d46a1e 100644 --- a/tests/frameworks/test_aiohttp_server.py +++ b/tests/frameworks/test_aiohttp_server.py @@ -474,7 +474,7 @@ async def test(): assert aioclient_span.n == "aiohttp-client" assert aioclient_span.data["http"]["status"] == 500 - assert aioclient_span.data["http"]["error"] == "Internal Server Error" + assert aioclient_span.data["http"]["error"] == "500 Internal Server Error" assert aioclient_span.stack assert isinstance(aioclient_span.stack, list) assert len(aioclient_span.stack) > 1 diff --git a/tests/frameworks/test_tornado_server.py b/tests/frameworks/test_tornado_server.py index ddba6bf9..d4e1b4f8 100644 --- a/tests/frameworks/test_tornado_server.py +++ b/tests/frameworks/test_tornado_server.py @@ -388,7 +388,7 @@ async def test(): assert aiohttp_span.data["http"]["status"] == 500 assert testenv["tornado_server"] + "/500" == aiohttp_span.data["http"]["url"] assert aiohttp_span.data["http"]["method"] == "GET" - assert aiohttp_span.data["http"]["error"] == "Internal Server Error" + assert aiohttp_span.data["http"]["error"] == "500 Internal Server Error" assert aiohttp_span.stack assert isinstance(aiohttp_span.stack, list) assert len(aiohttp_span.stack) > 1 @@ -451,7 +451,7 @@ async def test(): assert aiohttp_span.data["http"]["status"] == 504 assert testenv["tornado_server"] + "/504" == aiohttp_span.data["http"]["url"] assert aiohttp_span.data["http"]["method"] == "GET" - assert aiohttp_span.data["http"]["error"] == "Gateway Timeout" + assert aiohttp_span.data["http"]["error"] == "504 Gateway Timeout" assert aiohttp_span.stack assert isinstance(aiohttp_span.stack, list) assert len(aiohttp_span.stack) > 1 diff --git a/tests/frameworks/test_twisted_client.py b/tests/frameworks/test_twisted_client.py index 55b23058..1a2b7256 100644 --- a/tests/frameworks/test_twisted_client.py +++ b/tests/frameworks/test_twisted_client.py @@ -155,7 +155,7 @@ def test_get_500(self) -> None: assert client_span.data["http"]["status"] == 500 assert client_span.data["http"]["method"] == "GET" assert client_span.data["http"]["url"] == testenv["twisted_server"] + "/500" - assert client_span.data["http"]["error"] == "Internal Server Error" + assert client_span.data["http"]["error"] == "500 Internal Server Error" def test_get_with_params_to_scrub(self) -> None: response, failure = self._make_request("/", params={"secret": "yeah"}) diff --git a/tests/frameworks/test_twisted_server.py b/tests/frameworks/test_twisted_server.py index c3ce7174..888a7160 100644 --- a/tests/frameworks/test_twisted_server.py +++ b/tests/frameworks/test_twisted_server.py @@ -234,7 +234,7 @@ def test_get_500(self) -> None: assert twisted_span.data["http"]["url"] == testenv["twisted_server"] + "/500" assert twisted_span.data["http"]["method"] == "GET" assert not twisted_span.stack - assert twisted_span.data["http"]["error"] == "Internal Server Error" + assert twisted_span.data["http"]["error"] == "500 Internal Server Error" def test_get_with_params_to_scrub(self) -> None: with self.tracer.start_as_current_span("test"): diff --git a/tests/test_options.py b/tests/test_options.py index 4c167b8e..3ef3cf1b 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1744,3 +1744,135 @@ def test_stack_trace_precedence_yaml_over_agent(self) -> None: # YAML should override agent config assert self.options.stack_trace_level == "error" assert self.options.stack_trace_length == 20 + + +class TestHttpExitClassification: + """Tests for HTTP exit 4xx error classification configuration.""" + + @pytest.fixture(autouse=True) + def _cleanup(self) -> Generator[None, None, None]: + yield + for key in ( + "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS", + "INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS", + ): + os.environ.pop(key, None) + + # ------------------------------------------------------------------ defaults + def test_defaults_are_off(self) -> None: + opts = BaseOptions() + assert opts.http_exit_classify_all_4xx_as_errors is False + assert opts.http_exit_classify_as_errors == [] + + # ------------------------------------------------------------------ env var: classify_all + def test_env_classify_all_true(self) -> None: + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] = "true" + opts = BaseOptions() + assert opts.http_exit_classify_all_4xx_as_errors is True + assert opts.http_exit_classify_as_errors == [] + + def test_env_classify_all_false(self) -> None: + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] = "false" + opts = BaseOptions() + assert opts.http_exit_classify_all_4xx_as_errors is False + + # ------------------------------------------------------------------ env var: classify_codes + def test_env_classify_codes_valid(self) -> None: + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS"] = "401,403" + opts = BaseOptions() + assert opts.http_exit_classify_as_errors == [401, 403] + assert opts.http_exit_classify_all_4xx_as_errors is False + + def test_env_classify_codes_with_spaces(self) -> None: + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS"] = " 401 , 403 " + opts = BaseOptions() + assert opts.http_exit_classify_as_errors == [401, 403] + + def test_env_classify_codes_out_of_range_ignored(self) -> None: + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS"] = "401,500,200" + opts = BaseOptions() + # 500 and 200 are outside 400-499, only 401 should remain + assert opts.http_exit_classify_as_errors == [401] + + def test_env_classify_codes_takes_precedence_over_classify_all(self) -> None: + """When classify_as_errors env var is set, classify_all env var must be ignored.""" + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_AS_ERRORS"] = "401" + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] = "true" + opts = BaseOptions() + assert opts.http_exit_classify_as_errors == [401] + # classify_all must stay False because classify_codes took precedence + assert opts.http_exit_classify_all_4xx_as_errors is False + + # ------------------------------------------------------------------ agent config via set_tracing + def test_set_tracing_classify_all(self) -> None: + opts = StandardOptions() + opts.set_tracing({"http": {"exit": {"classify-all-4xx-as-errors": True}}}) + assert opts.http_exit_classify_all_4xx_as_errors is True + + def test_set_tracing_classify_codes(self) -> None: + opts = StandardOptions() + opts.set_tracing({"http": {"exit": {"classify-as-errors": [401, 403]}}}) + assert opts.http_exit_classify_as_errors == [401, 403] + + def test_set_tracing_classify_codes_out_of_range_ignored(self) -> None: + opts = StandardOptions() + opts.set_tracing({"http": {"exit": {"classify-as-errors": [401, 500, 200]}}}) + assert opts.http_exit_classify_as_errors == [401] + + def test_set_tracing_classify_codes_wins_over_classify_all(self) -> None: + """classify-as-errors in agent config takes precedence over classify-all-4xx-as-errors.""" + opts = StandardOptions() + opts.set_tracing({ + "http": { + "exit": { + "classify-all-4xx-as-errors": True, + "classify-as-errors": [401], + } + } + }) + assert opts.http_exit_classify_as_errors == [401] + # classify_all must remain False (codes list took precedence inside _apply_agent_http_exit_classification) + assert opts.http_exit_classify_all_4xx_as_errors is False + + def test_env_var_takes_precedence_over_agent_config(self) -> None: + """Env var has higher priority than agent config.""" + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] = "true" + opts = StandardOptions() + # Agent config says False — env var should win + opts.set_tracing({"http": {"exit": {"classify-all-4xx-as-errors": False}}}) + assert opts.http_exit_classify_all_4xx_as_errors is True + + # ------------------------------------------------------------------ env var: classify_all invalid value + def test_env_classify_all_non_truthy_value_stays_false(self, monkeypatch) -> None: + """A value not recognised as truthy by is_truthy() results in False (no warning needed).""" + monkeypatch.setenv("INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS", "yes") + opts = BaseOptions() + assert opts.http_exit_classify_all_4xx_as_errors is False + + def test_env_classify_all_invalid_does_not_fall_through_to_yaml( + self, monkeypatch, tmp_path + ) -> None: + """An invalid CLASSIFY_ALL value must still block YAML from being read (env var set → YAML skipped).""" + yaml_file = tmp_path / "config.yaml" + yaml_file.write_text( + "tracing:\n http:\n exit:\n classify-all-4xx-as-errors: true\n" + ) + monkeypatch.setenv("INSTANA_CONFIG_PATH", str(yaml_file)) + monkeypatch.setenv("INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS", "garbage") + opts = BaseOptions() + # Invalid env var → warning + default; YAML must NOT override because the env var branch was entered + assert opts.http_exit_classify_all_4xx_as_errors is False + + def test_env_classify_all_false_does_not_fall_through_to_yaml( + self, monkeypatch, tmp_path + ) -> None: + """CLASSIFY_ALL=false must take precedence over YAML (explicit false wins, YAML skipped).""" + yaml_file = tmp_path / "config.yaml" + yaml_file.write_text( + "tracing:\n http:\n exit:\n classify-all-4xx-as-errors: true\n" + ) + monkeypatch.setenv("INSTANA_CONFIG_PATH", str(yaml_file)) + monkeypatch.setenv("INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS", "false") + opts = BaseOptions() + # Env var says false explicitly → YAML must NOT override + assert opts.http_exit_classify_all_4xx_as_errors is False diff --git a/tests/util/test_config_reader.py b/tests/util/test_config_reader.py index 71bead0d..293ceaf6 100644 --- a/tests/util/test_config_reader.py +++ b/tests/util/test_config_reader.py @@ -7,8 +7,10 @@ import pytest from yaml import YAMLError +from instana.options import BaseOptions from instana.util.config import ( get_disable_trace_configurations_from_yaml, + get_http_exit_classification_from_yaml, parse_filter_rules_yaml, ) from instana.util.config_reader import ConfigReader @@ -41,7 +43,7 @@ def test_config_reader_default(self) -> None: config_reader = ConfigReader(os.environ.get("INSTANA_CONFIG_PATH", "")) assert config_reader.file_path == filename assert "tracing" in config_reader.data - assert len(config_reader.data["tracing"]) == 2 + assert len(config_reader.data["tracing"]) == 3 def test_config_reader_file_not_found_error( self, caplog: "LogCaptureFixture" @@ -163,6 +165,11 @@ def test_load_configuration_with_tracing(self, caplog: "LogCaptureFixture") -> N assert "redis" not in disabled_spans assert "redis" in enabled_spans + # Check HTTP exit 4xx classification (classify-as-errors: [401, 403]) + classify_all, codes = get_http_exit_classification_from_yaml() + assert classify_all is False + assert codes == [401, 403] + assert ( 'Please use "tracing" instead of "com.instana.tracing" for local configuration file.' not in caplog.messages @@ -222,7 +229,36 @@ def test_load_configuration_legacy(self, caplog: "LogCaptureFixture") -> None: assert "redis" not in disabled_spans assert "redis" in enabled_spans + # Check HTTP exit 4xx classification (classify-all-4xx-as-errors: true) + classify_all, codes = get_http_exit_classification_from_yaml() + assert classify_all is True + assert codes == [] + assert ( 'Please use "tracing" instead of "com.instana.tracing" for local configuration file.' in caplog.messages ) + + def test_base_options_yaml_classify_as_errors(self) -> None: + """BaseOptions reads classify-as-errors codes from YAML (test_configuration-1.yaml).""" + os.environ["INSTANA_CONFIG_PATH"] = "tests/util/test_configuration-1.yaml" + opts = BaseOptions() + assert opts.http_exit_classify_as_errors == [401, 403] + assert opts.http_exit_classify_all_4xx_as_errors is False + + def test_base_options_yaml_classify_all_4xx(self) -> None: + """BaseOptions reads classify-all-4xx-as-errors from YAML (test_configuration-2.yaml).""" + os.environ["INSTANA_CONFIG_PATH"] = "tests/util/test_configuration-2.yaml" + opts = BaseOptions() + assert opts.http_exit_classify_all_4xx_as_errors is True + assert opts.http_exit_classify_as_errors == [] + + def test_base_options_yaml_http_env_var_takes_precedence(self) -> None: + """Env var overrides YAML http classification.""" + os.environ["INSTANA_CONFIG_PATH"] = "tests/util/test_configuration-1.yaml" + os.environ["INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS"] = "true" + opts = BaseOptions() + # YAML says [401, 403] via classify-as-errors, but env var (classify_all) takes precedence + assert opts.http_exit_classify_all_4xx_as_errors is True + assert opts.http_exit_classify_as_errors == [] + os.environ.pop("INSTANA_TRACING_HTTP_EXIT_CLASSIFY_ALL_4XX_AS_ERRORS") diff --git a/tests/util/test_configuration-1.yaml b/tests/util/test_configuration-1.yaml index 3f19a384..69ad52e8 100644 --- a/tests/util/test_configuration-1.yaml +++ b/tests/util/test_configuration-1.yaml @@ -44,6 +44,11 @@ tracing: - key: "kafka.service" values: ["topic"] match_type: "contains" + http: + exit: + classify-as-errors: + - 401 + - 403 disable: - "logging": true - "databases": true diff --git a/tests/util/test_configuration-2.yaml b/tests/util/test_configuration-2.yaml index 9021cc26..67e3173f 100644 --- a/tests/util/test_configuration-2.yaml +++ b/tests/util/test_configuration-2.yaml @@ -24,6 +24,9 @@ com.instana.tracing: match_type: "strict" - key: "kafka.access" values: ["*"] + http: + exit: + classify-all-4xx-as-errors: true disable: - "logging": true - "databases": true diff --git a/tests/util/test_http_utils.py b/tests/util/test_http_utils.py new file mode 100644 index 00000000..9294673a --- /dev/null +++ b/tests/util/test_http_utils.py @@ -0,0 +1,94 @@ +# (c) Copyright IBM Corp. 2026 + +from typing import Optional +from unittest.mock import MagicMock + +from instana.util.http import should_mark_http_exit_as_error + + +def _opts(classify_all: bool = False, classify_codes: Optional[list] = None) -> MagicMock: + """Return a minimal options-like object.""" + opts = MagicMock() + opts.http_exit_classify_all_4xx_as_errors = classify_all + opts.http_exit_classify_as_errors = classify_codes if classify_codes is not None else [] + return opts + + +class TestShouldMarkHttpExitAsError: + # ------------------------------------------------------------------ 5xx + def test_500_is_always_error(self) -> None: + assert should_mark_http_exit_as_error(500, _opts()) + + def test_503_is_always_error(self) -> None: + assert should_mark_http_exit_as_error(503, _opts()) + + def test_599_is_always_error(self) -> None: + assert should_mark_http_exit_as_error(599, _opts()) + + # ------------------------------------------------------------------ 2xx / 3xx + def test_200_is_not_error(self) -> None: + assert not should_mark_http_exit_as_error(200, _opts()) + + def test_301_is_not_error(self) -> None: + assert not should_mark_http_exit_as_error(301, _opts()) + + # ------------------------------------------------------------------ 4xx default (opt-in off) + def test_400_default_not_error(self) -> None: + assert not should_mark_http_exit_as_error(400, _opts()) + + def test_401_default_not_error(self) -> None: + assert not should_mark_http_exit_as_error(401, _opts()) + + def test_403_default_not_error(self) -> None: + assert not should_mark_http_exit_as_error(403, _opts()) + + def test_404_default_not_error(self) -> None: + assert not should_mark_http_exit_as_error(404, _opts()) + + def test_499_default_not_error(self) -> None: + assert not should_mark_http_exit_as_error(499, _opts()) + + # ------------------------------------------------------------------ classify_all_4xx + def test_400_classify_all_is_error(self) -> None: + assert should_mark_http_exit_as_error(400, _opts(classify_all=True)) + + def test_404_classify_all_is_error(self) -> None: + assert should_mark_http_exit_as_error(404, _opts(classify_all=True)) + + def test_499_classify_all_is_error(self) -> None: + assert should_mark_http_exit_as_error(499, _opts(classify_all=True)) + + def test_399_classify_all_not_error(self) -> None: + """399 is outside 4xx range — must not be affected by classify_all.""" + assert not should_mark_http_exit_as_error(399, _opts(classify_all=True)) + + # ------------------------------------------------------------------ classify_as_errors list + def test_401_in_list_is_error(self) -> None: + assert should_mark_http_exit_as_error(401, _opts(classify_codes=[401, 403])) + + def test_403_in_list_is_error(self) -> None: + assert should_mark_http_exit_as_error(403, _opts(classify_codes=[401, 403])) + + def test_404_not_in_list_not_error(self) -> None: + assert not should_mark_http_exit_as_error(404, _opts(classify_codes=[401, 403])) + + # ------------------------------------------------------------------ precedence: list wins over classify_all + def test_list_takes_precedence_over_classify_all(self) -> None: + """When classify_as_errors is set, classify_all_4xx is ignored.""" + opts = _opts(classify_all=True, classify_codes=[401]) + # 401 is in the list → error + assert should_mark_http_exit_as_error(401, opts) + # 404 is NOT in the list → no error, even though classify_all=True + assert not should_mark_http_exit_as_error(404, opts) + + # ------------------------------------------------------------------ boundary + def test_boundary_399_not_4xx(self) -> None: + assert not should_mark_http_exit_as_error(399, _opts(classify_all=True)) + + def test_boundary_500_always_error(self) -> None: + assert should_mark_http_exit_as_error(500, _opts()) + + def test_boundary_499_classify_all(self) -> None: + assert should_mark_http_exit_as_error(499, _opts(classify_all=True)) + +