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 request-id to log entries, log as json, add sql log #1034

Merged
merged 3 commits into from
Aug 10, 2018
Merged

Add request-id to log entries, log as json, add sql log #1034

merged 3 commits into from
Aug 10, 2018

Conversation

alikins
Copy link
Contributor

@alikins alikins commented Aug 7, 2018

This is more or less the config I'm using for local devel stuff at the moment.
Can simplify/split commits/move to defaults.py etc as needed.

Add 'request_id' and json formatting to galaxy logs

This adds a log_request_id.filters.RequestIDFilter log filter
to the console/stdout log handlers. The filter adds the
request-id to log records so they can be used by the formatters.

Also add and setup a 'jog.JogFormatter' for formatting normal
python log entries as JSON.

Add RequestIdMiddleware to MIDDLEWARE_CLASSES

This enables adding or tracking a X-Request-Id per
request that can be used in logging.

https://github.com/dabapps/django-log-request-id

various tweaks of log levels up to DEBUG-ish levels

alikins and others added 2 commits August 9, 2018 15:49
This enables adding or tracking a X-Request-Id per
request that can be used in logging.

https://github.com/dabapps/django-log-request-id

Add 'django-log-request-id' and 'jog' to requirements

Add RequestIdMiddleware to MIDDLEWARE_CLASSES

This adds a log_request_id.filters.RequestIDFilter log filter
to the console/stdout log handlers. The filter adds the
request-id to log records so they can be used by the formatters.

Also add and setup a 'jog.JogFormatter' for formatting normal
python log entries as JSON.
@chouseknecht
Copy link
Contributor

chouseknecht commented Aug 10, 2018

Pushed a commit that sets the config via a function. Seems a little more concise to me, but if you hate it, feel free to revert. Fixed the lint issue too.

Tried it out locally by setting DJANGO_SETTINGS_MODULE=galaxy.settings.production and giving GALAXY_SECRET_KEY a value. Logging comes out as a JSON object with request_id populated.

Wasn't sure if there is some magic to getting SQL to appear in the log, other than turning on logging for django.db.backend. That's OK though. I don't think we want all the SQL logged. In a production setting, I think it will be way too verbose.

Overall LGTM.

@chouseknecht
Copy link
Contributor

Back port to release/3.0.8

@alikins
Copy link
Contributor Author

alikins commented Aug 10, 2018

@chouseknecht

Wasn't sure if there is some magic to getting SQL to appear in the log, other than turning on logging for django.db.backend. That's OK though. I don't think we want all the SQL logged. In a production setting, I think it will be way too verbose.

No other magic required to log it. The previous versions of this branch had changes to improve the output and added a handler, formatter, and filters for SQL. The handler was just to log to a separate file (useful for localhost dev work, but not useful in openshift). The custom formatter just makes the output easier to read. The custom filter was used to ignore the large number of queries made by celery it uses to poll for new tasks.

@alikins
Copy link
Contributor Author

alikins commented Aug 10, 2018

I don't think we want all the SQL logged. In a production setting, I think it will be way too verbose.

Definitely too verbose in production.

One thing we could investigate for the future would be allowing log levels to be increased for particular authenticated users. That could include SQL logging. We did this for the RHSM server (candlepin) and it was very useful for tracking down problems that only showed up for some users.

@@ -303,6 +304,11 @@
'disable_existing_loggers': False,

'formatters': {
'json': {
'()': 'jog.JogFormatter',
'format': ('%(asctime)s %(request_id)s %(levelname)s] '
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I left an unneeded ']' in there that can be removed.

@alikins
Copy link
Contributor Author

alikins commented Aug 10, 2018

@chouseknecht

Pushed a commit that sets the config via a function. Seems a little more concise to me, but if you hate it, feel free to revert

My preference is the explicit data style, since you usually end up needing the granularity, but if the method works, thats okay for now.

@alikins
Copy link
Contributor Author

alikins commented Aug 10, 2018

LGTM

@chouseknecht chouseknecht changed the title [WIP] Add request-id to log entries, log as json, add sql log Add request-id to log entries, log as json, add sql log Aug 10, 2018
@chouseknecht chouseknecht merged commit 06497a8 into ansible:devel Aug 10, 2018
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.

2 participants