Repository navigation
feat(options): add option to disable the Server-Timing header #921
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| get_disable_trace_configurations_from_env, | ||
| get_disable_trace_configurations_from_local, | ||
| get_disable_trace_configurations_from_yaml, | ||
| get_disable_server_timing_from_yaml, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this above |
||
| get_http_exit_classification_from_yaml, | ||
| get_stack_trace_config_from_yaml, | ||
| is_truthy, | ||
|
|
@@ -64,6 +65,9 @@ def __init__(self, **kwds: dict[str, Any]) -> None: | |
| self.stack_trace_level = "all" # Options: "all", "error", "none" | ||
| self.stack_trace_length = 30 # Default: 30, recommended range: 10-40 | ||
|
|
||
| # Disable injection of the Server-Timing response header (tracing.global.disable-server-timing) | ||
| self.disable_server_timing = False | ||
|
|
||
| # Technology-specific stack trace overrides | ||
| # Format: {"kafka": {"level": "all", "length": 25}, "redis": {"level": "error", "length": 20}} | ||
| self.stack_trace_technology_config = {} | ||
|
|
@@ -128,6 +132,7 @@ def set_trace_configurations(self) -> None: | |
| self.set_http_exit_classification_configurations() | ||
| self.set_disable_trace_configurations() | ||
| self.set_stack_trace_configurations() | ||
| self.set_disable_server_timing_configuration() | ||
| self.set_span_filter_configurations() | ||
|
|
||
| def set_http_exit_classification_configurations(self) -> None: | ||
|
|
@@ -290,6 +295,30 @@ def set_stack_trace_configurations(self) -> None: | |
| elif isinstance(config.get("tracing"), dict): | ||
| self._apply_in_code_stack_trace_config() | ||
|
|
||
| @staticmethod | ||
| def _has_env_disable_server_timing() -> bool: | ||
| """ | ||
| Return True if INSTANA_TRACING_DISABLE_SERVER_TIMING is set to a non-empty value. | ||
| An empty string is treated as unset, so it does not take precedence over | ||
| the YAML file or agent config. Any non-empty value (even a non-truthy one | ||
| such as "false" or "0") is an explicit setting and takes precedence. | ||
| """ | ||
| return bool(os.environ.get("INSTANA_TRACING_DISABLE_SERVER_TIMING")) | ||
|
|
||
| def set_disable_server_timing_configuration(self) -> None: | ||
| """ | ||
| Set disable-server-timing following precedence: | ||
| INSTANA_TRACING_DISABLE_SERVER_TIMING > INSTANA_CONFIG_PATH > agent config > default (False) | ||
| The agent config is handled in StandardOptions.set_tracing(). | ||
| """ | ||
| if self._has_env_disable_server_timing(): | ||
| self.disable_server_timing = is_truthy( | ||
| os.environ["INSTANA_TRACING_DISABLE_SERVER_TIMING"] | ||
| ) | ||
| elif "INSTANA_CONFIG_PATH" in os.environ: | ||
| self.disable_server_timing = get_disable_server_timing_from_yaml() | ||
|
|
||
| def set_disable_trace_configurations(self) -> None: | ||
| disabled_spans = [] | ||
| enabled_spans = [] | ||
|
|
@@ -459,10 +488,28 @@ def set_tracing(self, tracing: dict[str, Any]) -> None: | |
| # Handle stack trace configuration from agent config | ||
| self.set_stack_trace_from_agent(tracing) | ||
|
|
||
| self._apply_agent_disable_server_timing(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_disable_server_timing(self, tracing: dict[str, Any]) -> None: | ||
| """ | ||
| Apply tracing.global.disable-server-timing from agent config (lowest | ||
| priority: the environment variable and the YAML file take precedence). | ||
| """ | ||
| global_config = tracing.get("global") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add this parameter to the configuration test yaml files? |
||
| if ( | ||
| not self._has_env_disable_server_timing() | ||
| and "INSTANA_CONFIG_PATH" not in os.environ | ||
| and isinstance(global_config, dict) | ||
| and "disable-server-timing" in global_config | ||
| ): | ||
| self.disable_server_timing = is_truthy( | ||
| global_config["disable-server-timing"] | ||
| ) | ||
|
|
||
| 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) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,11 @@ def inject( | |
| trace_id = format_span_id(span_context.trace_id).encode() | ||
| span_id = format_span_id(span_context.span_id).encode() | ||
| level = str(span_context.level).encode() | ||
| server_timing = define_server_timing(span_context.trace_id).encode() | ||
| server_timing = ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If if not self.is_server_timing_disabled():
headers.append((self.HEADER_SERVER_TIMING, define_server_timing(span_context.trace_id).encode()))
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do prefer the way @arjun-rajappa proposed here. With that, we have a common place to set all necessary values, even if used just once. |
||
| None | ||
| if self.is_server_timing_disabled() | ||
| else define_server_timing(span_context.trace_id).encode() | ||
| ) | ||
|
|
||
| if disable_w3c_trace_context: | ||
| traceparent, tracestate = [None] * 2 | ||
|
|
@@ -53,42 +57,26 @@ def inject( | |
| except Exception: | ||
| traceparent, tracestate = [None] * 2 | ||
|
|
||
| headers = [] | ||
| if traceparent and tracestate: | ||
| headers.append((self.HEADER_KEY_TRACEPARENT, traceparent)) | ||
| headers.append((self.HEADER_KEY_TRACESTATE, tracestate)) | ||
| headers.append((self.HEADER_KEY_T, trace_id)) | ||
| headers.append((self.HEADER_KEY_S, span_id)) | ||
| headers.append((self.HEADER_KEY_L, level)) | ||
| if server_timing: | ||
| headers.append((self.HEADER_SERVER_TIMING, server_timing)) | ||
|
|
||
| if isinstance(carrier, dict) or hasattr(carrier, "__dict__"): | ||
| if traceparent and tracestate: | ||
| carrier[self.HEADER_KEY_TRACEPARENT] = traceparent # type: ignore[index] | ||
| carrier[self.HEADER_KEY_TRACESTATE] = tracestate # type: ignore[index] | ||
| carrier[self.HEADER_KEY_T] = trace_id # type: ignore[index] | ||
| carrier[self.HEADER_KEY_S] = span_id # type: ignore[index] | ||
| carrier[self.HEADER_KEY_L] = level # type: ignore[index] | ||
| carrier[self.HEADER_SERVER_TIMING] = server_timing # type: ignore[index] | ||
| for key, value in headers: | ||
| carrier[key] = value # type: ignore[index] | ||
| elif isinstance(carrier, list): | ||
| if traceparent and tracestate: | ||
| carrier.append((self.HEADER_KEY_TRACEPARENT, traceparent)) | ||
| carrier.append((self.HEADER_KEY_TRACESTATE, tracestate)) | ||
| carrier.append((self.HEADER_KEY_T, trace_id)) | ||
| carrier.append((self.HEADER_KEY_S, span_id)) | ||
| carrier.append((self.HEADER_KEY_L, level)) | ||
| carrier.append((self.HEADER_SERVER_TIMING, server_timing)) | ||
| carrier.extend(headers) | ||
| elif isinstance(carrier, tuple): | ||
| if traceparent and tracestate: | ||
| carrier = carrier.__add__( | ||
| ((self.HEADER_KEY_TRACEPARENT, traceparent),) | ||
| ) | ||
| carrier = carrier.__add__( | ||
| ((self.HEADER_KEY_TRACESTATE, tracestate),) | ||
| ) | ||
| carrier = carrier.__add__(((self.HEADER_KEY_T, trace_id),)) | ||
| carrier = carrier.__add__(((self.HEADER_KEY_S, span_id),)) | ||
| carrier = carrier.__add__(((self.HEADER_KEY_L, level),)) | ||
| carrier = carrier.__add__(((self.HEADER_SERVER_TIMING, server_timing),)) | ||
| carrier = carrier.__add__(tuple(headers)) | ||
| elif hasattr(carrier, "__setitem__"): | ||
| if traceparent and tracestate: | ||
| carrier.__setitem__(self.HEADER_KEY_TRACEPARENT, traceparent) | ||
| carrier.__setitem__(self.HEADER_KEY_TRACESTATE, tracestate) | ||
| carrier.__setitem__(self.HEADER_KEY_T, trace_id) | ||
| carrier.__setitem__(self.HEADER_KEY_S, span_id) | ||
| carrier.__setitem__(self.HEADER_KEY_L, level) | ||
| carrier.__setitem__(self.HEADER_SERVER_TIMING, server_timing) | ||
| for key, value in headers: | ||
| carrier.__setitem__(key, value) | ||
| else: | ||
| raise Exception("Unsupported carrier type", type(carrier)) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,30 +29,30 @@ def inject( | |
| try: | ||
| trace_id = format_span_id(span_context.trace_id) | ||
| span_id = format_span_id(span_context.span_id) | ||
| server_timing = define_server_timing(span_context.trace_id).encode() | ||
| server_timing = ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as this. |
||
| None | ||
| if self.is_server_timing_disabled() | ||
| else define_server_timing(span_context.trace_id).encode() | ||
| ) | ||
|
|
||
| headers = [ | ||
| (self.LC_HEADER_KEY_T, trace_id), | ||
| (self.LC_HEADER_KEY_S, span_id), | ||
| (self.LC_HEADER_KEY_L, "1"), | ||
| ] | ||
| if server_timing: | ||
| headers.append((self.LC_HEADER_KEY_SERVER_TIMING, server_timing)) | ||
|
|
||
| if isinstance(carrier, dict) or hasattr(carrier, "__dict__"): | ||
| carrier[self.LC_HEADER_KEY_T] = trace_id # type: ignore[index] | ||
| carrier[self.LC_HEADER_KEY_S] = span_id # type: ignore[index] | ||
| carrier[self.LC_HEADER_KEY_L] = "1" # type: ignore[index] | ||
| carrier[self.LC_HEADER_KEY_SERVER_TIMING] = server_timing # type: ignore[index] | ||
| for key, value in headers: | ||
| carrier[key] = value # type: ignore[index] | ||
| elif isinstance(carrier, list): | ||
| carrier.append((self.LC_HEADER_KEY_T, trace_id)) | ||
| carrier.append((self.LC_HEADER_KEY_S, span_id)) | ||
| carrier.append((self.LC_HEADER_KEY_L, "1")) | ||
| carrier.append((self.LC_HEADER_KEY_SERVER_TIMING, server_timing)) | ||
| carrier.extend(headers) | ||
| elif isinstance(carrier, tuple): | ||
| carrier = carrier.__add__(((self.LC_HEADER_KEY_T, trace_id),)) | ||
| carrier = carrier.__add__(((self.LC_HEADER_KEY_S, span_id),)) | ||
| carrier = carrier.__add__(((self.LC_HEADER_KEY_L, "1"),)) | ||
| carrier = carrier.__add__( | ||
| ((self.LC_HEADER_KEY_SERVER_TIMING, server_timing),) | ||
| ) | ||
| carrier = carrier.__add__(tuple(headers)) | ||
| elif hasattr(carrier, "__setitem__"): | ||
| carrier.__setitem__(self.LC_HEADER_KEY_T, trace_id) | ||
| carrier.__setitem__(self.LC_HEADER_KEY_S, span_id) | ||
| carrier.__setitem__(self.LC_HEADER_KEY_L, "1") | ||
| carrier.__setitem__(self.LC_HEADER_KEY_SERVER_TIMING, server_timing) | ||
| for key, value in headers: | ||
| carrier.__setitem__(key, value) | ||
| else: | ||
| raise Exception("Unsupported carrier type", type(carrier)) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| # (c) Copyright IBM Corp. 2026 | ||
|
|
||
| from typing import Any, Generator | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| from instana.instrumentation.aws import lambda_inst | ||
|
|
||
| TRACE_ID = 1812338823475918251 | ||
| SERVER_TIMING = "intid;desc=1922fa4b3d1d76ab" | ||
|
|
||
|
|
||
| class TestLambdaHandlerServerTiming: | ||
| @pytest.fixture(autouse=True) | ||
| def _resources(self) -> Generator[None, None, None]: | ||
| self.agent = MagicMock() | ||
| self.agent.collector.started = False | ||
| self.agent.options.disable_server_timing = False | ||
| self.span = MagicMock() | ||
| self.span.context.trace_id = TRACE_ID | ||
| tracer = MagicMock() | ||
| tracer.start_as_current_span.return_value.__enter__.return_value = self.span | ||
|
|
||
| with ( | ||
| patch.object(lambda_inst, "get_agent", return_value=self.agent), | ||
| patch("instana.singletons.get_agent", return_value=self.agent), | ||
| patch.object(lambda_inst, "get_tracer", return_value=tracer), | ||
| patch.object(lambda_inst, "get_context"), | ||
| patch.object(lambda_inst, "enrich_lambda_span"), | ||
| patch.object( | ||
| lambda_inst, "define_server_timing", return_value=SERVER_TIMING | ||
| ), | ||
| ): | ||
| yield | ||
|
|
||
| def _invoke(self, result: Any) -> Any: | ||
| handler = MagicMock(return_value=result) | ||
| return lambda_inst.lambda_handler_with_instana(handler, None, ({}, {}), {}) | ||
|
|
||
| def test_headers_get_server_timing(self) -> None: | ||
| result = self._invoke({"statusCode": 200, "headers": {}}) | ||
| assert result["headers"]["Server-Timing"] == SERVER_TIMING | ||
|
|
||
| def test_multi_value_headers_get_server_timing(self) -> None: | ||
| result = self._invoke({"statusCode": 200, "multiValueHeaders": {}}) | ||
| assert result["multiValueHeaders"]["Server-Timing"] == [SERVER_TIMING] | ||
|
|
||
| def test_headers_untouched_when_disabled(self) -> None: | ||
| self.agent.options.disable_server_timing = True | ||
| result = self._invoke({"statusCode": 200, "headers": {}}) | ||
| assert "Server-Timing" not in result["headers"] | ||
|
|
||
| def test_multi_value_headers_untouched_when_disabled(self) -> None: | ||
| self.agent.options.disable_server_timing = True | ||
| result = self._invoke({"statusCode": 200, "multiValueHeaders": {}}) | ||
| assert "Server-Timing" not in result["multiValueHeaders"] | ||
|
|
||
| def test_status_code_still_recorded_when_disabled(self) -> None: | ||
| self.agent.options.disable_server_timing = True | ||
| self._invoke({"statusCode": 502, "headers": {}}) | ||
| self.span.set_attribute.assert_called_once() | ||
| self.span.record_exception.assert_called_once_with("HTTP status 502") | ||
|
|
||
| def test_non_dict_result_returned_unchanged(self) -> None: | ||
| assert self._invoke("plain") == "plain" | ||
|
|
||
| def test_handler_exception_is_recorded_and_raised(self) -> None: | ||
| handler = MagicMock(side_effect=ValueError("boom")) | ||
| with pytest.raises(ValueError): | ||
| lambda_inst.lambda_handler_with_instana(handler, None, ({}, {}), {}) | ||
| self.span.record_exception.assert_called_once() | ||
| self.agent.collector.shutdown.assert_called() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use dict instead of Dict? And avoid using Any if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with @CagriYonca.
Since Python 3.9, we can use the built-ins
dict,list, andtupleinstead of the importedDict,List, andTuplefrom thetypingmodule.I don't like the indiscriminate use of
Any, but Bob does. If you know the possible types or classes for the dictionary values, please list them.