Swap 3853 #389
Swap 3853 #389
Conversation
| logger.logError(bind + " requires elevated privileges", error); | ||
|
|
There was a problem hiding this comment.
I think we should also include a logger.logeError on line 21
src/routes/zip.ts
Outdated
| if (!hasAccess) { | ||
| logger.logError("Error : ", { statusCode, error }); |
There was a problem hiding this comment.
Remove the spacing. It made sense while I was debugging. Not anymore
src/routes/zip.ts
Outdated
| console.log("Request body : ", req.body); | ||
| const { hasAccess, statusCode, error, directory, fileNames } = hasFileAccess( | ||
| req, | ||
| req.body.directory, | ||
| req.body.files | ||
| ); | ||
| console.log("Has access : ", hasAccess); | ||
| console.log("Status code : ", statusCode); | ||
| console.log("Error : ", error); | ||
| console.log("Directory : ", directory); | ||
| console.log("File names : ", fileNames); |
There was a problem hiding this comment.
Maybe we should include the logs like so to prevent spam while keeping access logs?
Logger.logInfo("File accessed", {
hasAccess: hasAccess,
statusCode: statusCode,
error: error,
directory:directory,
fileNames:fileNames
});
There was a problem hiding this comment.
@martin-trajanovski can we log that a request has been submitted to this endpoint?
Also, in the log, could we make sure that we list all the files/datasets requested and the user that requested them?
| logger.logError(bind + " requires elevated privileges", error); | ||
|
|
src/routes/zip.ts
Outdated
| console.log("Request body : ", req.body); | ||
| const { hasAccess, statusCode, error, directory, fileNames } = hasFileAccess( | ||
| req, | ||
| req.body.directory, | ||
| req.body.files | ||
| ); | ||
| console.log("Has access : ", hasAccess); | ||
| console.log("Status code : ", statusCode); | ||
| console.log("Error : ", error); | ||
| console.log("Directory : ", directory); | ||
| console.log("File names : ", fileNames); |
There was a problem hiding this comment.
@martin-trajanovski can we log that a request has been submitted to this endpoint?
Also, in the log, could we make sure that we list all the files/datasets requested and the user that requested them?
src/routes/zip.ts
Outdated
| if (!hasAccess) { | ||
| logger.logError("Error : ", { statusCode, error }); |
There was a problem hiding this comment.
Remove the spacing. It made sense while I was debugging. Not anymore
|
|
||
| export const router = express.Router(); | ||
|
|
||
| /* POST zip */ | ||
| router.post("/", function(req: express.Request, res: express.Response) { | ||
| router.post("/", function (req: express.Request, res: express.Response) { |
There was a problem hiding this comment.
Can we log the request including the list of files requested?
src/common/config.ts
Outdated
| console.log("Configuration"); | ||
| console.log(config) | ||
|
|
||
| configureLogger( |
There was a problem hiding this comment.
Is this file the correct place for the logger configuration?
I would put it in the app.ts or index.ts
Description
This PR aims to add the configuration for GrayLog logger and do some cleanup
Motivation
There was no logger configured before so we were not able to get any notification if incident happens in this service.
Fixes:
Changes:
Tests included/Docs Updated?