diff --git a/src/instana/agent/host.py b/src/instana/agent/host.py index c773b712..926f7e03 100644 --- a/src/instana/agent/host.py +++ b/src/instana/agent/host.py @@ -375,10 +375,15 @@ def filter_spans(self, spans: List[Dict[str, Any]]) -> List[Dict[str, Any]]: if isinstance(span.data[span_value], dict): service_name = span_value + # Skip if no valid service name found + if not service_name: + filtered_spans.append(span) + continue + # Set span attributes for filtering attributes_to_check = { "type": service_name, - "kind": span.k, + "kind": getattr(span, "k", None), } # Add operation specifiers to the attributes diff --git a/src/instana/instrumentation/urllib3.py b/src/instana/instrumentation/urllib3.py index fa1f620a..01023071 100644 --- a/src/instana/instrumentation/urllib3.py +++ b/src/instana/instrumentation/urllib3.py @@ -55,11 +55,13 @@ def _collect_kvs( agent.options.secrets_list, ) - url = kvs["host"] + ":" + str(kvs["port"]) + kvs["path"] - if isinstance(instance, urllib3.connectionpool.HTTPSConnectionPool): - kvs["url"] = f"https://{url}" - else: - kvs["url"] = f"http://{url}" + # Only construct URL if host is not None + if kvs.get("host") and kvs.get("path"): + url = f'{kvs["host"]}:{kvs["port"]}{kvs["path"]}' + if isinstance(instance, urllib3.connectionpool.HTTPSConnectionPool): + kvs["url"] = f"https://{url}" + else: + kvs["url"] = f"http://{url}" except Exception: logger.debug("urllib3 _collect_kvs error: ", exc_info=True) return kvs diff --git a/src/instana/util/span_utils.py b/src/instana/util/span_utils.py index 20209a62..e736be0d 100644 --- a/src/instana/util/span_utils.py +++ b/src/instana/util/span_utils.py @@ -18,6 +18,7 @@ def matches_rule(rule_attributes: List[Any], span_attributes: List[Any]) -> bool if key == "category": if ( "type" in span_attributes + and span_attributes["type"] is not None and span_attributes["type"] in SPAN_TYPE_TO_CATEGORY ): actual = SPAN_TYPE_TO_CATEGORY[span_attributes["type"]] @@ -51,6 +52,10 @@ def matches_rule(rule_attributes: List[Any], span_attributes: List[Any]) -> bool def match_key_filter(span_value: str, rule_value: str, match_type: str) -> bool: """Check if the first value matches the second value based on the match type.""" + # Guard against None values + if span_value is None: + return False + if rule_value == "*": return True elif match_type == "strict" and span_value == rule_value: diff --git a/tests/agent/test_host.py b/tests/agent/test_host.py index e89c47ce..7cd1da3e 100644 --- a/tests/agent/test_host.py +++ b/tests/agent/test_host.py @@ -777,4 +777,30 @@ def test_set_from_missing_required_keys( assert agent.announce_data is None assert "Missing required keys in announce response" in caplog.messages[-1] - assert str(res_data) in caplog.messages[-1] + + def test_filter_spans_with_empty_service_name(self) -> None: + """Test that filter_spans handles spans with empty service_name gracefully.""" + # Create a mock span with no valid service name in data + mock_span = Mock() + mock_span.n = "test" + mock_span.k = 1 + mock_span.data = { + "invalid_key": "value" + } # No dict value, so service_name stays empty + + # Should not crash and should include the span + filtered = self.agent.filter_spans([mock_span]) + assert len(filtered) == 1 + assert filtered[0] == mock_span + + def test_filter_spans_with_none_kind(self) -> None: + """Test that filter_spans handles spans with None kind gracefully.""" + # Create a mock span without 'k' attribute + mock_span = Mock() + mock_span.n = "http" + del mock_span.k # Remove k attribute + mock_span.data = {"http": {"method": "GET", "url": "http://example.com"}} + + # Should not crash - getattr will return None for missing k + filtered = self.agent.filter_spans([mock_span]) + assert len(filtered) == 1 diff --git a/tests/clients/test_urllib3.py b/tests/clients/test_urllib3.py index 3cdab441..9a740b50 100644 --- a/tests/clients/test_urllib3.py +++ b/tests/clients/test_urllib3.py @@ -1035,5 +1035,18 @@ def test_internal_span_creation_with_url_in_path(self) -> None: test_span = filtered_spans[0] assert test_span.data["sdk"]["name"] == "test" - urllib3_spans = [span for span in filtered_spans if span.n == "urllib3"] - assert len(urllib3_spans) == 0 + def test_collect_kvs_with_none_host(self) -> None: + """Test that _collect_kvs handles None host gracefully without crashing.""" + # Create a mock connection pool with None host + pool = urllib3.HTTPConnectionPool(host="example.com", port=80) + pool.host = None # Simulate edge case where host becomes None + + # Call _collect_kvs - should not crash + kvs = collect_kvs(pool, ("GET", "/test"), {}) + + # Verify that URL is not constructed when host is None + assert "url" not in kvs + assert kvs.get("host") is None + assert kvs.get("port") == 80 + assert kvs.get("method") == "GET" + assert kvs.get("path") == "/test" diff --git a/tests/util/test_span_utils.py b/tests/util/test_span_utils.py index 4dee18d8..0f4d248c 100644 --- a/tests/util/test_span_utils.py +++ b/tests/util/test_span_utils.py @@ -108,3 +108,39 @@ def test_matches_rule_multiple_rules(self) -> None: }, ] assert not matches_rule(rules_fail, span_attrs) + + def test_match_key_filter_with_none_value(self) -> None: + """Test that match_key_filter handles None span_value gracefully.""" + # None span_value should return False for all match types + assert not match_key_filter(None, "foo", "strict") + assert not match_key_filter(None, "foo", "contains") + assert not match_key_filter(None, "foo", "startswith") + assert not match_key_filter(None, "foo", "endswith") + assert not match_key_filter(None, "*", "strict") + + def test_matches_rule_with_none_type_in_category(self) -> None: + """Test that matches_rule handles None type when checking category.""" + # When type is None, category check should not match + span_attrs_none_type = {"type": None} + rule_category = [{"key": "category", "values": ["databases"]}] + assert not matches_rule(rule_category, span_attrs_none_type) + + # When type is missing, category check should not match + span_attrs_no_type = {} + assert not matches_rule(rule_category, span_attrs_no_type) + + def test_matches_rule_with_none_attribute_value(self) -> None: + """Test that matches_rule handles None attribute values gracefully.""" + # When an attribute value is None, it should not match + span_attrs = {"http.url": None, "http.method": "GET"} + + rule_url = [ + {"key": "http.url", "values": ["example.com"], "match_type": "contains"} + ] + assert not matches_rule(rule_url, span_attrs) + + # But other attributes should still match + rule_method = [ + {"key": "http.method", "values": ["GET"], "match_type": "strict"} + ] + assert matches_rule(rule_method, span_attrs)