Project

General

Profile

Development #23471

ajout du support de logs structurés vers journald

Added by Benjamin Dauvergne about 1 year ago. Updated 5 months ago.

Status:
Solution déployée
Priority:
Normal
Category:
-
Start date:
27 Apr 2018
Due date:
% Done:

100%

Patch proposed:
Yes
Planning:
No

Description

  • utiliser python-systemd
  • capitaliser nos champs structurés (journald l'exige :/ )

0002-hobo-do-not-clobber-the-resolved-user-in-RequestCont.patch View (732 Bytes) Benjamin Dauvergne, 27 Apr 2018 12:57 AM

0001-debian-add-journald-support-to-debian_config_common-.patch View (2.79 KB) Benjamin Dauvergne, 27 Apr 2018 12:57 AM

journal.py.diff View (28.1 KB) Benjamin Dauvergne, 04 Dec 2018 08:37 PM

0001-hobo-do-not-clobber-the-resolved-user-in-RequestCont.patch View (732 Bytes) Benjamin Dauvergne, 05 Dec 2018 01:59 PM

0002-debian-add-journald-support-to-debian_config_common-.patch View (13.9 KB) Benjamin Dauvergne, 05 Dec 2018 01:59 PM

Associated revisions

Revision 203b8894 (diff)
Added by Benjamin Dauvergne 5 months ago

hobo: do not clobber the resolved user in RequestContextFilter (#23471)

Revision ffaeb450 (diff)
Added by Benjamin Dauvergne 5 months ago

debian: add journald support to debian_config_common (fixes #23471)

History

#1 Updated by Benjamin Dauvergne about 1 year ago

  • 0001 : assez évident
  • 0002 : bug introduit en #11316, même si je ne sais pas si la fonctionnalité de passer explicitement un user aux appels de log est utilisée.

#2 Updated by Benjamin Dauvergne about 1 year ago

Il faut backporter python-systemd 234 de testing pour que ce soit utile (la version 233 dans jessie-backports/stretch ne transmet pas le contenu complet de record.__dict__ à journald).

#3 Updated by Emmanuel Cazenave about 1 year ago

Est-ce qu'avec ça on peut bien filtrer les logs de journald par unit ?

Ici tu laisses de coté les loggers 'sentry.errors' et 'py.warnings' qui utilisent encore le handler syslog.

Idélament je serai d'abord pour tester une solution plus radicale : se passer de python-systemd. Si systemd est présent, remplacer partout le handler syslog par un handler stdout ou stderr. Par défaut systemd envoie tout à journald. A voir si ça pollue pas trop dans un terminal lors de commandes de management. Il nous manque aussi quelques unit file (hobo n'en a pas).

#4 Updated by Benjamin Dauvergne about 1 year ago

Emmanuel Cazenave a écrit :

Est-ce qu'avec ça on peut bien filtrer les logs de journald par unit ?

Oui, ça ne peut pas bloquer ça, il y a toujours la unit citée dans les enregistrements journald, ce qui est moche c'est que via syslog on a le choix du formatage alors qu'avec le passage journald->syslog on a forcément comme nom de programme /usr/bin/gunicorn parce que c'est journald qui formate de façon uniforme "$date $user $program".

Ici tu laisses de coté les loggers 'sentry.errors' et 'py.warnings' qui utilisent encore le handler syslog.

Bien vu je vais corriger ça.

Idélament je serai d'abord pour tester une solution plus radicale : se passer de python-systemd. Si systemd est présent, remplacer partout le handler syslog par un handler stdout ou stderr. Par défaut systemd envoie tout à journald. A voir si ça pollue pas trop dans un terminal lors de commandes de management. Il nous manque aussi quelques unit file (hobo n'en a pas).

Ça me va d'aller d'abord vers ce que tu dis mais là mon objectif c'était d'avoir des logs structurés, donc de pouvoir filtrer au niveau journald par tenant, utilisateur, uuid, etc... et de le faire en plus sur le serveur qui centralise tous les logs, ainsi on peut voir l'activité sur l'ensemble de la plate-forme d'un client (si on avait en plus un champ "PLATFORM" en provenance de hobo ce serait top).

#5 Updated by Benjamin Dauvergne about 1 year ago

  • File 0001-tests_multitenant-PEP8ness-and-cleanup-23471.patch added
  • File 0002-use-pg_advisory_lock-to-prevent-running-cronjobs-dur.patch added

#6 Updated by Benjamin Dauvergne about 1 year ago

  • File deleted (0002-use-pg_advisory_lock-to-prevent-running-cronjobs-dur.patch)

#7 Updated by Benjamin Dauvergne about 1 year ago

  • File deleted (0001-tests_multitenant-PEP8ness-and-cleanup-23471.patch)

#8 Updated by Emmanuel Cazenave about 1 year ago

Ici tu laisses de coté les loggers 'sentry.errors' et 'py.warnings' qui utilisent encore le handler syslog.

Peut-être que quelque chose m'échappe mais j'ai l'impression qu'en l'état ça ne marche pas : il faut que le filter 'request_context' soit ajouté à ton handler pour qu'on ait les infos contextuelles qui vont bien.

Aussi j'avais pas vu la première fois, mais le monkeypatch de python-systemd ...

Thomas dit aussi que c'est une galère pas possible, voir impossible de backporter python-systemd.

#9 Updated by Benjamin Dauvergne about 1 year ago

Bon ben tant pis alors, ça aurait été beau (Thomas si tu peux dire les soucis que tu entrevois pour le backport, genre si ça changeait un jour qu'on le sache).

#10 Updated by Benjamin Dauvergne about 1 year ago

Je réponds quand même au reste.

Emmanuel Cazenave a écrit :

Ici tu laisses de coté les loggers 'sentry.errors' et 'py.warnings' qui utilisent encore le handler syslog.

Peut-être que quelque chose m'échappe mais j'ai l'impression qu'en l'état ça ne marche pas : il faut que le filter 'request_context' soit ajouté à ton handler pour qu'on ait les infos contextuelles qui vont bien.

Oui tu as raison.

Aussi j'avais pas vu la première fois, mais le monkeypatch de python-systemd ...

Ils mettent une contrainte un peu chiante sur le nommage des attributs du record, on peut s'en passer si on change le nommage dans request_context, mais faut changer aussi les chaînes de formatage dans ce cas, qui font référence explicitement à ces propriétés.

            'format': '%(application)s %(levelname)s %(tenant)s %(ip)s %(user)s %(request_id)s'
                      ' %(message)s',

#11 Updated by Thomas Noël about 1 year ago

Benjamin Dauvergne a écrit :

Bon ben tant pis alors, ça aurait été beau (Thomas si tu peux dire les soucis que tu entrevois pour le backport, genre si ça changeait un jour qu'on le sache).

Backbackporter systemd sur jessie me semble déraisonnable. Utiliser le backport sur stretch beaucoup moins. Reste qu'il faut passer à stretch, donc django 1.11. Ce patch devra sans doute attendre encore un peu ;)

#12 Updated by Benjamin Dauvergne about 1 year ago

J'ai l'impression que tu sous-entends que ça demande de backporter systemd (je n'avais pas conscience que ça en faisait partie, mais en voyant le numéro de version ça devient logique).

#13 Updated by Benjamin Dauvergne about 1 year ago

  • Priority changed from Normal to Bas

à voir plus tard (ou si quelqu'un se sent, backporter juste python-systemd).

#14 Updated by Benjamin Dauvergne 7 months ago

  • Priority changed from Bas to Normal

Comme on est pas loin de passer à stretch, je remonte.

#15 Updated by Frédéric Péters 7 months ago

Tu aurais une branche rebasée à pousser en wip ?

#17 Updated by Benjamin Dauvergne 7 months ago

Un petit topo sur les limitations :
  • systemd récent (strech-backports) parce que sinon python-systemd (qui est généré par le même paquet source que systemd) ne prend pas en compte les autres champs

sd_journal_send() may be used to submit structured log entries to the system journal. It takes a series of format strings, each immediately followed by their associated parameters, terminated by NULL. The strings passed should be of the format "VARIABLE=value". The variable name must be in uppercase and consist only of characters, numbers and underscores, and may not begin with an underscore. (All assignments that do not follow this syntax will be ignored.)

  • on est obligé de mettre en majuscule tous nos champs avant de les passer à l'API systemd sinon ils sont ignorés, ça passe malheureusement par du monkeypatching, mais on pourrait aussi faire proprement de l'héritage pour modifier JournalHandler.emit() mais comme il s'y passe pas mal de chose c'est plus simple de monkeypatché la fonction systemd.journal.send().

#18 Updated by Frédéric Péters 7 months ago

systemd récent (strech-backports)

J'avais raté ça, ça reste pour moi pas folichon de devoir suivre les changements/bugs de systemd dans stretch-backports :/

#19 Updated by Benjamin Dauvergne 7 months ago

Frédéric Péters a écrit :

systemd récent (strech-backports)

J'avais raté ça, ça reste pour moi pas folichon de devoir suivre les changements/bugs de systemd dans stretch-backports :/

systemd n'est pas important c'est le paquet python-systemd qui pose problème, si on backporte le fix en question ça peut aller aussi (l'implémentation de JournalHandler.emit() uniquement), on peut aussi s'en sortir en copiant le fichier journal.py dans hobo.

#20 Updated by Benjamin Dauvergne 7 months ago

Le diff entre journal.py 230 (jessie) et 234 (stretch-backports), il n'y a vraiment que JournalHandler.emit() qui a changé (en dehors de choses concernant la lecture des logs).

#21 Updated by Benjamin Dauvergne 7 months ago

Voilà, j'ai backporté le handler de systemd 234 dans hobo.journal et j'ai ajouté un test en utitilisant le systemd local, lancer les tests nécessite d'avoir libsystemd-dev d'installer sur le système pour pouvoir installer systemd-python 234 depuis pypi.

#22 Updated by Benjamin Dauvergne 6 months ago

  • Tracker changed from Bug to Development

#23 Updated by Frédéric Péters 5 months ago

Le hobo/journal.py qui est copié de systemd, tu pourrais le taper dans un vendor/systemd/journal.py, genre ?

-        from graypy import GELFHandler
+        from systemd import journal

et ce bout-là serait alors à changer en from .vendor.systemd, ou ce que tu auras choisi.

+ if os.path.exists('/run/systemd/journal/socket'):

Juste tester /run/systemd/journal peut-être ? (pour tester que systemd est utilisé, la bonne pratique est de tester /run/systemd/systemd/, pas les trucs à l'intérieur, je me dis que pour journald ça peut être pareil).

#24 Updated by Frédéric Péters 5 months ago

  • Status changed from Solution proposée to Solution validée

Le hobo/journal.py qui est copié de systemd, tu pourrais le taper dans un vendor/systemd/journal.py, genre ?

Bon en fait je relis et c'est juste ce fichier mais la dépendance à python-systemd existe quand même; bon, ok.

(aucune idée de comment ça sera exploité derrière).

#25 Updated by Benjamin Dauvergne 5 months ago

  • % Done changed from 0 to 100
  • Status changed from Solution validée to Résolu (à déployer)

#26 Updated by Frédéric Péters 5 months ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF