Improve observability in EmbeddingService model lifecycle in #193

[tanii1125]

Related Issue

• Supports BUG:Race Condition Risk in EmbeddingService Model Initialization #193

📝 Description

This PR adds logging and observability around EmbeddingService model initialization and reuse.
It does not modify model loading logic, concurrency, or locking.

🔧 Changes Made

added logs like :

In __init__ of class EmbeddingService -

class EmbeddingService:
   .
   .
    def __init__(self, model_name: str = MODEL_NAME, device: str = EMBEDDING_DEVICE):
        .
        .
        self._model_loading = False
        self._model_access_count = 0
        .
        .
    @property

Added access count of model -

def model(self) -> SentenceTransformer:
        ## track how often model is accessed
        self._model_access_count+=1
        logger.debug(
            f"EmbeddingService.model accessed "    
            f"(access count={self._model_access_count}, "
            f"model_loaded={self._model is not None})"
        )

Case 1 : if self._model is None:

Set _model_loading to True before model initialization to detect concurrent access. -

if self._model is None:
            # Detect concurrent initialization attempts (observability only)
            if self._model_loading:
                logger.warning(
                    "Concurrent access detected while embedding model is initializing. "
                    "This may indicate a race condition."
                )

            self._model_loading = True

and after loading made it false -

            finally:
                self._model_loading = False

Case 2 : if self._model is None: is False (or else simply)

else:
    logger.debug("Reusing existing embedding model instance from cache.")

Notes

This PR is intended to complement the race-condition fix discussed in #193 by improving visibility
into model lifecycle behavior under concurrent access.

✅ Checklist

• I have read the contributing guidelines.

Summary by CodeRabbit

Release Notes

• Refactor
  • Enhanced internal state management and error handling for the embedding service backend to improve system reliability and observability during model initialization and access.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The embedding service now includes internal state tracking with model loading and access counters. Enhancements add concurrency detection during lazy-loading, emit debug logs tracking access counts and load state, and improve error handling with enhanced logging throughout the initialization flow.

Changes

Cohort / File(s)

Summary

Embedding Service Observability backend/app/services/embedding_service/service.py

Added _model_loading (bool) and _model_access_count (int) state tracking; enhanced model property access to log debug info with access counts and load state; improved lazy-loading with concurrency detection via warning logs and safe state reset in finally block; refined initialization flow logging (no cache found, start, completion with embedding dimension); added explicit reuse logging for cached models; improved error handling with exc_info in logs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰✨ Internal states now dance and play,
Access counts light up the way,
Concurrent loading foes we see,
With logs that make debugging spree,
State tracking hops with grace and cheer! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: adding logging and observability improvements to the EmbeddingService model lifecycle, which aligns with the file changes and PR objectives.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

backend/app/services/embedding_service/service.py (2)

43-49: Add spacing around the += operator.

The access tracking logic is good for observability. However, line 44 is missing spaces around the += operator.

🔎 Style fix

-        self._model_access_count+=1
+        self._model_access_count += 1

---

73-75: Simplify exception string conversion.

The addition of exc_info=True is excellent for debugging. However, str(e) in the f-string is redundant since the exception will be automatically converted to a string.

🔎 Simplification

-                logger.error(f"Error loading model {self.model_name}: {str(e)}",
+                logger.error(f"Error loading model {self.model_name}: {e}",
                 exc_info=True
                 )

As per static analysis hint RUF010.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8eeacad and a4a0f72.

📒 Files selected for processing (1)

• backend/app/services/embedding_service/service.py

🧰 Additional context used

🪛 Ruff (0.14.10)

backend/app/services/embedding_service/service.py

73-73: Use explicit conversion flag

Replace with conversion flag

(RUF010)

🔇 Additional comments (5)

backend/app/services/embedding_service/service.py (5)

37-38: Good addition for observability.

These tracking fields will help surface concurrent access patterns and model lifecycle behavior.

---

54-58: Effective observability for concurrent initialization attempts.

This warning will help surface race conditions during concurrent model access. As noted in the comment, this is observation-only and complements the separate fix planned for issue #193.

---

60-71: Excellent use of finally block and enhanced logging.

The flag management with a finally block ensures _model_loading is always reset, and the detailed logging messages provide clear visibility into the model lifecycle.

Also applies to: 77-78

---

79-83: Great addition for complete observability.

Logging model reuse alongside initialization provides a complete picture of the model lifecycle and helps distinguish between cached and fresh model access.

---

37-38: Note: Access counter may be inaccurate under concurrent access.

Since _model_access_count is incremented without synchronization, concurrent accesses may result in lost increments and an undercount. This is acceptable for observability purposes where exact counts aren't critical, but worth noting for interpretation of the logs.

If precise counting becomes important in the future, consider using threading.Lock or atomic operations. For now, this provides useful approximate metrics.

Also applies to: 43-49

[tanii1125]

@coderabbitai generate docstrings

[coderabbitai]

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #203