Skip to content

Commit 150890a

Browse files
authored
fix: Clean up logging and metrics handling across the application
fix: Clean up logging and metrics handling across the application
2 parents 4d98cac + 37b12a0 commit 150890a

8 files changed

Lines changed: 227 additions & 189 deletions

File tree

src/sample_python_app/app/lifecycle.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
def start_metrics_server(port: int) -> None:
1616
"""Start the Prometheus metrics server on the specified port."""
1717
if _port_in_use(port):
18-
logger.error("Port %s already in use; metrics disabled", port)
18+
logger.error(f"Port {port} already in use; metrics disabled")
1919
return
2020

2121
logger.info(f"Starting Prometheus metrics on 0.0.0.0:{port}")

src/sample_python_app/app/runner.py

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,12 @@
33
Handles fetching, validation, and display of astronomical data.
44
"""
55

6-
import json
76
import time
87
from datetime import date
98

10-
import httpx
11-
from pydantic import ValidationError
12-
139
from sample_python_app.core import (
1410
FETCH_COUNTER,
1511
FETCH_DURATION,
16-
FETCH_ERRORS,
1712
setup_logger,
1813
weather_settings,
1914
)
@@ -41,30 +36,31 @@ def __init__(self, client: CustomHTTPClient) -> None:
4136
self.client = client
4237

4338
def fetch(self, *, exit_on_error: bool = True) -> None:
44-
"""Fetch astronomical data and display if not already displayed today."""
39+
"""Fetch astronomical data and display if not already shown today."""
4540
lat = weather_settings.LOCATION.latitude
4641
lon = weather_settings.LOCATION.longitude
47-
logger.info(f"Using latitude={lat} longitude={lon}")
42+
43+
logger.info("Using latitude=%s longitude=%s", lat, lon)
44+
4845
start = time.time()
46+
4947
try:
5048
astro = fetch_astronomical_data_from_api(lat, lon, client=self.client)
5149
forecast = fetch_hourly_forecast_from_api(lat, lon, client=self.client)
5250
FETCH_COUNTER.inc()
53-
except (
54-
httpx.HTTPStatusError,
55-
httpx.RequestError,
56-
ValidationError,
57-
json.JSONDecodeError,
58-
AppError,
59-
) as exc:
60-
self._handle_fetch_error(exc, exit_on_error)
51+
except AppError as exc:
52+
logger.error("Weather fetch failed: %s", exc)
53+
if exit_on_error:
54+
raise SystemExit(1) from None
6155
return
6256
finally:
6357
FETCH_DURATION.observe(time.time() - start)
64-
today_str = date.today().isoformat()
65-
if self._last_displayed_day != today_str:
58+
59+
today = date.today().isoformat()
60+
61+
if self._last_displayed_day != today:
6662
display_astronomical_data(astro, forecast)
67-
self._last_displayed_day = today_str
63+
self._last_displayed_day = today
6864

6965
def reset_display(self):
7066
"""Reset the last displayed day so display will occur again."""
@@ -82,22 +78,6 @@ def close(self) -> None:
8278
except Exception:
8379
logger.exception("Error closing HTTP client in AstroFetcher")
8480

85-
def _handle_fetch_error(self, exc: Exception, exit_on_error: bool) -> None:
86-
FETCH_ERRORS.inc()
87-
if isinstance(exc, httpx.HTTPStatusError):
88-
logger.error("HTTP status error: %s", exc)
89-
elif isinstance(exc, httpx.RequestError):
90-
logger.error("Network error: %s", exc)
91-
elif isinstance(exc, ValidationError):
92-
logger.error("Validation error: %s", exc)
93-
elif isinstance(exc, json.JSONDecodeError):
94-
logger.error("JSON decode error: %s", exc)
95-
else:
96-
logger.exception("Unexpected error")
97-
if exit_on_error:
98-
raise SystemExit(1) from exc
99-
raise AppError(str(exc)) from exc
100-
10181

10282
runner_client = CustomHTTPClient(
10383
headers=weather_settings.WEATHER_HEADERS,

src/sample_python_app/core/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
FETCH_COUNTER,
77
FETCH_DURATION,
88
FETCH_ERRORS,
9+
HTTP_REQUEST_DURATION,
10+
HTTP_REQUEST_EXCEPTIONS,
11+
HTTP_REQUESTS,
912
)
1013

1114
__all__ = [
@@ -16,4 +19,7 @@
1619
"FETCH_COUNTER",
1720
"FETCH_DURATION",
1821
"FETCH_ERRORS",
22+
"HTTP_REQUESTS",
23+
"HTTP_REQUEST_EXCEPTIONS",
24+
"HTTP_REQUEST_DURATION",
1925
]

src/sample_python_app/core/metrics.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,27 @@
1414
"fetch_duration_seconds",
1515
"Duration of astronomical data fetches in seconds",
1616
)
17-
17+
HTTP_REQUESTS = Counter(
18+
"http_requests_total",
19+
"Total HTTP requests made",
20+
["method", "host", "path", "status_code"],
21+
)
22+
HTTP_REQUEST_EXCEPTIONS = Counter(
23+
"http_request_exceptions_total",
24+
"Total HTTP request exceptions",
25+
["method", "host", "path", "exception_type"],
26+
)
27+
HTTP_REQUEST_DURATION = Histogram(
28+
"http_request_duration_seconds",
29+
"HTTP request latency in seconds",
30+
["method", "host", "path"],
31+
)
1832
__all__ = [
1933
"FETCH_COUNTER",
2034
"FETCH_ERRORS",
2135
"FETCH_DURATION",
36+
"HTTP_REQUESTS",
37+
"HTTP_REQUEST_EXCEPTIONS",
38+
"HTTP_REQUEST_DURATION",
2239
"start_http_server",
2340
]

src/sample_python_app/exceptions/__init__.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
from sample_python_app.exceptions.custom import (
44
AppError,
5+
HTTPTimeoutError,
6+
NetworkError,
7+
ServiceError,
58
)
69

7-
__all__ = ["AppError"]
10+
__all__ = ["AppError", "HTTPTimeoutError", "NetworkError", "ServiceError"]

src/sample_python_app/exceptions/custom.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,38 @@
11
"""Custom exceptions for the sample_python_app project."""
22

33

4+
class HTTPTimeoutError(Exception):
5+
"""Raised when an HTTP request times out."""
6+
7+
def __init__(self, message: str) -> None:
8+
"""Initialize the HTTPTimeoutError with a message."""
9+
super().__init__(message)
10+
11+
12+
class NetworkError(Exception):
13+
"""Raised when a network-level error occurs.
14+
15+
(connection refused, DNS failure, etc.).
16+
"""
17+
18+
def __init__(self, message: str) -> None:
19+
"""Initialize the NetworkError with a message."""
20+
super().__init__(message)
21+
22+
23+
class ServiceError(Exception):
24+
"""Raised when the HTTP response returns an error status (4xx or 5xx)."""
25+
26+
def __init__(self, status_code: int, body: str | None = None) -> None:
27+
"""Initialize the ServiceError with status code and optional response body."""
28+
self.status_code = status_code
29+
self.body = body
30+
msg = f"Service returned status {status_code}"
31+
if body:
32+
msg += f": {body}"
33+
super().__init__(msg)
34+
35+
436
class AppError(Exception):
537
"""Base exception for the application."""
638

0 commit comments

Comments
 (0)