Skip to content

Commit ebb31af

Browse files
committed
fix: Add null checks for span filtering attributes
Signed-off-by: Cagri Yonca <cagri@ibm.com>
1 parent bd5bbe8 commit ebb31af

6 files changed

Lines changed: 96 additions & 9 deletions

File tree

src/instana/agent/host.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,15 @@ def filter_spans(self, spans: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
375375
if isinstance(span.data[span_value], dict):
376376
service_name = span_value
377377

378+
# Skip if no valid service name found
379+
if not service_name:
380+
filtered_spans.append(span)
381+
continue
382+
378383
# Set span attributes for filtering
379384
attributes_to_check = {
380385
"type": service_name,
381-
"kind": span.k,
386+
"kind": getattr(span, "k", None),
382387
}
383388

384389
# Add operation specifiers to the attributes

src/instana/instrumentation/urllib3.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,13 @@ def _collect_kvs(
5555
agent.options.secrets_list,
5656
)
5757

58-
url = kvs["host"] + ":" + str(kvs["port"]) + kvs["path"]
59-
if isinstance(instance, urllib3.connectionpool.HTTPSConnectionPool):
60-
kvs["url"] = f"https://{url}"
61-
else:
62-
kvs["url"] = f"http://{url}"
58+
# Only construct URL if host is not None
59+
if kvs.get("host") and kvs.get("path"):
60+
url = f"{kvs["host"]}:{kvs["port"]}{kvs["path"]}"
61+
if isinstance(instance, urllib3.connectionpool.HTTPSConnectionPool):
62+
kvs["url"] = f"https://{url}"
63+
else:
64+
kvs["url"] = f"http://{url}"
6365
except Exception:
6466
logger.debug("urllib3 _collect_kvs error: ", exc_info=True)
6567
return kvs

src/instana/util/span_utils.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ def matches_rule(rule_attributes: List[Any], span_attributes: List[Any]) -> bool
1818
if key == "category":
1919
if (
2020
"type" in span_attributes
21+
and span_attributes["type"] is not None
2122
and span_attributes["type"] in SPAN_TYPE_TO_CATEGORY
2223
):
2324
actual = SPAN_TYPE_TO_CATEGORY[span_attributes["type"]]
@@ -51,6 +52,10 @@ def matches_rule(rule_attributes: List[Any], span_attributes: List[Any]) -> bool
5152

5253
def match_key_filter(span_value: str, rule_value: str, match_type: str) -> bool:
5354
"""Check if the first value matches the second value based on the match type."""
55+
# Guard against None values
56+
if span_value is None:
57+
return False
58+
5459
if rule_value == "*":
5560
return True
5661
elif match_type == "strict" and span_value == rule_value:

tests/agent/test_host.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -777,4 +777,30 @@ def test_set_from_missing_required_keys(
777777

778778
assert agent.announce_data is None
779779
assert "Missing required keys in announce response" in caplog.messages[-1]
780-
assert str(res_data) in caplog.messages[-1]
780+
781+
def test_filter_spans_with_empty_service_name(self) -> None:
782+
"""Test that filter_spans handles spans with empty service_name gracefully."""
783+
# Create a mock span with no valid service name in data
784+
mock_span = Mock()
785+
mock_span.n = "test"
786+
mock_span.k = 1
787+
mock_span.data = {
788+
"invalid_key": "value"
789+
} # No dict value, so service_name stays empty
790+
791+
# Should not crash and should include the span
792+
filtered = self.agent.filter_spans([mock_span])
793+
assert len(filtered) == 1
794+
assert filtered[0] == mock_span
795+
796+
def test_filter_spans_with_none_kind(self) -> None:
797+
"""Test that filter_spans handles spans with None kind gracefully."""
798+
# Create a mock span without 'k' attribute
799+
mock_span = Mock()
800+
mock_span.n = "http"
801+
del mock_span.k # Remove k attribute
802+
mock_span.data = {"http": {"method": "GET", "url": "http://example.com"}}
803+
804+
# Should not crash - getattr will return None for missing k
805+
filtered = self.agent.filter_spans([mock_span])
806+
assert len(filtered) == 1

tests/clients/test_urllib3.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,5 +1035,18 @@ def test_internal_span_creation_with_url_in_path(self) -> None:
10351035
test_span = filtered_spans[0]
10361036
assert test_span.data["sdk"]["name"] == "test"
10371037

1038-
urllib3_spans = [span for span in filtered_spans if span.n == "urllib3"]
1039-
assert len(urllib3_spans) == 0
1038+
def test_collect_kvs_with_none_host(self) -> None:
1039+
"""Test that _collect_kvs handles None host gracefully without crashing."""
1040+
# Create a mock connection pool with None host
1041+
pool = urllib3.HTTPConnectionPool(host="example.com", port=80)
1042+
pool.host = None # Simulate edge case where host becomes None
1043+
1044+
# Call _collect_kvs - should not crash
1045+
kvs = collect_kvs(pool, ("GET", "/test"), {})
1046+
1047+
# Verify that URL is not constructed when host is None
1048+
assert "url" not in kvs
1049+
assert kvs.get("host") is None
1050+
assert kvs.get("port") == 80
1051+
assert kvs.get("method") == "GET"
1052+
assert kvs.get("path") == "/test"

tests/util/test_span_utils.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,39 @@ def test_matches_rule_multiple_rules(self) -> None:
108108
},
109109
]
110110
assert not matches_rule(rules_fail, span_attrs)
111+
112+
def test_match_key_filter_with_none_value(self) -> None:
113+
"""Test that match_key_filter handles None span_value gracefully."""
114+
# None span_value should return False for all match types
115+
assert not match_key_filter(None, "foo", "strict")
116+
assert not match_key_filter(None, "foo", "contains")
117+
assert not match_key_filter(None, "foo", "startswith")
118+
assert not match_key_filter(None, "foo", "endswith")
119+
assert not match_key_filter(None, "*", "strict")
120+
121+
def test_matches_rule_with_none_type_in_category(self) -> None:
122+
"""Test that matches_rule handles None type when checking category."""
123+
# When type is None, category check should not match
124+
span_attrs_none_type = {"type": None}
125+
rule_category = [{"key": "category", "values": ["databases"]}]
126+
assert not matches_rule(rule_category, span_attrs_none_type)
127+
128+
# When type is missing, category check should not match
129+
span_attrs_no_type = {}
130+
assert not matches_rule(rule_category, span_attrs_no_type)
131+
132+
def test_matches_rule_with_none_attribute_value(self) -> None:
133+
"""Test that matches_rule handles None attribute values gracefully."""
134+
# When an attribute value is None, it should not match
135+
span_attrs = {"http.url": None, "http.method": "GET"}
136+
137+
rule_url = [
138+
{"key": "http.url", "values": ["example.com"], "match_type": "contains"}
139+
]
140+
assert not matches_rule(rule_url, span_attrs)
141+
142+
# But other attributes should still match
143+
rule_method = [
144+
{"key": "http.method", "values": ["GET"], "match_type": "strict"}
145+
]
146+
assert matches_rule(rule_method, span_attrs)

0 commit comments

Comments
 (0)