Skip to content

Commit 8706709

Browse files
authored
Merge pull request #125 from cyiallou/task/add-logger
Add consistent logger setup across all modules
2 parents e0a96ea + c98b0b8 commit 8706709

19 files changed

+215
-265
lines changed

RELEASE_NOTES.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
## New Features
1212

13-
<!-- Here goes the main new features and examples or instructions on how to use them -->
13+
- Added consistent logger setup across all modules for structured logging and improved observability. Example notebooks updated to demonstrate logger usage.
1414

1515
## Bug Fixes
1616

examples/Alerts Notebook.ipynb

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

examples/Solar Maintenance.ipynb

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

examples/solar_maintenance_example.ipynb

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,12 @@
8383
},
8484
{
8585
"cell_type": "code",
86-
"execution_count": 1,
86+
"execution_count": null,
8787
"metadata": {},
8888
"outputs": [],
8989
"source": [
9090
"import datetime\n",
91+
"import logging\n",
9192
"import os\n",
9293
"from dataclasses import asdict, dataclass, field\n",
9394
"\n",
@@ -98,28 +99,32 @@
9899
},
99100
{
100101
"cell_type": "code",
101-
"execution_count": 2,
102+
"execution_count": null,
102103
"metadata": {},
103-
"outputs": [
104-
{
105-
"data": {
106-
"text/plain": [
107-
"True"
108-
]
109-
},
110-
"execution_count": 2,
111-
"metadata": {},
112-
"output_type": "execute_result"
113-
}
114-
],
104+
"outputs": [],
105+
"source": [
106+
"logging.basicConfig(\n",
107+
" level=logging.WARNING,\n",
108+
" format=\"%(asctime)s %(levelname)s %(name)s: %(message)s\",\n",
109+
" datefmt=\"%H:%M:%S\",\n",
110+
")\n",
111+
"logging.getLogger(\"matplotlib.font_manager\").setLevel(logging.ERROR)\n",
112+
"logging.getLogger(\"frequenz.lib.notebooks\").setLevel(logging.WARNING)"
113+
]
114+
},
115+
{
116+
"cell_type": "code",
117+
"execution_count": null,
118+
"metadata": {},
119+
"outputs": [],
115120
"source": [
116121
"# Load environment variables from the .env file\n",
117122
"load_dotenv()"
118123
]
119124
},
120125
{
121126
"cell_type": "code",
122-
"execution_count": 3,
127+
"execution_count": null,
123128
"metadata": {},
124129
"outputs": [],
125130
"source": [
@@ -222,13 +227,13 @@
222227
},
223228
{
224229
"cell_type": "code",
225-
"execution_count": 4,
230+
"execution_count": null,
226231
"metadata": {},
227232
"outputs": [],
228233
"source": [
229234
"# example user request (1 year of data)\n",
230235
"end_time = datetime.datetime.now().astimezone(datetime.timezone.utc)\n",
231-
"start_time = end_time - datetime.timedelta(days=364)\n",
236+
"start_time = end_time - datetime.timedelta(days=14)\n",
232237
"\n",
233238
"USER_REQUEST = {\n",
234239
" \"weather_service_address\": os.environ.get(\"WEATHER_SERVICE_ADDRESS\"),\n",
@@ -253,7 +258,7 @@
253258
},
254259
{
255260
"cell_type": "code",
256-
"execution_count": 5,
261+
"execution_count": null,
257262
"metadata": {},
258263
"outputs": [],
259264
"source": [

src/frequenz/lib/notebooks/notification_service.py

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
from types import UnionType
7676
from typing import Any, Callable, TypeVar, Union, get_args, get_origin
7777

78-
_log = logging.getLogger(__name__)
78+
_logger = logging.getLogger(__name__)
7979

8080

8181
DataclassT = TypeVar("DataclassT", bound="FromDictMixin")
@@ -346,7 +346,7 @@ def start(self, task: Callable[..., None], **kwargs: Any) -> None:
346346
"""
347347
self.task = task
348348
self._task_name = task.__name__
349-
_log.info(
349+
_logger.info(
350350
"Starting scheduler for task '%s' to execute every %d seconds and %s",
351351
self._task_name,
352352
self._config.interval,
@@ -365,18 +365,18 @@ def stop(self) -> None:
365365
"""Stop the scheduler."""
366366
if self._thread is not None:
367367
if self._thread.is_alive():
368-
_log.info("Stopping scheduler for task '%s'", self._task_name)
368+
_logger.info("Stopping scheduler for task '%s'", self._task_name)
369369
self._stop_event.set()
370370
if not self._stop_event.is_set():
371-
_log.error(
371+
_logger.error(
372372
"Failed to stop scheduler for task '%s'", self._task_name
373373
)
374374
else:
375-
_log.warning(
375+
_logger.warning(
376376
"Attempted to stop scheduler for task '%s', but no active thread was found.",
377377
self._task_name,
378378
)
379-
_log.info("Scheduler successfully stopped")
379+
_logger.info("Scheduler successfully stopped")
380380

381381
def _run_task(self, kwargs: dict[str, Any]) -> None:
382382
"""Run the scheduled task.
@@ -389,19 +389,21 @@ def _run_task(self, kwargs: dict[str, Any]) -> None:
389389
elapsed = self._execute_task(kwargs)
390390
self._pace(elapsed)
391391
else:
392-
_log.info(
392+
_logger.info(
393393
"Waiting for first interval before sending the first notification."
394394
)
395395
self._pace(0)
396396
while not self._should_stop():
397397
elapsed = self._execute_task(kwargs)
398398
self._pace(elapsed)
399-
_log.info("Scheduler stopping: stop condition met.")
399+
_logger.info("Scheduler stopping: stop condition met.")
400400
self.stop()
401401

402402
def _should_stop(self) -> bool:
403403
"""Return True if the scheduler should stop."""
404-
_log.debug("Checking if scheduler for task '%s' should stop.", self._task_name)
404+
_logger.debug(
405+
"Checking if scheduler for task '%s' should stop.", self._task_name
406+
)
405407
return self._stop_event.is_set() or (
406408
self._config.duration is not None
407409
and self._start_time is not None
@@ -422,14 +424,14 @@ def _execute_task(self, kwargs: dict[str, Any]) -> float:
422424
if self.task:
423425
self.task(**kwargs)
424426
except Exception as e: # pylint: disable=broad-except
425-
_log.error(
427+
_logger.error(
426428
"Error occurred during scheduled execution of %s: %s",
427429
self._task_name,
428430
e,
429431
)
430432
finally:
431433
task_elapsed = time.time() - task_start_time
432-
_log.debug(
434+
_logger.debug(
433435
"Execution of task '%s' completed in %.2f seconds.",
434436
self._task_name,
435437
task_elapsed,
@@ -465,7 +467,9 @@ def _pace(self, elapsed_task_time: float) -> None:
465467
actual_sleep = max(0, sleep_duration - elapsed_task_time)
466468
if self._stop_event.is_set():
467469
return
468-
_log.info("Sleeping for %.2f seconds before next task execution.", actual_sleep)
470+
_logger.info(
471+
"Sleeping for %.2f seconds before next task execution.", actual_sleep
472+
)
469473
self._stop_event.wait(actual_sleep)
470474

471475

@@ -503,15 +507,17 @@ def send_with_retry(
503507
for attempt in range(retries + 1):
504508
try:
505509
send_func(**kwargs)
506-
_log.info("Successfully sent notification on attempt %d", attempt + 1)
510+
_logger.info(
511+
"Successfully sent notification on attempt %d", attempt + 1
512+
)
507513
return
508514
except Exception as e: # pylint: disable=broad-except
509515
last_exception = e
510-
_log.error("Attempt %d failed: %s", attempt + 1, e)
516+
_logger.error("Attempt %d failed: %s", attempt + 1, e)
511517
if attempt < retries - 1:
512518
linear_backoff = backoff_factor * (attempt + 1)
513519
time.sleep(min(max_sleep, linear_backoff))
514-
_log.error("Failed to send notification after %d retries", retries)
520+
_logger.error("Failed to send notification after %d retries", retries)
515521
raise NotificationSendError(
516522
"Notification failed after all retry attempts.",
517523
last_exception=last_exception,
@@ -520,17 +526,17 @@ def send_with_retry(
520526
def start_scheduler(self) -> None:
521527
"""Start the scheduler if configured."""
522528
if self._scheduler:
523-
_log.info("Starting scheduler for %s", self.__class__.__name__)
529+
_logger.info("Starting scheduler for %s", self.__class__.__name__)
524530
self._scheduler.start(self.send)
525531
else:
526-
_log.warning("No scheduler config provided. Cannot start scheduler.")
532+
_logger.warning("No scheduler config provided. Cannot start scheduler.")
527533

528534
def stop_scheduler(self) -> None:
529535
"""Stop the running scheduler."""
530536
if not self._scheduler:
531-
_log.warning("No active scheduler to stop.")
537+
_logger.warning("No active scheduler to stop.")
532538
return
533-
_log.info("Stopping scheduler for notification: %s", self.__class__.__name__)
539+
_logger.info("Stopping scheduler for notification: %s", self.__class__.__name__)
534540
self._scheduler.stop()
535541

536542
@abstractmethod
@@ -559,7 +565,7 @@ def __init__(self, config: EmailConfig) -> None:
559565
super().__init__()
560566
self._config: EmailConfig = config
561567
if self._config.scheduler:
562-
_log.debug(
568+
_logger.debug(
563569
"EmailNotification configured with scheduler: %s",
564570
self._config.scheduler,
565571
)
@@ -625,9 +631,9 @@ def _attach_files(self, msg: EmailMessage, attachments: list[str]) -> None:
625631
)
626632
except OSError as e:
627633
failed_attachments.append(file)
628-
_log.error("Failed to attach file %s: %s", file, e)
634+
_logger.error("Failed to attach file %s: %s", file, e)
629635
if failed_attachments:
630-
_log.warning(
636+
_logger.warning(
631637
"The following attachments could not be added: %s", failed_attachments
632638
)
633639

@@ -676,9 +682,9 @@ def _connect_and_send(
676682
server.starttls()
677683
server.login(str(smtp_settings["user"]), str(smtp_settings["password"]))
678684
server.send_message(msg)
679-
_log.info("Email sent successfully to %s", to_emails)
685+
_logger.info("Email sent successfully to %s", to_emails)
680686
except SMTPException as e:
681-
_log.error("Failed to send email: %s", e)
687+
_logger.error("Failed to send email: %s", e)
682688
raise
683689

684690

src/frequenz/lib/notebooks/notification_utils.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,19 @@
2222
NotificationSendError,
2323
)
2424

25-
_log = logging.getLogger(__name__)
25+
_logger = logging.getLogger(__name__)
2626

2727
EMAIL_REGEX = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$")
2828

2929

30-
def send_test_email(config: EmailConfig, verbose: bool = True) -> bool:
30+
def send_test_email(config: EmailConfig) -> bool:
3131
"""Send a test email using the given EmailConfig.
3232
3333
The email message is generated automatically based on the provided
3434
configuration and replaces the provided one.
3535
3636
Args:
3737
config: An EmailConfig instance.
38-
verbose: If True, prints the result to stdout.
3938
4039
Returns:
4140
True if the email was sent successfully, False otherwise.
@@ -75,19 +74,15 @@ def send_test_email(config: EmailConfig, verbose: bool = True) -> bool:
7574
email_notification = EmailNotification(config=config)
7675
email_notification.send()
7776
msg = "✅ Test email sent successfully!"
78-
_log.info(msg)
79-
if verbose:
80-
print(msg)
77+
_logger.info(msg)
8178
return True
8279
except NotificationSendError as e:
8380
msg = f"❌ Error sending test email: {e}"
84-
_log.error(msg)
81+
_logger.error(msg)
8582
if e.last_exception:
86-
_log.debug(
83+
_logger.debug(
8784
"Traceback:\n%s", "".join(traceback.format_exception(e.last_exception))
8885
)
89-
if verbose:
90-
print(msg)
9186
return False
9287

9388

0 commit comments

Comments
 (0)