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

Default logs in TRACE or DEBUG level #120

Open
theliuk opened this issue Jun 8, 2021 · 8 comments
Open

Default logs in TRACE or DEBUG level #120

theliuk opened this issue Jun 8, 2021 · 8 comments

Comments

@theliuk
Copy link

theliuk commented Jun 8, 2021

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
Copy link
Member

davidebianchi commented Jun 8, 2021

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
Copy link
Author

theliuk commented Jun 8, 2021

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
Copy link
Member

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
Copy link
Author

theliuk commented Jun 9, 2021

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
Copy link
Author

theliuk commented Jun 16, 2021

Gently ping

@davidebianchi
Copy link
Member

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

@theliuk
Copy link
Author

theliuk commented Jun 24, 2021

:( no free lunch theorem :(

@davidebianchi
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants