Skip to content

chore: improve code quality and production readiness#31

Merged
jy-tan merged 1 commit intomainfrom
improve-code-quality-1
Jan 14, 2026
Merged

chore: improve code quality and production readiness#31
jy-tan merged 1 commit intomainfrom
improve-code-quality-1

Conversation

@jy-tan
Copy link
Contributor

@jy-tan jy-tan commented Jan 14, 2026

Summary

Addresses several code quality issues to improve production readiness, consistency, and maintainability.

Changes

Version Management

  • Use importlib.metadata for single source of truth version (reads from pyproject.toml)
  • Fix __version__ in drift/__init__.py to use SDK_VERSION

Bug Fixes

  • Fix duplicate unreachable check in request_mock_sync()
  • Fix incorrect repository URLs in pyproject.toml (pointed to Node SDK)

Code Quality

  • Replace print() with logger.warning() in sampling validation
  • Add debug logging to silent except Exception: pass blocks
  • Use OSError instead of bare Exception for socket cleanup
  • Remove development debug logs ([CONNECT_SYNC], [INIT_COMPLETE], [CLEANUP])
  • Move TypeVar declaration to top of file in registry.py

Performance

  • Add threading.Condition to BatchSpanProcessor for efficient thread coordination
  • Export thread now wakes immediately when batch size threshold is reached

Validation

  • Add type validation for sampling_rate in config file parsing
  • Invalid config values now log a warning and fall back to defaults

Testing

  • All 143 unit tests pass
  • E2E tests verified: Flask (7), FastAPI (10), Requests (17), Redis (20), Django (7)

@jy-tan jy-tan merged commit d2344ff into main Jan 14, 2026
22 of 23 checks passed
@jy-tan jy-tan deleted the improve-code-quality-1 branch January 14, 2026 20:35
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 9 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="drift/core/batch_processor.py">

<violation number="1" location="drift/core/batch_processor.py:138">
P2: `_export_loop` always sleeps even when the queue already meets the export threshold, so condition notifications delivered while the thread wasn’t waiting are lost and batches aren’t exported immediately.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

# Wait for either: batch size reached, scheduled delay, or shutdown
with self._condition:
# Wait until batch is ready or timeout
self._condition.wait(timeout=self._config.scheduled_delay_seconds)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: _export_loop always sleeps even when the queue already meets the export threshold, so condition notifications delivered while the thread wasn’t waiting are lost and batches aren’t exported immediately.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At drift/core/batch_processor.py, line 138:

<comment>`_export_loop` always sleeps even when the queue already meets the export threshold, so condition notifications delivered while the thread wasn’t waiting are lost and batches aren’t exported immediately.</comment>

<file context>
@@ -121,16 +125,17 @@ def add_span(self, span: CleanSpanData) -> bool:
+            # Wait for either: batch size reached, scheduled delay, or shutdown
+            with self._condition:
+                # Wait until batch is ready or timeout
+                self._condition.wait(timeout=self._config.scheduled_delay_seconds)
 
             if self._shutdown_event.is_set():
</file context>
Suggested change
self._condition.wait(timeout=self._config.scheduled_delay_seconds)
if (
not self._shutdown_event.is_set()
and len(self._queue) < self._config.max_export_batch_size
):
self._condition.wait(timeout=self._config.scheduled_delay_seconds)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants