Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/instana/agent/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@
return response
return

def filter_spans(self, spans: List[Dict[str, Any]]) -> List[Dict[str, Any]]:

Check failure on line 358 in src/instana/agent/host.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=instana_python-sensor&issues=AZzdEuJPdc-jiK-fvdw2&open=AZzdEuJPdc-jiK-fvdw2&pullRequest=849
"""
Filters span list using new hierarchical filtering rules.
"""
Expand All @@ -375,10 +375,15 @@
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
Expand Down
12 changes: 7 additions & 5 deletions src/instana/instrumentation/urllib3.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
try:
import urllib3

def _collect_kvs(

Check failure on line 25 in src/instana/instrumentation/urllib3.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=instana_python-sensor&issues=AZzdEuGVdc-jiK-fvdw1&open=AZzdEuGVdc-jiK-fvdw1&pullRequest=849
instance: Union[
urllib3.connectionpool.HTTPConnectionPool,
urllib3.connectionpool.HTTPSConnectionPool,
Expand Down Expand Up @@ -55,11 +55,13 @@
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
Expand Down
5 changes: 5 additions & 0 deletions src/instana/util/span_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]]
Expand Down Expand Up @@ -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:
Expand Down
28 changes: 27 additions & 1 deletion tests/agent/test_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 15 additions & 2 deletions tests/clients/test_urllib3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
36 changes: 36 additions & 0 deletions tests/util/test_span_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading