Skip to content

Conversation

@michaeldwan
Copy link
Member

Cog sets COG_LOG_LEVEL=warning when running predictors.

Python's logger (used before cog-runtime came along) can parse warning,WARNING,warn,WARN zap only accepts warn,WARN.

This difference is causing Failed to parse log level "warning": unrecognized level "warning" to show up in user-facing logs.

We can fix the issue in cog, but we'd still have differences in behavior depending on the version of cog, cog-runtime, or cog-python being used. So safest thing is to normalize it here.

Cog sets `COG_LOG_LEVEL=warning` when running predictors.

Python's logger (used before cog-runtime came along) can parse `warning,WARNING,warn,WARN`
zap only accepts `warn,WARN`.

This difference is causing `Failed to parse log level "warning": unrecognized level "warning"` to show up in user-facing logs.

We _can_ fix the issue in cog, but we'd still have differences in behavior depending on the version of cog, cog-runtime, or cog-python being used. So safest thing is to normalize it here.
Copilot AI review requested due to automatic review settings September 12, 2025 20:31
@michaeldwan michaeldwan requested a review from a team as a code owner September 12, 2025 20:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Normalizes the log level environment variable COG_LOG_LEVEL to use "warn" instead of "warning" for compatibility with zap's log level parser, preventing user-facing error messages about unrecognized log levels.

  • Converts "WARNING" to "WARN" to match zap's expected format
  • Maintains uppercase formatting for consistency with zap parsing
  • Sets default log level to "INFO" when no level is specified

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +120 to 126
logLevel := strings.ToUpper(os.Getenv("COG_LOG_LEVEL"))
switch logLevel {
case "WARNING":
logLevel = "WARN"
case "":
logLevel = "INFO"
}
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch statement only handles uppercase values but converts all input to uppercase first. Consider adding a case for lowercase 'warning' or documenting that the ToUpper conversion handles case-insensitive matching.

Copilot uses AI. Check for mistakes.
@tempusfrangit
Copy link
Member

In theory go/replcate/logging does this already https://github.com/replicate/go/blob/b69e565fb9392b3016ed64c2e833d9c19b320e87/logging/logging.go#L39

Related, logging is on the short list to address post big refactor (introduce trace level and squash even more logging). Also big refactor resolved the ability to set debug level logging.

@michaeldwan
Copy link
Member Author

@tempusfrangit what's the right way to proceed? I just want the warnings to stop showing in cog. I also saw there's 2 places we parse logs in main, which we may want to unify: https://github.com/search?q=repo%3Areplicate%2Fcog-runtime%20ParseLevel&type=code

@michaeldwan
Copy link
Member Author

replaced by #216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants