feat(logs): add comprehensive PostgreSQL logging configuration#1194
feat(logs): add comprehensive PostgreSQL logging configuration#1194
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded multi-line Changes
Sequence Diagram(s)sequenceDiagram
participant Compose as Docker Compose
participant Cont as xyne-db container
participant PG as postgres process
Compose->>Cont: up service (xyne-db) with `command: >`
Note right of Cont `#E8F8FF`: command includes multiple `-c` flags\n(log_*, log_rotation, timezone, prefix)
Cont->>PG: exec `postgres` with provided `-c` options
PG-->>Cont: start-up with overridden runtime GUCs
Cont-->>Compose: healthcheck + logs (now structured/rotated)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
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. Comment |
Summary of ChangesHello @naSim087, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the PostgreSQL logging capabilities across all Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive PostgreSQL logging configuration across all deployment environments. The changes are consistent across all docker-compose files. While the added logging improves observability, there are a few areas for improvement. My review includes suggestions to optimize logging for production environments by adjusting the verbosity, correcting the log filename pattern for better rotation, removing a deprecated parameter, and using UTC for the log timezone for better consistency. These changes are duplicated across multiple files, and my comments reflect that.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
deployment/docker-compose.dev.yml (1)
97-114: Same critical and major issues present as in docker-compose.prod.yml.This file shares the identical logging configuration as the production variant, including:
- Ephemeral logs not persisted (log_directory=log without volume mount)
- Conflicting log_destination=stderr + logging_collector=on
- Hardcoded Asia/Kolkata timezone
Refer to the detailed comments on docker-compose.prod.yml (lines 117-134) for the full analysis and recommended fixes. Apply the same corrections to this file.
deployment/docker-compose.yml (1)
119-136: Same critical and major issues present as in docker-compose.prod.yml.Identical logging configuration across all files results in the same problems: ephemeral logs, conflicting destinations, hardcoded timezone. Refer to the detailed analysis on docker-compose.prod.yml for fixes.
deployment/docker-compose.selfhost.yml (1)
115-132: Same critical and major issues present as in docker-compose.prod.yml.Refer to docker-compose.prod.yml for complete analysis and fixes.
deployment/docker-compose.metrics.yml (1)
87-104: Same critical and major issues present as in docker-compose.prod.yml.Refer to docker-compose.prod.yml for complete analysis and fixes.
deployment/portable/docker-compose.infrastructure.yml (1)
132-149: Same critical and major issues present as in docker-compose.prod.yml.Refer to docker-compose.prod.yml for complete analysis and fixes.
deployment/portable/docker-compose.infrastructure-cpu.yml (1)
128-145: Same critical and major issues present as in docker-compose.prod.yml.Refer to docker-compose.prod.yml for complete analysis and fixes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.metrics.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/docker-compose.selfhost.yml(1 hunks)deployment/docker-compose.yml(1 hunks)deployment/portable/docker-compose.infrastructure-cpu.yml(1 hunks)deployment/portable/docker-compose.infrastructure.yml(1 hunks)
🔇 Additional comments (1)
deployment/docker-compose.prod.yml (1)
121-122: The review comment is incorrect.When
log_destinationincludes stderr andlogging_collector=on, PostgreSQL's collector captures those stderr messages and writes log files. This is not a conflict—it's the intended behavior. This configuration ensures logs are sent to stderr (which Docker can capture via its logging driver for container logs) while simultaneously being persisted to files via the logging collector. This is a standard and valid setup for containerized PostgreSQL.Likely an incorrect or invalid review comment.
| command: > | ||
| postgres | ||
| -c log_statement=mod | ||
| -c log_min_duration_statement=0 | ||
| -c logging_collector=on | ||
| -c log_destination=stderr | ||
| -c log_directory=log | ||
| -c log_filename=postgresql-%Y-%m-%d_%H%M%S.log | ||
| -c log_file_mode=0640 | ||
| -c log_rotation_age=3d | ||
| -c log_rotation_size=100MB | ||
| -c log_truncate_on_rotation=off | ||
| -c log_connections=on | ||
| -c log_disconnections=on | ||
| -c log_duration=on | ||
| -c log_hostname=on | ||
| -c log_line_prefix='%m [%p] %u@%d:%r ' | ||
| -c log_timezone=Asia/Kolkata |
There was a problem hiding this comment.
Critical: PostgreSQL logs will not persist across container restarts.
Line 123 sets log_directory=log as a relative path. Without a volume mount for the log directory, all PostgreSQL logs will be lost when the container restarts, defeating the purpose of comprehensive logging for observability and debugging.
Add a volume mount for PostgreSQL logs:
volumes:
- ../server/xyne-data:/var/lib/postgresql/data
+ - ../server/xyne-logs:/var/log/postgresqlThen update the log configuration to use the mounted path:
- -c log_directory=log
+ -c log_directory=/var/log/postgresqlCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In deployment/docker-compose.prod.yml around lines 117 to 134, the Postgres
config sets log_directory=log (a relative path) so logs won’t persist; add a
volume mount for the container log directory (e.g. map a named volume or host
path to an absolute container path like /var/log/postgresql) under the postgres
service volumes, change the log_directory value to that absolute mounted path
(for example /var/log/postgresql) in the command flags, and ensure the mounted
path has correct ownership/permissions for the Postgres process so logs are
written and persist across restarts.
Description
add comprehensive logging configuration to PostgreSQL across all deployment environments to enhance observability, debugging, and monitoring capabilities.
Automatic log rotation (3 days or 100MB)
Testing
Additional Notes
Summary by CodeRabbit