Improved logging process by removing the print statements#39
Improved logging process by removing the print statements#39Vinayak-2003 wants to merge 3 commits intoOpenVoiceX:mainfrom
Conversation
WalkthroughAdds logging across routes and services (replacing prints), enforces faster-whisper to use CPU, introduces a module-level Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Nitpick comments (3)
backend/src/services/telephony_service.py (1)
45-59: Optional: handle Twilio-specific exceptions for clearer error messagesCatching
TwilioRestExceptionfrom the SDK can provide more structured information (status, code), improving observability.# Add: from twilio.base.exceptions import TwilioRestException # Replace the generic except: except TwilioRestException as e: logger.error(f"Twilio API error: status={e.status} code={getattr(e, 'code', None)} msg={e.msg}", exc_info=True) raise except Exception as e: logger.error(f"Unexpected error during Twilio call creation: {e}", exc_info=True) raisebackend/src/api/routes/calls.py (2)
129-129: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
Apply this diff to remove the extraneous
fprefix:- log_call_event(call_sid=CallSid, event=f"➡️ Responding to Twilio with TwiML", details={"Response to Twilio": final_twiml}) + log_call_event(call_sid=CallSid, event="➡️ Responding to Twilio with TwiML", details={"Response to Twilio": final_twiml})
133-133: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
Apply this diff to remove the extraneous
fprefix:- log_call_event(call_sid=CallSid, event=f"🔥🔥🔥 UNEXPECTED ERROR IN WEBHOOK 🔥🔥🔥", details={"error": e}, level="error") + log_call_event(call_sid=CallSid, event="🔥🔥🔥 UNEXPECTED ERROR IN WEBHOOK 🔥🔥🔥", details={"error": e}, level="error")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/requirements.txt(1 hunks)backend/src/api/routes/agents.py(3 hunks)backend/src/api/routes/calls.py(4 hunks)backend/src/services/campaign_service.py(3 hunks)backend/src/services/llm_service.py(3 hunks)backend/src/services/stt_service.py(3 hunks)backend/src/services/telephony_service.py(3 hunks)backend/src/services/tts_service.py(2 hunks)backend/src/utils/logging.py(0 hunks)backend/src/utils/logging_func.py(1 hunks)
💤 Files with no reviewable changes (1)
- backend/src/utils/logging.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/src/api/routes/calls.py (2)
backend/src/utils/logging_func.py (1)
log_call_event(16-31)backend/src/utils/logging.py (1)
log_call_event(16-23)
backend/src/services/campaign_service.py (2)
scripts/test_sequential_calls.py (1)
test_sequential_calling(13-91)scripts/test_campaign.py (2)
test_campaign_functionality(13-81)test_twilio_configuration(83-110)
backend/src/utils/logging_func.py (1)
backend/src/utils/logging.py (1)
log_call_event(16-23)
🪛 Ruff (0.12.2)
backend/src/api/routes/calls.py
129-129: f-string without any placeholders
Remove extraneous f prefix
(F541)
133-133: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (2)
backend/src/utils/logging_func.py (1)
16-31: Remove incorrect reference to non-existentlogging.pyI was unable to locate any other
log_call_eventimplementation in the repo—only backend/src/utils/logging_func.py defines it. There is nobackend/src/utils/logging.pywith a differing signature, so this inconsistency concern can be dismissed.Likely an incorrect or invalid review comment.
backend/src/api/routes/calls.py (1)
10-10: Import path is correct — no changes needed.The file
backend/src/utils/logging_func.pyexists and matches the import inbackend/src/api/routes/calls.py.
No further action required.
| from ...core.database import get_db | ||
| from ...models import agent as agent_model | ||
| from ...schemas import agent as agent_schema | ||
| import logging |
There was a problem hiding this comment.
Fix logging misuse and tighten messages; minor copy edit
- Use a module logger with
.info()/.error()methods, notlogging.INFO/ERROR. - Improve log messages to include IDs and fix grammar.
import logging
+logger = logging.getLogger(__name__)
@@
- logging.INFO("A new agent has been created")
+ logger.info(f"Agent created id={db_agent.id} name={db_agent.name}")
@@
- logging.ERROR("Agent not found")
+ logger.error(f"Agent not found id={agent_id}")
@@
- logging.INFO(f"agent with the {agent_id} has been retrieved")
+ logger.info(f"Agent retrieved id={agent_id}")Also applies to: 27-27, 37-40
🤖 Prompt for AI Agents
In backend/src/api/routes/agents.py around lines 10, 27, and 37-40, replace the
misuse of the logging module constants with a module-level logger and use its
methods; add a logger via logging.getLogger(__name__) (import logging stays) and
change calls that currently use logging.INFO or logging.ERROR to
logger.info(...) and logger.error(...). While updating these calls, tighten and
correct the messages to include relevant agent or request IDs and fix grammar
(e.g., "Agent created: id=<id>" or "Failed to fetch agent id=<id>: <error>") so
logs are informative and consistent.
| self.test_mode = os.getenv('TEST_MODE', 'true').lower() == 'true' | ||
| if self.test_mode: | ||
| print("🧪 Running in TEST MODE - calls will be simulated") | ||
| logging.INFO("🧪 Running in TEST MODE - calls will be simulated") |
There was a problem hiding this comment.
Fix logging method call.
The code is using incorrect logging method name. logging.INFO() is not a valid logging method.
Apply this diff to use the correct logging method:
- logging.INFO("🧪 Running in TEST MODE - calls will be simulated")
+ logging.info("🧪 Running in TEST MODE - calls will be simulated")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.INFO("🧪 Running in TEST MODE - calls will be simulated") | |
| logging.info("🧪 Running in TEST MODE - calls will be simulated") |
🤖 Prompt for AI Agents
In backend/src/services/campaign_service.py at line 17, the code calls
logging.INFO("🧪 Running in TEST MODE - calls will be simulated"), which is
invalid; replace this with a proper logging call such as logging.info("🧪
Running in TEST MODE - calls will be simulated") (or logging.log(logging.INFO,
"...") if you prefer the explicit level form) so the message is emitted
correctly.
| campaign = db.query(campaign_model.Campaign).filter(campaign_model.Campaign.id == campaign_id).first() | ||
| if not campaign: | ||
| print(f"Campaign {campaign_id} not found in _make_calls_sequentially") | ||
| logging.INFO(f"Campaign {campaign_id} not found in _make_calls_sequentially") |
There was a problem hiding this comment.
Fix logging method call.
Same issue with incorrect logging method name.
Apply this diff to use the correct logging method:
- logging.INFO(f"Campaign {campaign_id} not found in _make_calls_sequentially")
+ logging.info(f"Campaign {campaign_id} not found in _make_calls_sequentially")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.INFO(f"Campaign {campaign_id} not found in _make_calls_sequentially") | |
| @@ backend/src/services/campaign_service.py:51 | |
| - logging.INFO(f"Campaign {campaign_id} not found in _make_calls_sequentially") | |
| + logging.info(f"Campaign {campaign_id} not found in _make_calls_sequentially") |
🤖 Prompt for AI Agents
In backend/src/services/campaign_service.py around line 51, the code calls
logging.INFO(...) which is incorrect; replace the call with
logging.info(f"Campaign {campaign_id} not found in _make_calls_sequentially") so
the lowercase logging method is used (ensure the logging module is imported and
no other instances use the uppercase level attribute as a function).
| logging.INFO(f"Starting sequential calls to {len(campaign.contacts)} contacts for campaign {campaign_id}") | ||
| logging.INFO(f"Mode: {'TEST (simulated)' if self.test_mode else 'LIVE'}") |
There was a problem hiding this comment.
Fix logging method calls.
Same issue with incorrect logging method names.
Apply this diff to use the correct logging methods:
- logging.INFO(f"Starting sequential calls to {len(campaign.contacts)} contacts for campaign {campaign_id}")
- logging.INFO(f"Mode: {'TEST (simulated)' if self.test_mode else 'LIVE'}")
+ logging.info(f"Starting sequential calls to {len(campaign.contacts)} contacts for campaign {campaign_id}")
+ logging.info(f"Mode: {'TEST (simulated)' if self.test_mode else 'LIVE'}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.INFO(f"Starting sequential calls to {len(campaign.contacts)} contacts for campaign {campaign_id}") | |
| logging.INFO(f"Mode: {'TEST (simulated)' if self.test_mode else 'LIVE'}") | |
| logging.info(f"Starting sequential calls to {len(campaign.contacts)} contacts for campaign {campaign_id}") | |
| logging.info(f"Mode: {'TEST (simulated)' if self.test_mode else 'LIVE'}") |
🤖 Prompt for AI Agents
In backend/src/services/campaign_service.py around lines 54 to 55, the logging
calls use incorrect uppercase method names (logging.INFO) instead of the
function logging.info; change both calls to use logging.info(...) and keep the
same message strings so the messages are emitted at INFO level.
| segments, info = self.model.transcribe(audio_file_path, beam_size=5) | ||
|
|
||
| print(f"Detected language '{info.language}' with probability {info.language_probability}") | ||
| logging.INFO(f"Detected language '{info.language}' with probability {info.language_probability}") |
There was a problem hiding this comment.
Fix logging method call.
Same issue with incorrect logging method name.
Apply this diff to use the correct logging method:
- logging.INFO(f"Detected language '{info.language}' with probability {info.language_probability}")
+ logging.info(f"Detected language '{info.language}' with probability {info.language_probability}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.INFO(f"Detected language '{info.language}' with probability {info.language_probability}") | |
| logging.info(f"Detected language '{info.language}' with probability {info.language_probability}") |
🤖 Prompt for AI Agents
In backend/src/services/stt_service.py around line 35, the code calls
logging.INFO(...) which is incorrect — replace this with the proper logging
method call (e.g., logging.info(f"Detected language '{info.language}' with
probability {info.language_probability}") or, if using a module logger,
logger.info(...)) so the message is logged at INFO level.
|
|
||
| except Exception as e: | ||
| print(f"Error during transcription with Faster-Whisper: {e}") | ||
| logging.ERROR(f"Error during transcription with Faster-Whisper: {e}") |
There was a problem hiding this comment.
Fix logging method call.
Same issue with incorrect logging method name.
Apply this diff to use the correct logging method:
- logging.ERROR(f"Error during transcription with Faster-Whisper: {e}")
+ logging.error(f"Error during transcription with Faster-Whisper: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.ERROR(f"Error during transcription with Faster-Whisper: {e}") | |
| logging.error(f"Error during transcription with Faster-Whisper: {e}") |
🤖 Prompt for AI Agents
In backend/src/services/stt_service.py around line 42, the code calls
logging.ERROR(...) which is not a valid logging method; change it to use
logging.error(...) (lowercase) and include the exception details by passing
exc_info=True or otherwise logging the exception object so the error message
contains the actual exception information.
| # backend/src/services/telephony_service.py | ||
| from twilio.rest import Client | ||
| from ..core.config import settings | ||
| import logging |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix logging misuse and preserve original traceback when re-raising
- Replace
logging.INFO/ERROR(...)withlogger.info/error(...). - Prefer
raiseoverraise eto preserve the original traceback.
from ..core.config import settings
import logging
+logger = logging.getLogger(__name__)
@@
- logging.INFO("Initializing Twilio client...")
+ logger.info("Initializing Twilio client...")
@@
- logging.INFO("✅ Twilio client initialized successfully.")
+ logger.info("✅ Twilio client initialized successfully.")
@@
- logging.ERROR(f"❌ Failed to initialize Twilio client: {e}")
+ logger.error(f"❌ Failed to initialize Twilio client: {e}", exc_info=True)
@@
- logging.INFO(f"Attempting to call {to_number} from {settings.TWILIO_PHONE_NUMBER}...")
+ logger.info(f"Attempting to call {to_number} from {settings.TWILIO_PHONE_NUMBER}...")
@@
- logging.INFO(f"Successfully initiated call with SID: {call.sid}")
+ logger.info(f"Successfully initiated call with SID: {call.sid}")
@@
- logging.ERROR(f"❌ Twilio call creation failed: {e}")
+ logger.error(f"❌ Twilio call creation failed: {e}", exc_info=True)
- raise e
+ raiseAlso applies to: 25-25, 27-27, 29-29, 46-46, 53-53, 56-56
🤖 Prompt for AI Agents
In backend/src/services/telephony_service.py around lines 4, 25, 27, 29, 46, 53,
and 56, replace direct calls to logging.INFO(...) or logging.ERROR(...) with a
module logger instance (e.g. logger = logging.getLogger(__name__)) and call
logger.info(...)/logger.error(...) instead; also when re-raising caught
exceptions change any `raise e` to a bare `raise` so the original traceback is
preserved. Ensure you add or reuse the logger variable at top of the file and
update each affected line to use logger.<method>(...) and convert `raise e` to
`raise`.
| # backend/src/services/tts_service.py | ||
| from TTS.api import TTS | ||
| from ..core.config import settings | ||
| import logging |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix logging misuse: use logger.info/error, not logging.INFO/ERROR
As with other modules, logging.INFO(...) and logging.ERROR(...) are invalid calls. Use a module logger and proper methods.
from ..core.config import settings
import logging
+
+logger = logging.getLogger(__name__)
@@
- logging.INFO(f"Initializing Coqui TTS engine with model: {model_name}")
+ logger.info(f"Initializing Coqui TTS engine with model: {model_name}")
@@
- logging.INFO("TTS service initialized successfully.")
+ logger.info("TTS service initialized successfully.")
@@
- logging.ERROR(f"Error initializing Coqui TTS: {e}")
- logging.ERROR("This might be the first run, and the model needs to be downloaded.")
+ logger.error(f"Error initializing Coqui TTS: {e}", exc_info=True)
+ logger.error("This might be the first run, and the model needs to be downloaded.")
@@
- logging.INFO(f"Synthesized audio with Coqui TTS and saved to {output_path}")
+ logger.info(f"Synthesized audio with Coqui TTS and saved to {output_path}")
@@
- logging.ERROR(f"Error during Coqui TTS synthesis: {e}")
+ logger.error(f"Error during Coqui TTS synthesis: {e}", exc_info=True)Also applies to: 15-15, 20-20, 22-24, 31-33
🤖 Prompt for AI Agents
In backend/src/services/tts_service.py around lines 4 and also affecting lines
15, 20, 22-24, and 31-33, the code incorrectly calls logging.INFO(...) /
logging.ERROR(...) as functions; create a module logger (logger =
logging.getLogger(__name__)) and replace those invalid calls with the correct
methods (logger.info(...), logger.error(...), or logger.warning(...)) passing
the original messages and any exception info or variables as before; ensure you
remove any misuse of logging.<LEVEL>(...) and use the logger instance
consistently throughout the file.
| logging.INFO(f"Initializing Coqui TTS engine with model: {model_name}") | ||
|
|
||
| # Initialize TTS with the specified model, forcing it to run on CPU | ||
| self.tts = TTS(model_name, gpu=False) | ||
|
|
||
| print("TTS service initialized successfully.") | ||
| logging.INFO("TTS service initialized successfully.") |
There was a problem hiding this comment.
Service still uses Coqui TTS but requirements install gTTS — reconcile or migrate
This module imports and uses Coqui TTS, while requirements add gTTS. If TTS is not installed, this module will fail to import. Decide on one path:
-
Short-term: keep Coqui by restoring
TTSin requirements (see my comment in requirements.txt). -
Preferred: migrate this service to gTTS. Example implementation:
# backend/src/services/tts_service.py (gTTS-based)
from gtts import gTTS
from ..core.config import settings
import logging
logger = logging.getLogger(__name__)
class TTSService:
"""
A service for Text-to-Speech using Google gTTS.
"""
def __init__(self):
try:
self.lang = getattr(settings, "TTS_LANGUAGE", "en")
logger.info(f"Initializing gTTS with language: {self.lang}")
logger.info("TTS service initialized successfully.")
except Exception as e:
logger.error(f"Error initializing gTTS: {e}", exc_info=True)
raise
def synthesize(self, text: str, output_path: str):
"""Synthesizes text and saves it to an audio file."""
try:
gTTS(text=text, lang=self.lang).save(output_path)
logger.info(f"Synthesized audio with gTTS and saved to {output_path}")
except Exception as e:
logger.error(f"Error during gTTS synthesis: {e}", exc_info=True)
raiseIf you want, I can open a follow-up PR to complete this migration and configuration (e.g., add TTS_LANGUAGE to settings).
🤖 Prompt for AI Agents
In backend/src/services/tts_service.py around lines 15-20, the module imports
and uses Coqui TTS but requirements include gTTS; choose one path and make the
code match: either restore the Coqui/TTS dependency in requirements (short-term)
or migrate this service to gTTS (preferred). To migrate, replace the Coqui
import with from gtts import gTTS, use the project logger, read a TTS_LANGUAGE
from settings with a sensible default, log initialization and errors with
exc_info=True, implement a synthesize(text, output_path) method that calls
gTTS(...).save(output_path) and logs success/failure, and remove any
Coqui-specific options (gpu flag etc.). If you keep Coqui instead, revert
requirements change and ensure imports and error logging remain consistent.
|
@Vinayak-2003 please resolve all the conflicts raised by coderabbit |
Hiteshydv001
added
GSSoc'25
Level 2
|
@Vinayak-2003 How much more time is required?? |
|
@Hiteshydv001 actually I am out of city without my device, will try my best to complete this ASAP. |
|
Okay 👍 |
2 participants
Hi @Hiteshydv001, as per the need I have made the changes for Feature: #15
Summary of Changes
log_call_eventfunction with an extra argument of specifying log level.Summary by CodeRabbit
Bug Fixes
Chores