feat: improve startup logging #564
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Need to be careful not to leak env secret values in the log! |
|
Yeah, that's why I'm masking URLs (in case they contain secrets). Also, I'm logging things like |
alexluong
left a comment
alexluong
left a comment
There was a problem hiding this comment.
Congrats on the 1st PR!
From the implementation side, everything looks good and we can certainly proceed as-is.
My biggest concern about this issue from the beginning was how to prevent drift as we continue building Outpost. Or we effectively have to add to our mental model (and probably should extend the contributing guide related to config?) that we need to account for this log whenever we add new configuration. Not a dealbreaker but something to consider.
My other idea was documented in #480 where we simply log the full config struct as-is. We should investigate the "sensitive string" struct of some sort and see if there's something we can do with that.
One other idea that's a bit simpler is for each "section" of the config to have its own logging logic. For example, c.MQsConfig.ToLogFields(). Not 100% sure if that's better, just an idea.
Overall, happy to proceed as is but do let me know what you think about this concern.
|
We sanitize the API logging based on config that tells us the values that are sensitive. https://github.com/hookdeck/outpost/blob/main/internal/services/api/sanitizer.go |
Fair point. I'm going to address it with docs for now.
Yeah, I feel you. I kinda did something similar by moving all these logs to
Thanks, @leggetter. Having a look to see if I can reuse some stuff. Although, these parts of the code are in two very different domains, if I'm understanding the codebase correctly. So I would hesitate from reusing directly. I could consider using a shared functionality in both. Perhaps a function inside "util". |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive startup configuration logging to improve troubleshooting and operational visibility. When Outpost starts, it now logs a detailed configuration summary with all relevant settings, properly masking sensitive data like passwords, secrets, and API keys. The implementation includes MQ-specific configuration fields that vary based on the message queue type being used.
Key changes:
- New
LogConfigurationSummary()method that returns structured zap fields for all configuration values - URL masking utilities to hide credentials while preserving host information
- Documentation explaining configuration logging guidelines for contributors
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/config/logging.go | New file implementing configuration logging with sensitive data masking and MQ-specific field handlers |
| internal/app/app.go | Updated startup logging to use the new comprehensive configuration summary |
| contributing/config.md | Expanded documentation with detailed guidelines for adding configuration fields and maintaining logging |
| CONTRIBUTING.md | Added reference to the new configuration documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@alexluong Added docs to the Contribution Guide. I like your solution at #480 much better but I think it can be done in later on if it becomes a pain to maintain. @leggetter I considered reusing some code but even though they do similar stuff, it feels like completely unrelated domains. I think creating an abstraction layer on top of both could actually be harder to maintain so I'm going to leave it as is. |
Fixes #485
Logs a configuration summary at startup with the following info:
{ "msg": "starting outpost", "service": "api", "config_file_path": ".env", "log_level": "info", "audit_log": true, "deployment_id": "", "topics": [ "user.created", "user.updated", "user.deleted" ], "organization_name": "", "http_user_agent": "", "api_port": 3333, "api_key_configured": true, "api_jwt_secret_configured": true, "gin_mode": "release", "aes_encryption_secret_configured": true, "redis_host": "redis", "redis_port": 6379, "redis_password_configured": true, "redis_database": 0, "redis_tls_enabled": false, "redis_cluster_enabled": false, "postgres_configured": true, "postgres_host": "postgres:5432", "mq_type": "rabbitmq", "publish_max_concurrency": 1, "delivery_max_concurrency": 1, "log_max_concurrency": 1, "retry_schedule": [], "retry_interval_seconds": 30, "retry_max_limit": 10, "max_destinations_per_tenant": 20, "delivery_timeout_seconds": 5, "publish_idempotency_key_ttl": 3600, "delivery_idempotency_key_ttl": 3600, "log_batch_threshold_seconds": 10, "log_batch_size": 1000, "telemetry_disabled": false, "alert_callback_url": "", "alert_consecutive_failure_count": 20, "alert_auto_disable_destination": true, "idgen_type": "uuidv4", "idgen_event_prefix": "", }Depending on the configured MQ, it will add the following fields:
RabbitMQ:
{ "rabbitmq_url": "amqp://***:***@rabbitmq:5672", "rabbitmq_exchange": "outpost", "rabbitmq_delivery_queue": "outpost-delivery", "rabbitmq_log_queue": "outpost-log" }AWS SQS:
{ "aws_access_key_configured": true, "aws_secret_key_configured": true, "aws_region": "us-east-1", "aws_delivery_queue": "queue-name" "aws_log_queue": "log-queue-name" }GCP Pub/Sub:
{ "gcp_credentials_configured": true, "gcp_project_id": "project_id" "gcp_delivery_topic": "topic", "gcp_delivery_subscription": "subscription-name", "gcp_log_topic": "log-topic", "gcp_log_subscription": "log-subscription-name" }Azure Service Bus:
{ "azure_connection_string_configured": true, "azure_delivery_topic": "topic", "azure_delivery_subscription": "subscription-name", "azure_log_topic": "log-topic", "azure_log_subscription": "log-subscription-name" }