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

debug mode: more readable logging #297

Merged
merged 5 commits into from
Sep 14, 2021
Merged

Conversation

dalf
Copy link
Member

@dalf dalf commented Sep 6, 2021

What does this PR do?

  • In development add a new dependency: coloredlogs (declared in requirements-dev.txt but optional in the code, see searx/__init__.py)
  • Logs about an engine always use the engine logger ( one logger per engine #296 ), include log from searx.search.processors and searx.metrics.
  • HTTP request are logged with the network name: a wikipedia HTTP request is logged with searx.network.wikipedia instead of httpx._client.

Make the debug log more readable:

image

Why is this change important?

A lot of information are logged in debug mode, which are not easy to read.

But it makes messages more difficult to find in the code:
image

But a git grep is easy:

$ git grep "engine time"
searx/search/__init__.py:        # max of all selected engine timeout
searx/search/__init__.py:                    PROCESSORS[th._engine_name].logger.error('engine timeout')

How to test this PR locally?

  • make run
  • production mode:
    • change the secret key
    • start in production mode
  • check the log with uwsgi
  • check the log with docker

Draft because

Author's checklist

Related issues

@dalf dalf changed the title Engine logger enh debug mode: more readable logging Sep 6, 2021
@dalf
Copy link
Member Author

dalf commented Sep 7, 2021

@return42 @unixfox what are you opinions on this type of log?

@return42
Copy link
Member

return42 commented Sep 7, 2021

It seems coloredlogs is active in dumb terminals ...

grafik

In the lib.sh we use ..

searxng/utils/lib.sh

Lines 112 to 114 in 9ef7f38

if [ ! -p /dev/stdout ] && [ ! "$TERM" = 'dumb' ] && [ ! "$TERM" = 'unknown' ]; then
set_terminal_colors
fi

to detect pipes and dumb terminals ...

Is there something similar we can do when initialize coloredlogs?

@dalf
Copy link
Member Author

dalf commented Sep 7, 2021

I can't find something about this, but this function should be enough:

import os
import sys

def is_color_terminal():
    if os.getenv('TERM', None) in ('dumb', 'unknown'):
        return False
    return sys.stdout.isatty()

@dalf dalf force-pushed the engine-logger-enh branch 2 times, most recently from 7cf04dc to db9ddd8 Compare September 7, 2021 16:34
@dalf
Copy link
Member Author

dalf commented Sep 7, 2021

Updated:

  • make run | cat works without color.
  • TERM=dumb make run works too.

@unixfox
Copy link
Member

unixfox commented Sep 7, 2021

Looks cool, I like it.

Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Tested / much better to read logs, like it .. Thanks!

@return42 return42 marked this pull request as ready for review September 8, 2021 06:18
metrics & processors use the engine logger
in debug mode, add some padding to make the output more readable
in procution mode, add the timestamp
For example wikipedia requests use the logger name "searx.network.wikipedia"

Log is disable when searx_debug is False
set:
* the root logger level to WARNING
* the werkzeug logger level to WARNING (useful when there is no uwsgi)
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

@dalf you force pushed but did not ask for a review .. can we merge this PR anyway?

For me this PR works and I like to use this logging in my development :)

@dalf
Copy link
Member Author

dalf commented Sep 12, 2021

About push force fix this method:

async def log_response(self, response: httpx.Response):
request = response.request
status = f"{response.status_code} {response.reason_phrase}"
response_line = f"{response.http_version} {status}"
if hasattr(response, "_elapsed"):
elapsed_time = f"{response.elapsed.total_seconds()} sec"
else:
elapsed_time = "stream"
self._logger.debug(
f'HTTP Request: {request.method} {request.url} "{response_line}" ({elapsed_time})'
)

Before the push force, the embedded image proxy didn't work because log_response was expecting a value for response.elapsed. Since the response is streamed, there is no value.

The new value takes of that.


About merging: everything works except one thing: HTTP redirect are not logged anymore.
See httpx PR : encode/httpx#1806
We need to upgrade to httpx 0.19.0 or later --> #261

If the missing log are not an issue (it is temporary), we can merge.

@return42
Copy link
Member

return42 commented Sep 12, 2021

If the missing log are not an issue (it is temporary), we can merge.

Thanks for the info, then I think it is better you decide if we should merge :)

@dalf
Copy link
Member Author

dalf commented Sep 14, 2021

Ok I'm going to merge.


httpx uses rich was has nice traceback logger, but it requires more configuration to fit into SearXNG.

But it is a dev dependency we can change later.

@dalf dalf merged commit 602cbc2 into searxng:master Sep 14, 2021
@dalf dalf deleted the engine-logger-enh branch October 2, 2021 11:33
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.

None yet

3 participants