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

Add the error code in the logs #3415

Open
steve-chavez opened this issue Apr 16, 2024 · 3 comments
Open

Add the error code in the logs #3415

steve-chavez opened this issue Apr 16, 2024 · 3 comments
Labels
idea Needs of discussion to become an enhancement, not ready for implementation logging observability

Comments

@steve-chavez
Copy link
Member

Problem

Currently the logs show errors (on log-level=error or warn) as:

127.0.0.1 - postgrest_test_anonymous [16/Apr/2024:14:11:52 -0500] "GET /designers?select=x(id) HTTP/1.1" 400 - "" "curl/7.81.0"

When doing:

curl 'localhost:3000/designers?select=x(id)'

{"code":"PGRST200","details":"Searched for a foreign key relationship between 'designers' and 'x' in the schema 'test', but no matches were found.","hint":null,"message":"Could not find a relationship between 'designers' and 'x' in the schema cache"}

But the status code on the log line is not enough to identify the error. Considering that 400 is used for various errors (ref)

Solution

Add the error code next to the status code on the logs:

127.0.0.1 - postgrest_test_anonymous [16/Apr/2024:14:11:52 -0500] "GET /designers?select=x(id) HTTP/1.1" 400 (PGRST200) - "" "curl/7.81.0"
@steve-chavez steve-chavez added idea Needs of discussion to become an enhancement, not ready for implementation logging labels Apr 16, 2024
@wolfgangwalther
Copy link
Member

This breaks the convention of the "Combined Log Format", that we are using, so far.

To do such things we should change the log format entirely, for example produce JSON logs or so.

@steve-chavez
Copy link
Member Author

Are we really complying to http://justsolve.archiveteam.org/wiki/Combined_Log_Format? Right now we're not adding the "Number of bytes transferred in requested object" field.

To do such things we should change the log format entirely, for example produce JSON logs or so.

I wouldn't be opposed to json logs but that seems like another feature.

Nginx for example allows configuring its plain text log format: https://docs.nginx.com/nginx/admin-guide/monitoring/logging/#setting-up-the-access-log

So maybe we could add a config for this?

@steve-chavez
Copy link
Member Author

Btw, it shouldn't be hard to implement a json logger. We already have all the logic centralized in Logger.hs, some of the required functions are already there too. We would just need to stop using the Logger.middleware.

Maybe we can have a config similar to pg:

log_destination = 'jsonlog' # Valid values are combinations of
# stderr, csvlog, jsonlog, syslog, and
# eventlog, depending on platform.
# csvlog and jsonlog require
# logging_collector to be on.

Like:

log_format = "combined" ## default
# combined-w-error-code
# jsonlog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Needs of discussion to become an enhancement, not ready for implementation logging observability
Development

No branches or pull requests

2 participants