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

[TECH] Remplacer les usages de Logger par les méthodes avec correlations IDs (PIX-12823) #9157

Closed
wants to merge 2 commits into from

Conversation

Libouk
Copy link
Member

@Libouk Libouk commented Jun 4, 2024

🦄 Problème

Ajout d'infos dans les logs. Le correlationID n'est pas loggué partout pour l'instant.
Le but est de pouvoir prévenir automatiquement chaque équipe en cas d'erreur, en fonction d'un label @team qu'il faudra ajouter sur chaque route.

🤖 Proposition

Remplacer les usages de Logger.info, error, warn, etc par loggerInfoWithCorrelationId, etc

🌈 Remarques

Pour l'instant logger.warn & logger.traces & logger.fatal ne sont pas gérées

💯 Pour tester

Un log drain sur la review app a été ajouté pour avoir les logs dans scalingo.
(scalingo --region osc-fr1 -app pix-api-review-pr9157 log-drains-add --type datadog --token --drain-region eu --with-addons)
datadog logs
https://app-pr9157.review.pix.fr/

@Libouk Libouk added Development in progress cross-team Toutes les équipes de dev labels Jun 4, 2024
@Libouk Libouk requested a review from a team as a code owner June 4, 2024 13:29
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@BerengereC BerengereC closed this Jun 4, 2024
@BerengereC BerengereC reopened this Jun 4, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Libouk Libouk removed the request for review from a team June 4, 2024 15:04
@Libouk
Copy link
Member Author

Libouk commented Jun 4, 2024

Avis de @HEYGUL : Inverser la logique suivie ici, plutôt appeler logErrorWithCorrelationIds au sein de l'appel de logger.error. et ainsi de suite pour les autres méthodes logger.

@dlahaye
Copy link
Contributor

dlahaye commented Jun 10, 2024

Avis de @HEYGUL : Inverser la logique suivie ici, plutôt appeler logErrorWithCorrelationIds au sein de l'appel de logger.error. et ainsi de suite pour les autres méthodes logger.

👍 Oui, ça permettra 1) de respecter l'inversion de dépendance et 2) de moins (voire plus du tout) casser les tests

@dlahaye
Copy link
Contributor

dlahaye commented Jun 25, 2024

Inverser la logique suivie ici, plutôt appeler logErrorWithCorrelationIds au sein de l'appel de logger.error. et ainsi de suite pour les autres méthodes logger.

@Libouk @BerengereC je pense qu'on peut reprendre cette PR à zéro (sur une branche neuve sans conflits) en prenant en compte cette proposition. Cela permettrait d'isoler la nouvelle dépendance sans casser autant de tests et en limitant les risques de conflits d'ici le merge. Qu'en pensez-vous ?

@HEYGUL
Copy link
Contributor

HEYGUL commented Jul 10, 2024

On ferme la PR au profit de #9425 ?

@HEYGUL HEYGUL closed this Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team Toutes les équipes de dev Development in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants