fix: make Prometheus alert rules runtime-agnostic and add comprehensive production alerts - #82
Conversation
…ve production alerts - Replace Go-only http_requests_total metric with atlas_http_requests_total (the actual metric exposed by Python services via atlas_observability) - Replace Go-only process_resident_memory_bytes / process_virtual_memory_bytes (not available in Node.js, Python, or Java runtimes) - Add HighLatencyP99 and HighLatencyP99Critical using atlas_http_request_duration_seconds histogram for p99 latency monitoring - Add HighInProgressRequests using atlas_http_requests_in_progress gauge - Add DiskSpaceWarning / DiskSpaceCritical using node_filesystem_* metrics (requires node_exporter to be added to monitoring stack) - Add NodeMemoryUsageWarning / NodeMemoryUsageCritical using node_memory_* (requires node_exporter) - Add NodeCPUUsageHigh using node_cpu_seconds_total (requires node_exporter) - Add TLSCertExpiryWarning / TLSCertExpiryCritical using probe_ssl_earliest_cert_expiry (requires blackbox_exporter to be added to monitoring stack) Fixes issue Senthil455#60
There was a problem hiding this comment.
This is a substantial PR adding Prometheus metrics instrumentation across the stack. A few observations:
-
Cross-language consistency looks good — Node.js (
prom-client), Python (AtlasMetricsMiddleware), and Go/Fiber (prometheus/client_golang) all appear to implement the same core metrics:atlas_http_requests_totalatlas_http_request_duration_secondsatlas_http_requests_in_progress
-
auth-service and api-gateway
- Excluding
/metricsand/healthfrom instrumentation is the correct approach. - Path normalization for parameterized routes appears reasonable and should help control label cardinality.
- Excluding
-
Python services
-
AtlasMetricsMiddlewareis referenced throughout the diff, but I don't see its implementation or import source included here. -
Please verify that:
- The middleware exists in the shared observability package.
- The import path resolves correctly in all services.
- The middleware is actually registered in the request pipeline for each service.
-
This is the main area I'd like to double-check before approval.
-
-
Go services
- The Fiber middleware implementation looks correct.
- Using
adaptor/v2for/metricsand collecting request metrics in middleware follows established patterns.
-
Alerting
- Alert rules have been updated consistently to use the new
atlas_metric namespace.
- Alert rules have been updated consistently to use the new
-
Prometheus configuration
- The additional scrape targets (
live-serviceandworkforce-planning-service) look appropriate.
- The additional scrape targets (
Overall, the instrumentation approach is well-structured and consistent across the stack. My only significant concern is ensuring the Python AtlasMetricsMiddleware dependency and registration path are present and validated in all affected services.
|
Thanks for the review. The three
These are implemented in from atlas_observability import AtlasMetricsMiddlewareAs a result, no code changes are required in this PR. This PR only updates |
Summary
Fixes #60 by replacing Go-only Prometheus metrics (
http_requests_total,process_resident_memory_bytes) with runtime-agnostic alternatives and expanding from 3 to 12 production-grade alert rules.Changes
Bugs Fixed
http_requests_total{status=~"5.."}(Go-only, doesn't exist in Node.js/Python) withatlas_http_requests_total{status_code=~"5[0-9][0-9]"}— the actual metric name exposed by Python services via theatlas_observabilitylibrary. Also changed from raw error rate to error percentage (errors/total requests).process_resident_memory_bytes / process_virtual_memory_bytes(Go runtime metrics, not available in Node.js, Python, or Java). Replaced with node-level memory alerts usingnode_memory_*metrics.New Alerts Added
Runtime Compatibility
atlas_*metrics are exposed by 11 Python services via theatlas_observabilitylibrarynode_*metrics requirenode_exporterto be added to the monitoring stackprobe_ssl_*metrics requireblackbox_exporterto be added to the monitoring stackPrerequisites
node_exportertodocker-compose.monitoring.ymlblackbox_exportertodocker-compose.monitoring.yml