Skip to content

Commit 9be7b28

Browse files
committed
fix: Apply PR#861 review suggestions.
- Introduced type hints and code cleanup. - removed None defaults from .get() calls. - import reorganization. - boolean expression formatting. Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
1 parent aec920b commit 9be7b28

File tree

4 files changed

+67
-43
lines changed

4 files changed

+67
-43
lines changed

src/instana/collector/helpers/fargate/docker.py

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55

66
from __future__ import division
77

8-
from ....log import logger
9-
from ....util import DictionaryOfStan
10-
from ..base import BaseHelper
8+
from typing import Any, Type
9+
10+
from instana.collector.base import BaseCollector
11+
from instana.collector.helpers.base import BaseHelper
12+
from instana.log import logger
13+
from instana.util import DictionaryOfStan
1114

1215

1316
class DockerHelper(BaseHelper):
1417
"""This class acts as a helper to collect Docker snapshot and metric information"""
1518

16-
def __init__(self, collector):
19+
def __init__(self, collector: Type[BaseCollector]) -> None:
1720
super(DockerHelper, self).__init__(collector)
1821

1922
# The metrics from the previous report cycle
@@ -23,7 +26,7 @@ def __init__(self, collector):
2326
# Indexed by docker_id: self.previous_blkio[docker_id][metric]
2427
self.previous_blkio = DictionaryOfStan()
2528

