Skip to content

Conversation

@matthias
Copy link
Contributor

@matthias matthias commented Jul 3, 2023

  • use logger.xxx instead of console.xxx
  • added express middleware logging (using jsonl)
  • added LOG_PATH as environment variable
  • more configs postponed for later iteration

- use logger.xxx instead of console.xxx
- added express middleware logging (using jsonl)
- added LOG_PATH as environment variable
- more configs postponed for later iteration
@matthias matthias marked this pull request as ready for review July 3, 2023 15:59
@matthias
Copy link
Contributor Author

matthias commented Jul 3, 2023

Created a new clean merge

Copy link
Contributor Author

@matthias matthias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you also want to respect the DEBUG setting.

But not sure if the DEBUG flag is the right one to enable / disable request logging at all. Even in a production environment you want HTTP request logging.

I would suggest to move this into the logger.ts / expressRequestLogger -> so it can be configured via config.

@HenryHengZJ
Copy link
Contributor

good point @matthias do you reckon we should add another env var like ENABLE_LOGGING? This allow user to have flexibiltity to turn on/off loggers if they dont want to burden the system with logs

@matthias
Copy link
Contributor Author

matthias commented Jul 6, 2023

If the status is fine for you - I would go ahead and also apply this to /components

What's the best way (git wise) to do this. Create a sub-branch from this status / create a new branch / keep on working on this branch? Thanks for your advice here.

And: The main goal behind introducing winston is to get better chat / chain logging. Any ideas in this direction I should be aware off? Otherwise I would try and come up with an approach.

/logs was in /packages -> moved it to project root (one level up)
@HenryHengZJ
Copy link
Contributor

You can create another branch base from this branch.

Are you trying to log the output of each component? I was trying to see if we can add the logs output from langchain when the DEBUG is true, as currently they are just printed on the terminal/console

@HenryHengZJ
Copy link
Contributor

@matthias I reverted the DEBUG changes and allow logger by default. Let me know if you are still working on this, if not we can merge this and open another PR for the logger on components

@matthias
Copy link
Contributor Author

Yes you can merge it ... logger on compnents / flows should be another PR imo.

@HenryHengZJ HenryHengZJ merged commit 2bcc2f9 into FlowiseAI:main Jul 10, 2023
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

Successfully merging this pull request may close these issues.

2 participants