mia-platform / custom-plugin-lib

Library that allows you to define Mia-Platform custom plugins easily
https://www.mia-platform.eu/
Apache License 2.0
29 stars 8 forks source link

Default logs in TRACE or DEBUG level #120

Open theliuk opened 3 years ago

theliuk commented 3 years ago

The description of the bug or the rationale of your proposal

It's quite common for developers to forget to implement some logs which are essentials to debug a service behavior and the integration with external systems. Some of this logs are always the same, and the developer should take them for granted, so that she can focus on logs for her specific domain. Here is the proposal for some of this default (or boilerplate) logs:

  1. For each route defined by means of addRawCustomPlugin, the following logs should be set: a. a TRACE/DEBUG log of the entire request with headers and payload b. a TRACE/DEBUG log of the entire response with headers and payload

Sensitive information should be take into account in order not to accidentally log it. A simple solution for this problem could consist of a simple flag opt-out boolean flag to be set in the json schema of the endpoint for all the fields that should be omitted from the logs (like all the personal information Here's an example of such a JSON schema:

{

  "headers": {
    "type": "object",
    "properties": {
        "authorization": {"type":"string", "hide": true"}
     }
   },
  "body": {
    "type": "object",
    "properties": {
       "name": {"type": "string"},
       "surname": {"type": "string"},
       "taxCode": {"type": "string", "hide": true"}
    },
  },
  "response": {
      "200": {
        "type": "object",
       "properties": {
            "medicalRecordID": {"type": "string", "hide": "true"}
        }
      }
   }
  1. For each call that is directed to an external service by means of a service proxy: a. a TRACE/DEBUG log with all the request that is forwarded to the external service b. a TRACE/DEBUG log with all the response that is received from the external service

With these default/boilerplate logs, the developer should not remember to add noisy logs to her code, and she would know that it only takes to set the right LOG_LEVEL environment variable to have all the information she needs to debug a problem.

davidebianchi commented 3 years ago

There are already some default logs in the lib about request and response.

We could add logs for header and body in trace level.

For the header, we could leave the user to write the custom headers to hide (and by default we could hide the authorization and cookie headers for example). A possible configuration could be as you show in the json schema.

For the body, it's quite difficult to hide some custom values because the request and response body could be in some other format than json, or if in JSON some sensitive information could be very nested. So this could bring to unexpected leak of sensitive information in logs, also if in trace.

theliuk commented 3 years ago

The problem of nested sensitive data should be a problem; pino logger, for instance, allows the user to define complex paths to prevent information to be logged: log redaction

davidebianchi commented 3 years ago

Yes, so adding defined json path (generally without wildcards to avoid performance reduction). Pino uses https://github.com/davidmarkclements/fast-redact. It is possible, but redact should be defined per logger, not per log. So, or it is created a specific logger per use case or it is used fast-redact in some ways I suppose.

theliuk commented 3 years ago

I was thinking about a log.child for each endpoint, each child with its own serialization options. Also, in my opinion, wildcards may still be used, as they should be evaluated only in the case a log is actually sent to the out stream. So, performance degradation, if it happens, should happen when the log level of the service is low (TRACE or DEBUG). We should investigate this hypothesis of mine.

theliuk commented 3 years ago

Gently ping

davidebianchi commented 3 years ago

The feature is interesting, it could be interesting to try to implement it. @theliuk can you open a PR?

theliuk commented 3 years ago

:( no free lunch theorem :(

davidebianchi commented 2 years ago

Added log in trace and error during the call. It could be activated passing the logger to the new http client function