26-
def collect_metrics(self, **kwargs):
29+
def collect_metrics(self, **kwargs: Any) -> list[dict[str, Any]]:
2730
"""
2831
Collect and return docker metrics (and optionally snapshot data) for this task
2932
@return: list - with one or more plugin entities
@@ -33,7 +36,7 @@ def collect_metrics(self, **kwargs):
3336
if self.collector.task_metadata is not None:
3437
containers = self.collector.task_metadata.get("Containers", [])
3538
for container in containers:
36-
plugin_data = dict()
39+
plugin_data = {}
3740
plugin_data["name"] = "com.instana.plugin.docker"
3841
docker_id = container.get("DockerId")
3942

@@ -43,7 +46,7 @@ def collect_metrics(self, **kwargs):
4346

4447
plugin_data["entityId"] = f"{task_arn}::{name}"
4548
plugin_data["data"] = DictionaryOfStan()
46-
plugin_data["data"]["Id"] = container.get("DockerId", None)
49+
plugin_data["data"]["Id"] = container.get("DockerId")
4750

4851
with_snapshot = kwargs.get("with_snapshot", False)
4952
# Metrics
@@ -61,25 +64,27 @@ def collect_metrics(self, **kwargs):
6164
logger.debug("DockerHelper.collect_metrics: ", exc_info=True)
6265
return plugins
6366

64-
def _collect_container_snapshot(self, plugin_data, container):
67+
def _collect_container_snapshot(
68+
self, plugin_data: dict[str, Any], container: dict[str, Any]
69+
) -> None:
6570
try:
6671
# Snapshot Data
67-
plugin_data["data"]["Created"] = container.get("CreatedAt", None)
68-
plugin_data["data"]["Started"] = container.get("StartedAt", None)
69-
plugin_data["data"]["Image"] = container.get("Image", None)
70-
plugin_data["data"]["Labels"] = container.get("Labels", None)
71-
plugin_data["data"]["Ports"] = container.get("Ports", None)
72+
plugin_data["data"]["Created"] = container.get("CreatedAt")
73+
plugin_data["data"]["Started"] = container.get("StartedAt")
74+
plugin_data["data"]["Image"] = container.get("Image")
75+
plugin_data["data"]["Labels"] = container.get("Labels")
76+
plugin_data["data"]["Ports"] = container.get("Ports")
7277

7378
networks = container.get("Networks", [])
7479
if len(networks) >= 1:
75-
plugin_data["data"]["NetworkMode"] = networks[0].get(
76-
"NetworkMode", None
77-
)
80+
plugin_data["data"]["NetworkMode"] = networks[0].get("NetworkMode")
7881
except Exception:
7982
logger.debug("_collect_container_snapshot: ", exc_info=True)
8083

81-
def _collect_container_metrics(self, plugin_data, docker_id, with_snapshot):
82-
container = self.collector.task_stats_metadata.get(docker_id, None)
84+
def _collect_container_metrics(
85+
self, plugin_data: dict[str, Any], docker_id: str, with_snapshot: bool
86+
) -> None:
87+
container = self.collector.task_stats_metadata.get(docker_id)
8388
if container is not None:
8489
self._collect_network_metrics(
8590
container, plugin_data, docker_id, with_snapshot
@@ -93,10 +98,14 @@ def _collect_container_metrics(self, plugin_data, docker_id, with_snapshot):
9398
)
9499

95100
def _collect_network_metrics(
96-
self, container, plugin_data, docker_id, with_snapshot
97-
):
101+
self,
102+
container: dict[str, Any],
103+
plugin_data: dict[str, Any],
104+
docker_id: str,
105+
with_snapshot: bool,
106+
) -> None:
98107
try:
99-
networks = container.get("networks", None)
108+
networks = container.get("networks")
100109
tx_bytes_total = tx_dropped_total = tx_errors_total = tx_packets_total = 0
101110
rx_bytes_total = rx_dropped_total = rx_errors_total = rx_packets_total = 0
102111

@@ -173,11 +182,17 @@ def _collect_network_metrics(
173182
except Exception:
174183
logger.debug("_collect_network_metrics: ", exc_info=True)
175184

176-
def _collect_cpu_metrics(self, container, plugin_data, docker_id, with_snapshot):
185+
def _collect_cpu_metrics(
186+
self,
187+
container: dict[str, Any],
188+
plugin_data: dict[str, Any],
189+
docker_id: str,
190+
with_snapshot: bool,
191+
) -> None:
177192
try:
178193
cpu_stats = container.get("cpu_stats", {})
179-
cpu_usage = cpu_stats.get("cpu_usage", None)
180-
throttling_data = cpu_stats.get("throttling_data", None)
194+
cpu_usage = cpu_stats.get("cpu_usage")
195+
throttling_data = cpu_stats.get("throttling_data")
181196

182197
if cpu_usage is not None:
183198
online_cpus = cpu_stats.get("online_cpus", 1)
@@ -234,10 +249,16 @@ def _collect_cpu_metrics(self, container, plugin_data, docker_id, with_snapshot)
234249
except Exception:
235250
logger.debug("_collect_cpu_metrics: ", exc_info=True)
236251

237-
def _collect_memory_metrics(self, container, plugin_data, docker_id, with_snapshot):
252+
def _collect_memory_metrics(
253+
self,
254+
container: dict[str, Any],
255+
plugin_data: dict[str, Any],
256+
docker_id: str,
257+
with_snapshot: bool,
258+
) -> None:
238259
try:
239260
memory = container.get("memory_stats", {})
240-
memory_stats = memory.get("stats", None)
261+
memory_stats = memory.get("stats")
241262

242263
self.apply_delta(
243264
memory,
@@ -307,11 +328,17 @@ def _collect_memory_metrics(self, container, plugin_data, docker_id, with_snapsh
307328
except Exception:
308329
logger.debug("_collect_memory_metrics: ", exc_info=True)
309330

310-
def _collect_blkio_metrics(self, container, plugin_data, docker_id, with_snapshot):
331+
def _collect_blkio_metrics(
332+
self,
333+
container: dict[str, Any],
334+
plugin_data: dict[str, Any],
335+
docker_id: str,
336+
with_snapshot: bool,
337+
) -> None:
311338
try:
312-
blkio_stats = container.get("blkio_stats", None)
339+
blkio_stats = container.get("blkio_stats")
313340
if blkio_stats is not None:
314-
service_bytes = blkio_stats.get("io_service_bytes_recursive", None)
341+
service_bytes = blkio_stats.get("io_service_bytes_recursive")
315342
if service_bytes is not None:
316343
for entry in service_bytes:
317344
if entry["op"] == "Read":

src/instana/util/config.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ def parse_span_disabling_dict(items: Dict[str, bool]) -> Tuple[List[str], List[s
348348

349349
def get_disable_trace_configurations_from_env() -> Tuple[List[str], List[str]]:
350350
# Read INSTANA_TRACING_DISABLE environment variable
351-
if tracing_disable := os.environ.get("INSTANA_TRACING_DISABLE", None):
351+
if tracing_disable := os.environ.get("INSTANA_TRACING_DISABLE"):
352352
if is_truthy(tracing_disable):
353353
# INSTANA_TRACING_DISABLE is True/true/1, then we disable all tracing
354354
disabled_spans = []
@@ -388,14 +388,14 @@ def get_disable_trace_configurations_from_yaml() -> Tuple[List[str], List[str]]:
388388
if not root_key:
389389
return [], []
390390

391-
if tracing_disable_config := config_reader.data[root_key].get("disable", None):
391+
if tracing_disable_config := config_reader.data[root_key].get("disable"):
392392
return parse_span_disabling(tracing_disable_config)
393393
return [], []
394394

395395

396396
def get_disable_trace_configurations_from_local() -> Tuple[List[str], List[str]]:
397397
if "tracing" in config and (
398-
tracing_disable_config := config["tracing"].get("disable", None)
398+
tracing_disable_config := config["tracing"].get("disable")
399399
):
400400
return parse_span_disabling(tracing_disable_config)
401401
return [], []

src/instana/util/span_utils.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,10 @@ def match_key_filter(span_value: str, rule_value: str, match_type: str) -> bool:
5757

5858
return bool(
5959
rule_value == "*"
60-
or match_type == "strict"
61-
and span_value == rule_value
62-
or match_type == "contains"
63-
and rule_value in span_value
64-
or match_type == "startswith"
65-
and span_value.startswith(rule_value)
66-
or match_type == "endswith"
67-
and span_value.endswith(rule_value)
60+
or (match_type == "strict" and span_value == rule_value)
61+
or (match_type == "contains" and rule_value in span_value)
62+
or (match_type == "startswith" and span_value.startswith(rule_value))
63+
or (match_type == "endswith" and span_value.endswith(rule_value))
6864
)
6965

7066

tests/apps/fastapi_app/__init__.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,18 @@
22
# (c) Copyright Instana Inc. 2020
33

44
import uvicorn
5-
from ...helpers import testenv
6-
from instana.log import logger as logger
5+
6+
from tests.helpers import testenv
77

88
testenv["fastapi_port"] = 10816
99
testenv["fastapi_server"] = "http://127.0.0.1:" + str(testenv["fastapi_port"])
1010

1111

1212
def launch_fastapi():
13-
from .app import fastapi_server
1413
from instana.singletons import agent
1514

15+
from .app import fastapi_server
16+
1617
# Hack together a manual custom headers list; We'll use this in tests
1718
agent.options.extra_http_headers = [
1819
"X-Capture-This",

0 commit comments

Comments
 (0)