Merge lp:~bloodearnest/canonical-identity-provider/hi-fi-logging into lp:canonical-identity-provider/release

Proposed by Simon Davy
Status: Rejected
Rejected by: Simon Davy
Proposed branch: lp:~bloodearnest/canonical-identity-provider/hi-fi-logging
Merge into: lp:canonical-identity-provider/release
Diff against target: 91 lines (+7/-48)
3 files modified
Makefile (+1/-1)
django_project/settings_base.py (+1/-46)
requirements.txt (+5/-1)
To merge this branch: bzr merge lp:~bloodearnest/canonical-identity-provider/hi-fi-logging
Reviewer Review Type Date Requested Status
Canonical ISD hackers Pending
Review via email: mp+286877@code.launchpad.net

Description of the change

Use the new logging library, talisker:

https://code.launchpad.net/~bloodearnest/+junk/talisker

This library aims to standardise logging across our services, as well as provide structured logging. It does the following

 - a standard log format
 - arbitrary key=value pairs
 - log to stderr, for the charm to handle files, directories and perms, etc (12 factor app)

This change:

 - uses the gunicorn wrapper for make run, which sets up logging
 - disable django's default logging, as it's not needed any more (and has bad production defaults, see https://docs.djangoproject.com/en/1.8/topics/logging/#default-logging-configuration )

To post a comment you must log in.
1399. By Simon Davy

clean up whitespace

Revision history for this message
Maximiliano Bertacchini (maxiberta) :
Revision history for this message
Simon Davy (bloodearnest) wrote :

On Tue, Feb 23, 2016 at 4:34 PM, Maximiliano Bertacchini
<email address hidden> wrote:
>
>
> Diff comments:
>
>> === modified file 'Makefile'
>> --- Makefile 2016-01-21 15:45:34 +0000
>> +++ Makefile 2016-02-23 10:59:30 +0000
>> @@ -151,7 +151,7 @@
>>
>> run: ARGS=0.0.0.0:8000
>> run:
>> - $(ENV)/bin/gunicorn django_project.wsgi:application --workers=2 --reload --pid=logs/gunicorn.pid --bind=$(ARGS) --timeout=99999 --error-logfile=- --access-logfile=-
>> + $(ENV)/bin/talisker_gunicorn sso --devel -- django_project.wsgi:application --reload --pid logs/gunicorn.pid --access-logfile - --timeout 99999 --workers 2 --bind=$(ARGS)
>
> Is the --devel flag necessary?

For that makefile rule, yes. The defaults are production orientated,
and thus silence python's warning system (which is also what django's
default logging did before). --devel logs the warnings, which is what
we want in development, when this make target is designed to be used.

Thanks

--
Simon

Unmerged revisions

1399. By Simon Davy

clean up whitespace

1398. By Simon Davy

use explicit gunicorn version

1397. By Simon Davy

merging

1396. By Simon Davy

Remove manual wsgi wrapping

1395. By Simon Davy

merging

1394. By Simon Davy

Use new talisker runner

1393. By Simon Davy

basic talisker integration

1392. By Simon Davy

merging

1391. By Simon Davy

fix timezone formatting in access logger

1390. By Simon Davy

merging

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2016-01-21 15:45:34 +0000
+++ Makefile 2016-02-23 10:59:30 +0000
@@ -151,7 +151,7 @@
151151
152run: ARGS=0.0.0.0:8000152run: ARGS=0.0.0.0:8000
153run:153run:
154 $(ENV)/bin/gunicorn django_project.wsgi:application --workers=2 --reload --pid=logs/gunicorn.pid --bind=$(ARGS) --timeout=99999 --error-logfile=- --access-logfile=- 154 $(ENV)/bin/talisker_gunicorn sso --devel -- django_project.wsgi:application --reload --pid logs/gunicorn.pid --access-logfile - --timeout 99999 --workers 2 --bind=$(ARGS)
155155
156start: bootstrap start-db156start: bootstrap start-db
157157
158158
=== modified file 'django_project/settings_base.py'
--- django_project/settings_base.py 2016-01-27 12:00:39 +0000
+++ django_project/settings_base.py 2016-02-23 10:59:30 +0000
@@ -318,52 +318,7 @@
318LOCALE_PATHS = [318LOCALE_PATHS = [
319 os.path.join(BASE_DIR, 'branches/translations/locale'),319 os.path.join(BASE_DIR, 'branches/translations/locale'),
320]320]
321LOGGING = {321LOGGING_CONFIG = None
322 'disable_existing_loggers': False,
323 'filters': {},
324 'formatters': {
325 'simple': {
326 'datefmt': None,
327 'format': '%(asctime)s %(levelname)s %(name)s %(message)s',
328 },
329 'verbose': {
330 'datefmt': None,
331 'format': ('%(asctime)s %(levelname)s %(module)s %(process)d '
332 '%(thread)d %(message)s'),
333 },
334 },
335 'handlers': {
336 'file': {
337 'filename': os.path.join(LOGS_DIR, 'sso.log'),
338 'formatter': 'simple',
339 'class': 'logging.FileHandler',
340 'filters': '',
341 'level': 'INFO',
342 },
343 },
344 'incremental': False,
345 'loggers': {
346 'sso': {
347 'level': 'INFO',
348 'propagate': False,
349 'filters': [],
350 'handlers': ['file'],
351 },
352 'django': {
353 'level': 'INFO',
354 'propagate': False,
355 'filters': [],
356 'handlers': ['file'],
357 },
358 },
359 'root': {
360 'level': 'INFO',
361 'filters': [],
362 'handlers': ['file'],
363 },
364 'version': 1,
365}
366LOGGING_CONFIG = 'django.utils.log.dictConfig'
367LOGIN_REDIRECT_URL = '/'322LOGIN_REDIRECT_URL = '/'
368LOGIN_URL = '/+login'323LOGIN_URL = '/+login'
369LOGOUT_URL = '/accounts/logout/'324LOGOUT_URL = '/accounts/logout/'
370325
=== modified file 'requirements.txt'
--- requirements.txt 2016-02-01 15:30:36 +0000
+++ requirements.txt 2016-02-23 10:59:30 +0000
@@ -10,7 +10,6 @@
10django-secure==1.0.110django-secure==1.0.1
11django-statsd-mozilla==0.3.1511django-statsd-mozilla==0.3.15
12gargoyle==0.11.012gargoyle==0.11.0
13gunicorn==19.3.0
14lazr.authentication==0.1.313lazr.authentication==0.1.3
15nexus==0.3.114nexus==0.3.1
16oath==1.4.015oath==1.4.0
@@ -29,3 +28,8 @@
29timeline==0.0.428timeline==0.0.4
30timeline-django==0.0.429timeline-django==0.0.4
31whitenoise==2.0.630whitenoise==2.0.6
31
32# temporary talisker deps
33talisker==0.1.0
34gunicorn==19.4.5.dev1.690b11e
35