search

MOV-AI / movai-core-shared

shared repository for the core elements
Other
1 stars 2 forks source link

Changed default verbosity level to INFO #157

Closed VicenteMovai closed 5 months ago

VicenteMovai commented 5 months ago

There were a lot of DEBUG logs about Creating tcp connection (redis-master) , etc..

:warning: Beware that Robotics and CSAI nodes that depend on DEBUG logs will be impacted

guide-bot[bot] commented 5 months ago

Thanks for opening this Pull Request! We need you to:

  1. Complete the activities.

    Action: Complete Create docs on how to easily change the verbosity level to see the DEBUG again

    If an activity is not applicable, use '\~activity description\~' to mark it not applicable.

duartecoelhomovai commented 5 months ago

We fixed the duplication of logs

brunomovai commented 5 months ago

Should this be being handled like this ? just desabling a platform feature ( debug logs)

We have debug logs in a lot of our Nodes, with parameters to enable/disable them on demand. Lossing all of that work , because the platform is not able to handle it's logs perpery doesn't seem the correct way to go.

And having a workadound to be able to get my logs, and then take the platform garbage again , is not a proper way to have this.

Not to mention that taking all those logs my change sginificantly the performance level; to the point that we are not able to have a comparable product in deployment.

@VicenteMovai @AlexFernandes-MOVAI @DanielGoncalves-MOVAI @cptn-ahab

duartecoelhomovai commented 5 months ago

Tests failing with:

FAILED tests/test_logger.py::test_logger - UnboundLocalError: local variable

resolved

duartecoelhomovai commented 5 months ago

Should this be being handled like this ? just desabling a platform feature ( debug logs)

We have debug logs in a lot of our Nodes, with parameters to enable/disable them on demand. Lossing all of that work , because the platform is not able to handle it's logs perpery doesn't seem the correct way to go.

And having a workadound to be able to get my logs, and then take the platform garbage again , is not a proper way to have this.

Not to mention that taking all those logs my change sginificantly the performance level; to the point that we are not able to have a comparable product in deployment.

* Run system on client without debug's - Runs a certain way

* Run the sysmte on the client wtih debug ( because need to debug 1 node) - runs differently

@VicenteMovai @AlexFernandes-MOVAI @DanielGoncalves-MOVAI @cptn-ahab

I believe you jumped to fast out of your seat xD What we did is changing the default logs on the whole system to INFO. Since this is a product that goes to clients (including you) i don't believe its acceptable to have the default as DEBUG. Even INFO is questionable to be honest but its just a first step. This means the DEFAULT level will be set to a less verbose logging set, and then be able to be customized on the start of the robots to adapt to the running needs. Another thing that is important to mention is that there are logger levels for different use cases. Logging level for call backs, for the platform, etc.. All them should be configurable independently. We will provide an image on how to do it. If you guys have more questions let me know. @cptn-ahab @brunomovai

brunomovai commented 5 months ago

@duartecoelhomovai

Then maybe i'm misunderstanding the scope and description of this PR:

1 - Debug Log in a python code from a callaback Current behaviour : Shows in the terminal as a debug log 2 - Info Log in a python code from a callaback Current behaviour : Shows in the terminal as a info log 3 - Debug Log from the platform ( e.g. creating tcp connection) Current behaviour : Shows in the terminal as a debug log 4 - Info log from the platform ( e.g. shutting down node) Current behaviour : Shows in the terminal as a info log

What would be the behaviour after this PR for the 4 cases ?

This means the DEFAULT level will be set to a less verbose logging set, and then be able to be customized on the start of the robots to adapt to the running needs. via terminal ? something on the range of e.g. "movai-cli start robot_a debug on"

Another thing that is important to mention is that there are logger levels for different use cases. Logging level for call backs, for the platform, etc.. All them should be configurable independently. We will provide an image on how to do it.

I should not have the need to touch something like that, unless there's something very wrong that requires an in-depth debug though the infrastruture. Otherwise, the default of the platform should be the one that coves well the needs ot ir's clients/users ( developers, integrators, deployers, and operators )

cptn-ahab commented 5 months ago

@duartecoelhomovai Currently as a user of the system I have 2 types of logs

Question Does this PR auto publish all debug logs as info logs or does it crop debug logs so they do not show anywhere?

cptn-ahab commented 5 months ago

To add to what @brunomovai said, it seems you're solving the problem just technically and not practically. Or at least not with the user in mind

duartecoelhomovai commented 5 months ago

Kept call back execution level in debug. Platform verbosity was set to INFO

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Issues
48 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud