Logging improvements #216
Merged
Logging improvements #216
Conversation
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
tempusfrangit
force-pushed
the
logging
branch
from
cea3226 to
0ec1e75
Compare
tempusfrangit
force-pushed
the
logging-improvements
branch
from
e79f4e3 to
c450a17
Compare
tempusfrangit
force-pushed
the
logging
branch
from
0ec1e75 to
f9009b9
Compare
tempusfrangit
force-pushed
the
logging-improvements
branch
from
c450a17 to
4df689e
Compare
tempusfrangit
force-pushed
the
logging
branch
from
f9009b9 to
a9f8075
Compare
- Create internal/logging package with embedded zap logger - Add Trace level (-8) below Debug level with full sugar support - Implement Python process protection to cap log levels at debug - Replace external github.com/replicate/go/logging dependency - Wire new logger throughout codebase (main.go, server, service, manager) - Create loggingtest package for test logger compatibility - Update all test files to use new test logger infrastructure - Add comprehensive unit tests for logging functionality - Configure golangci-yml with depguard to prevent test package misuse - Support environment variables: COG_LOG_LEVEL, LOG_LEVEL, LOG_FORMAT, LOG_FILE
- Add custom level encoders (uppercase, lowercase, color) to display "TRACE" instead of "LEVEL(-8)" - Implement test logger with proper TRACE level display for zaptest compatibility - Add SugaredLogger chaining support (With/Named) to maintain custom type - Migrate 25 verbose Debug logs to Trace level across runner, server, service, webhook - Keep important Debug logs for actual debugging (errors, state changes, subprocess management) - Verbose implementation details now at Trace: file operations, response watchers, shutdown flow
tempusfrangit
force-pushed
the
logging-improvements
branch
from
4df689e to
9c5dbc9
Compare
- Move 16 Info logs to Debug: component initialization, runner management, prediction flow details - Keep critical Info logs: service lifecycle, prediction completion, force kills, webhooks - Move 4 Info logs to Trace: internal shutdown flow and cleanup state tracking - Maintain clean Info level for exceptional events only - Remove unused customLevelEncoder function to fix linter warnings
tempusfrangit
requested review from
Copilot and
meatballhat
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR implements trace logging and factors out replicate/go/logging by creating a new internal logging package. The changes include replacing all external logging dependencies with the custom internal logging solution and introducing trace-level logging functionality.
- Introduces custom internal logging package with trace level support
- Replaces zap/zaptest usage throughout codebase with internal logging implementations
- Updates log levels from debug/info to trace for more detailed debugging information
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/logging/logger.go | New custom logging implementation with trace level support |
| internal/logging/logging_test.go | Tests for the new logging functionality |
| internal/loggingtest/test_helper.go | Test utility for creating loggers in tests |
| internal/loggingtest/test_helper_test.go | Tests for the logging test utilities |
| cmd/cog/main.go | Updated to use internal logging instead of replicate/go/logging |
| internal/service/service.go | Updated logging calls and switched to internal logging |
| internal/server/server.go | Updated to use internal logging and trace-level calls |
| internal/webhook/webhook.go | Updated to use internal logging and trace-level calls |
| internal/runner/runner.go | Updated to use internal logging and trace-level calls |
| internal/runner/manager.go | Updated to use internal logging and trace-level calls |
| internal/webhook/webhook_test.go | Updated to use loggingtest helper |
| internal/tests/harness_test.go | Updated to use loggingtest helper |
| internal/runner/runner_test.go | Updated to use loggingtest helper |
| internal/runner/manager_test.go | Updated to use loggingtest helper |
| .golangci.yml | Added depguard rule to prevent loggingtest usage in production code |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
meatballhat
approved these changes
Sep 16, 2025
Contributor
There was a problem hiding this comment.
IIUC this is effectively a vendored copy of github.com/replicate/go/logging, right?
Member
Author
There was a problem hiding this comment.
Mostly. It does much of what replicate/go does but also implements the Trace level logging. We can either:
- pull this code back to replicate/go
- leave it here, diverged (we don't need trace logging in replicate/go really)
- plan to drop replicate/go/logging across all code bases (this is a bigger decision than should be determined here in a gh pr).
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.
Implements trace logging and factors out replicate/go/logging.