Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ml): error logging #6646

Merged
merged 3 commits into from
Jan 26, 2024
Merged

fix(ml): error logging #6646

merged 3 commits into from
Jan 26, 2024

Conversation

mertalev
Copy link
Contributor

Description

The ML service's logging configuration is buggy. It only logs log.error calls, not actual exceptions. This led to misleading logs that suggested that it e.g. loads models several times, when in fact it has just been failing to load them and didn't print the exception.

I also tweaked the traceback output to suppress frames from FastAPI etc. The traceback ends up being very long and noisy with irrelevant frames. It still shows the path and line for those, but doesn't show the code.

Copy link

cloudflare-pages bot commented Jan 26, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: a768036
Status:⚡️  Build in progress...

View logs

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Were you able to produce a 500 and see it correctly log and pass the error the the immich-server/microservices?

@mertalev
Copy link
Contributor Author

I produced a 500 error by adding a runtime exception and requesting from Postman.

excluded-traceback

@mertalev
Copy link
Contributor Author

mertalev commented Jan 26, 2024

Also for reference, this is what it looks like without the traceback hack to exclude fastapi etc.

suppressed-traceback

@jrasm91
Copy link
Contributor

jrasm91 commented Jan 26, 2024

Yo that stack trace looks fancy haha

@mertalev
Copy link
Contributor Author

lol I can even get it to print local variables and stuff

@jrasm91 jrasm91 enabled auto-merge (squash) January 26, 2024 00:23
@jrasm91 jrasm91 merged commit ca28e1e into main Jan 26, 2024
23 of 24 checks passed
@jrasm91 jrasm91 deleted the fix/ml-error-logging branch January 26, 2024 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants