Merged
Conversation
Closed
There was a problem hiding this comment.
Pull request overview
Introduces structured logging across GoNetSim using log/slog, with configurable log format/level and per-service logger injection to improve observability and log consistency.
Changes:
- Add
internal/observability.NewLogger(tint text output or JSON) and wire it into CLI commands. - Inject per-service
*slog.Loggervia a new optionalservice.LoggerAwareinterface and update HTTP/DNS services to use it. - Add logging configuration defaults and TOML config keys (
[logging] format/level), plus small TTY-aware banner formatting.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| main.go | TTY-aware banner formatting and new color/TTY deps. |
| cmd/root.go | Create/configure default slog logger from config; structured startup/error logging. |
| cmd/dns.go | Configure slog logger for standalone DNS command. |
| cmd/http.go | Configure slog logger for standalone HTTP command. |
| cmd/https.go | Configure slog logger for standalone HTTPS command. |
| internal/observability/logging.go | New logger factory supporting text (tint) and JSON output. |
| internal/config/config.go | Add logging config to schema, defaults, and validation. |
| internal/config/default_config.toml | Add default [logging] section. |
| internal/service/service.go | Add LoggerAware interface for logger injection. |
| internal/service/manager.go | Switch manager to slog, add prefix handler, inject loggers into services. |
| internal/httpserver/config.go | Store injected logger on HTTP server service. |
| internal/httpserver/server.go | Pass logger into handler/server and use slog for listening logs. |
| internal/httpserver/fakemode.go | Structured request logging with captured status code. |
| internal/httpserver/http_test.go | Update server constructor calls for new logger parameter. |
| internal/dnsserver/config.go | Store injected logger on DNS server service. |
| internal/dnsserver/server.go | Add logger-aware server creation and structured query/listen logging. |
| go.mod | Add tint/colorable/isatty deps; bump x/sys. |
| go.sum | Add checksums for new deps / updated x/sys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solves #12