Skip to content

Commit d2344ff

Browse files
authored
chore: improve code quality and production readiness (#31)
1 parent 7f7e5b6 commit d2344ff

File tree

9 files changed

+49
-54
lines changed

9 files changed

+49
-54
lines changed

drift/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@
3333
from .instrumentation.flask import FlaskInstrumentation
3434
from .instrumentation.requests import RequestsInstrumentation
3535
from .instrumentation.urllib3 import Urllib3Instrumentation
36+
from .version import SDK_VERSION
3637

37-
__version__ = "0.1.0"
38+
__version__ = SDK_VERSION
3839

3940
__all__ = [
4041
# Core

drift/core/batch_processor.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ def __init__(
5757
self._config = config or BatchSpanProcessorConfig()
5858
self._queue: deque[CleanSpanData] = deque(maxlen=self._config.max_queue_size)
5959
self._lock = threading.Lock()
60+
self._condition = threading.Condition(self._lock)
6061
self._shutdown_event = threading.Event()
6162
self._export_thread: threading.Thread | None = None
6263
self._started = False
@@ -88,6 +89,9 @@ def stop(self, timeout: float | None = None) -> None:
8889
return
8990

9091
self._shutdown_event.set()
92+
# Wake up the export thread so it can see the shutdown event
93+
with self._condition:
94+
self._condition.notify_all()
9195

9296
if self._export_thread is not None:
9397
self._export_thread.join(timeout=timeout or self._config.export_timeout_seconds)
@@ -108,7 +112,7 @@ def add_span(self, span: CleanSpanData) -> bool:
108112
Returns:
109113
True if span was added, False if queue is full and span was dropped
110114
"""
111-
with self._lock:
115+
with self._condition:
112116
if len(self._queue) >= self._config.max_queue_size:
113117
self._dropped_spans += 1
114118
logger.warning(
@@ -121,16 +125,17 @@ def add_span(self, span: CleanSpanData) -> bool:
121125

122126
# Trigger immediate export if batch size reached
123127
if len(self._queue) >= self._config.max_export_batch_size:
124-
# Signal export thread to wake up (if using condition variable)
125-
pass
128+
self._condition.notify()
126129

127130
return True
128131

129132
def _export_loop(self) -> None:
130133
"""Background thread that periodically exports spans."""
131134
while not self._shutdown_event.is_set():
132-
# Wait for scheduled delay or shutdown
133-
self._shutdown_event.wait(timeout=self._config.scheduled_delay_seconds)
135+
# Wait for either: batch size reached, scheduled delay, or shutdown
136+
with self._condition:
137+
# Wait until batch is ready or timeout
138+
self._condition.wait(timeout=self._config.scheduled_delay_seconds)
134139

135140
if self._shutdown_event.is_set():
136141
break
@@ -141,7 +146,7 @@ def _export_batch(self) -> None:
141146
"""Export a batch of spans from the queue."""
142147
# Get batch of spans
143148
batch: list[CleanSpanData] = []
144-
with self._lock:
149+
with self._condition:
145150
while self._queue and len(batch) < self._config.max_export_batch_size:
146151
batch.append(self._queue.popleft())
147152

@@ -171,7 +176,7 @@ def _export_batch(self) -> None:
171176
def _force_flush(self) -> None:
172177
"""Force export all remaining spans in the queue."""
173178
while True:
174-
with self._lock:
179+
with self._condition:
175180
if not self._queue:
176181
break
177182

@@ -180,7 +185,7 @@ def _force_flush(self) -> None:
180185
@property
181186
def queue_size(self) -> int:
182187
"""Get the current queue size."""
183-
with self._lock:
188+
with self._condition:
184189
return len(self._queue)
185190

186191
@property

drift/core/communication/communicator.py

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ def connect_sync(
248248
ConnectionError: If connection fails
249249
TimeoutError: If connection times out
250250
"""
251-
logger.info("[CONNECT_SYNC] Starting synchronous connection")
252251
# Determine address
253252
if connection_info:
254253
if "socketPath" in connection_info:
@@ -319,8 +318,6 @@ def connect_sync(
319318
if response.success:
320319
logger.debug("CLI acknowledged connection successfully")
321320
self._connected = True
322-
logger.info(f"[CONNECT_SYNC] Connection successful! Socket is: {self._socket}")
323-
logger.info(f"[CONNECT_SYNC] _connected={self._connected}, is_connected={self.is_connected}")
324321
else:
325322
error_msg = response.error or "Unknown error"
326323
raise ConnectionError(f"CLI rejected connection: {error_msg}")
@@ -336,8 +333,6 @@ def connect_sync(
336333
finally:
337334
calling_library_context.reset(context_token)
338335

339-
logger.info(f"[CONNECT_SYNC] Exiting connect_sync(). Socket still open: {self._socket is not None}")
340-
341336
async def disconnect(self) -> None:
342337
"""Disconnect from CLI."""
343338
self._cleanup()
@@ -509,8 +504,9 @@ async def send_unpatched_dependency_alert(
509504

510505
try:
511506
await self._send_protobuf_message(sdk_message)
512-
except Exception:
513-
pass # Alerts are non-critical
507+
except Exception as e:
508+
# Alerts are non-critical, just log at debug level
509+
logger.debug(f"Failed to send unpatched dependency alert: {e}")
514510

515511
async def _send_protobuf_message(self, message: SdkMessage) -> None:
516512
"""Send a protobuf message to CLI."""
@@ -765,21 +761,16 @@ def _clean_span(self, data: Any) -> Any:
765761

766762
def _cleanup(self) -> None:
767763
"""Clean up resources."""
768-
import traceback
769-
770-
logger.warning("[CLEANUP] _cleanup() called! Stack trace:")
771-
logger.warning("".join(traceback.format_stack()))
772-
773764
self._connected = False
774765
self._session_id = None
775766
self._incoming_buffer.clear()
776767

777768
if self._socket:
778769
try:
779-
logger.warning("[CLEANUP] Closing socket")
780770
self._socket.close()
781-
except Exception:
782-
pass
771+
except OSError as e:
772+
# Socket may already be closed, which is fine
773+
logger.debug(f"Error closing socket during cleanup: {e}")
783774
self._socket = None
784775

785776
self._pending_requests.clear()

drift/core/config.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,17 @@ def _parse_service_config(data: dict[str, Any]) -> ServiceConfig:
135135

136136
def _parse_recording_config(data: dict[str, Any]) -> RecordingConfig:
137137
"""Parse recording configuration from raw dict."""
138+
# Validate sampling_rate type
139+
sampling_rate = data.get("sampling_rate")
140+
if sampling_rate is not None and not isinstance(sampling_rate, (int, float)):
141+
logger.warning(
142+
f"Invalid 'sampling_rate' in config: expected number, got {type(sampling_rate).__name__}. "
143+
"This value will be ignored."
144+
)
145+
sampling_rate = None
146+
138147
return RecordingConfig(
139-
sampling_rate=data.get("sampling_rate"),
148+
sampling_rate=sampling_rate,
140149
export_spans=data.get("export_spans"),
141150
enable_env_var_recording=data.get("enable_env_var_recording"),
142151
enable_analytics=data.get("enable_analytics"),

drift/core/drift_sdk.py

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,6 @@ def _init_communicator_for_replay(self) -> None:
354354
self._is_connected_with_cli = False
355355
# Don't raise - allow Django to start even if CLI isn't ready yet
356356

357-
logger.info(f"[INIT_COMPLETE] SDK initialization finished. Connected to CLI: {self._is_connected_with_cli}")
358-
if self.communicator and self.communicator.is_connected:
359-
logger.info("[INIT_COMPLETE] Communicator reports connected: True")
360-
else:
361-
logger.info("[INIT_COMPLETE] Communicator reports connected: False")
362-
363357
def _init_auto_instrumentations(self) -> None:
364358
"""Initialize instrumentations."""
365359
try:
@@ -645,10 +639,6 @@ def request_mock_sync(self, mock_request: MockRequestInput) -> MockResponseOutpu
645639
logger.error("Requesting sync mock but CLI is not ready yet")
646640
return MockResponseOutput(found=False, error="CLI not connected yet")
647641

648-
if not self._is_connected_with_cli:
649-
logger.error("Requesting sync mock but CLI is not ready yet")
650-
raise RuntimeError("Requesting sync mock but CLI is not ready yet")
651-
652642
try:
653643
logger.debug(f"Sending protobuf request to CLI (sync) {mock_request.test_id}")
654644
response = self.communicator.request_mock_sync(mock_request)
@@ -685,8 +675,8 @@ async def send_instrumentation_version_mismatch_alert(
685675
await self.communicator.send_instrumentation_version_mismatch_alert(
686676
module_name, requested_version, supported_versions
687677
)
688-
except Exception:
689-
pass
678+
except Exception as e:
679+
logger.debug(f"Failed to send version mismatch alert: {e}")
690680

691681
async def send_unpatched_dependency_alert(
692682
self,
@@ -699,8 +689,8 @@ async def send_unpatched_dependency_alert(
699689

700690
try:
701691
await self.communicator.send_unpatched_dependency_alert(stack_trace, trace_test_server_span_id)
702-
except Exception:
703-
pass
692+
except Exception as e:
693+
logger.debug(f"Failed to send unpatched dependency alert: {e}")
704694

705695
def get_sampling_rate(self) -> float:
706696
"""Get the current sampling rate."""

drift/core/sampling.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
from __future__ import annotations
44

5+
import logging
56
import random
67

8+
logger = logging.getLogger(__name__)
9+
710

811
def should_sample(sampling_rate: float, is_app_ready: bool) -> bool:
912
"""
@@ -40,12 +43,8 @@ def validate_sampling_rate(rate: float | None, source: str = "config") -> float
4043
if rate is None:
4144
return None
4245

43-
if not isinstance(rate, (int, float)):
44-
print(f"Warning: Invalid sampling rate from {source}: not a number. Ignoring.")
45-
return None
46-
4746
if rate < 0.0 or rate > 1.0:
48-
print(f"Warning: Invalid sampling rate from {source}: {rate}. Must be between 0.0 and 1.0. Ignoring.")
47+
logger.warning(f"Invalid sampling rate from {source}: {rate}. Must be between 0.0 and 1.0. Ignoring.")
4948
return None
5049

5150
return float(rate)

drift/instrumentation/registry.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
import sys
55
from collections.abc import Callable, Sequence
66
from types import ModuleType
7-
from typing import override
7+
from typing import TypeVar, override
88

99
PatchFn = Callable[[ModuleType], None]
10+
T = TypeVar("T")
1011

1112
_registry: dict[str, PatchFn] = {}
1213
_installed = False
@@ -91,11 +92,6 @@ def _apply_patch(module: ModuleType, patch_fn: PatchFn) -> None:
9192
module.__drift_patched__ = True # type: ignore[attr-defined]
9293

9394

94-
from typing import TypeVar
95-
96-
T = TypeVar("T")
97-
98-
9995
def patch_instances_via_gc[T](class_type: type, patch_instance_fn: Callable[[T], None]) -> None:
10096
"""Use gc to patch instances created before SDK initialization"""
10197
for obj in gc.get_objects(): # pyright: ignore[reportAny]

drift/version.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
"""Version information for the Drift Python SDK."""
22

3-
# SDK version - should match package version
4-
SDK_VERSION = "1.0.0"
3+
import importlib.metadata
4+
5+
try:
6+
SDK_VERSION = importlib.metadata.version("tusk-drift-python-sdk")
7+
except importlib.metadata.PackageNotFoundError:
8+
SDK_VERSION = "0.0.0.dev"
59

610
# Minimum CLI version required for this SDK
711
MIN_CLI_VERSION = "0.1.0"

pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ dev = [
5252
[project.urls]
5353
Homepage = "https://usetusk.ai"
5454
Documentation = "https://docs.usetusk.ai"
55-
Repository = "https://github.com/Use-Tusk/drift-node-sdk"
56-
Issues = "https://github.com/Use-Tusk/drift-node-sdk/issues"
55+
Repository = "https://github.com/Use-Tusk/drift-python-sdk"
56+
Issues = "https://github.com/Use-Tusk/drift-python-sdk/issues"
5757

5858
[tool.setuptools.packages.find]
5959
where = ["."]

0 commit comments

Comments
 (0)