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

Log all server requests #2994

Closed
mattwiller opened this issue Oct 6, 2023 · 7 comments
Closed

Log all server requests #2994

mattwiller opened this issue Oct 6, 2023 · 7 comments
Labels
audit-logging Commits related to audit logging and observability good first issue Good for newcomers

Comments

@mattwiller
Copy link
Member

To improve observability of Medplum server, we should log all HTTP requests received and processed. This would likely need to be added as a middleware during server setup. The middleware should log at least the following data for each request handled:

  • Timestamp the request was received (not the timestamp when the log was written)
  • HTTP request method and path
  • User profile reference, or none if not authenticated
  • Client IP address
  • Time taken to process the request (from when it was first processed to when the response is rendered)
  • Status code returned
  • Number of bytes sent in the response to the client
  • User-Agent string
@mattwiller mattwiller added good first issue Good for newcomers audit-logging Commits related to audit logging and observability labels Oct 6, 2023
@codyebberson
Copy link
Member

This should definitely be configurable. In production systems, you get all of this for free through ALB logging. See: https://www.medplum.com/docs/self-hosting/aws-athena-guide

Are there any standards/conventions that we can align with, especially thinking in context of this discussion #2759 ?

@Dharshan-K
Copy link
Contributor

Hi, @mattwiller, I am interested in this issue to build a logging service.

@mattwiller
Copy link
Member Author

Hey @Dharshan-K, please feel free to get started with this one! I see you've already taken a look at the broader Observability plan — this is a great piece of that plan for you to contribute. We're trying to take an iterative, piece-by-piece approach to adding observability features to Medplum server, so for this issue I have a couple recommendations for your implementation:

  • As @codyebberson pointed out, this logging should be configurable: one should be able to turn off the access log if it's too verbose or redundant
  • For now, prefer a simple implementation of the access log described above over broader changes to the server logging: it'll be easier to merge your PR if it's more focused and narrow in scope

@vaibhavpnimkar
Copy link

Hello @mattwiller if no one is working on this i would like take this issue

@mattwiller
Copy link
Member Author

Hey @vaibhavpnimkar — I believe @Dharshan-K was planning to try working on this issue, but if they aren't still interested then it's all yours

@Dharshan-K
Copy link
Contributor

Hi @mattwiller I have written the logging middleware, which logs the necessary data mentioned in this issue. How to calculate the bytes sent and get the user reference. below is the code.

const loggingMiddleware = (req: Request, res: Response, next: NextFunction) =>{
  const start = new Date();  
  next();
  const afterNext = new Date().getTime(); // Record the time after next() completes
  const totalTime = afterNext - start.getTime(); // Calculate the time taken including the time spent in next()  
  globalLogger.log('INFO', 'logging data', {requestReceivedTimestamp:start,requestMethodAndPath: `${req.method} ${req.path}`,timeTaken: `${totalTime} ms`, IP_address: req.ip, status: res.statusCode, userAgentString: req.get('User-Agent')});
}

I welcome your feedback. Thank you

@reshmakh reshmakh added this to the Milestone Quality milestone Oct 25, 2023
@rahul1
Copy link
Member

rahul1 commented Oct 31, 2023

@Dharshan-K - are you interested in submitting a PR for this issue? If so, we can assign it to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-logging Commits related to audit logging and observability good first issue Good for newcomers
Projects
Status: ✅ Done
Development

No branches or pull requests

6 participants