-
Notifications
You must be signed in to change notification settings - Fork 52
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] Récupérer les correlations IDs dans le logger.js (PIX-12823) #9425
base: dev
Are you sure you want to change the base?
Conversation
Une fois les applications déployées, elles seront accessibles via les liens suivants :
Les variables d'environnement seront accessibles via les liens suivants : |
703409c
to
f913844
Compare
f913844
to
9ec29d1
Compare
*/ | ||
const logErrorWithCorrelationIds = _logWithCorrelationIds(internalLogger.error); | ||
|
||
/** @param {object} data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** @param {object} data | |
/** @param {object|string} data |
Et idem pour la jsDoc du logError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
au passage est-ce qu'on en profiterait pas pour proposer une jsDoc aussi exhaustive pour le logInfo
que ce qui a été fait pour le logError
?
error: _logWithCorrelationIds(internalLogger.error), | ||
info: _logWithCorrelationIds(internalLogger.info), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suite aux remontées d'erreur de la CI:
/home/circleci/pix/api/node_modules/pino/lib/tools.js:59
if (typeof this[msgPrefixSym] === 'string' && msg !== undefined && msg !== null) {
Je pense qu'il faudrait plutôt faire comme ça
error: _logWithCorrelationIds(internalLogger.error), | |
info: _logWithCorrelationIds(internalLogger.info), | |
error: _logWithCorrelationIds(internalLogger.error.bind(internalLogger)), | |
info: _logWithCorrelationIds(internalLogger.info.bind(internalLogger)), |
Ou un truc du genre 😅
c201409
to
457be8c
Compare
457be8c
to
2f4be3f
Compare
🦄 Problème
Ajout d'infos dans les logs. Les correlationIDs (
request-id
&user-id
) ne sont pas loggués 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
Dans le fichier
logger.js
, utiliserloggerInfoWithCorrelationId
lors de l'appel delogger.info
, etloggerErrorWithCorrelationId
lors de l'appel delogger.error
.Le
logger.js
expose alors sesloggerInfoWithCorrelationId
etloggerErrorWithCorrelationId
afin de permettre àmonitoring-tools
de les utiliser sans nécessiter de grosses refacto🌈 Remarques
Pour l'instant
logger.warn
&logger.trace
&logger.fatal
&logger.debug
ne sont pas gérées.💯 Pour tester