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

fix(log-messages): reduce excessive log messages #5971

Conversation

ahmeddesokyebrahim
Copy link
Contributor

@ahmeddesokyebrahim ahmeddesokyebrahim commented Dec 26, 2023

Description

This PR is one of group of PRs that aim to fix Autoware logging system to achieve the goal of reducing excessive error and warning logs on Autoware launch.

Part of:

Related links

More details in the issue.

Relevant PRS

Tests performed

  • Planning Simulation
    • Run Autoware planning simulator and you can see no more excessive and recurring error, warning, and info message when the system running as expected
  • Rosbag Replay Simulation
    • To be addressed in another PR
  • AWSIM
    • To be addressed in another PR - extended goal)

Notes for reviewers

Take into consideration relevant PRs.

🔴 Merge Dependency 🔴
❗ This PR is depending on a change in autoware_common which will require the autoware_common PR to be merged firstly to have the build and test differential successful ❗

Interface changes

N.A

Effects on system behavior

  • Improved console output => better developer experience specially for debugging and catching actual problems.
  • Some recurrent info messages are lowered to debug level. So the developer would need to set the logger level of that specific node to see the debug message.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:system System design and integration. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) component:simulation Virtual environment setups and simulations. (auto-assigned) labels Dec 26, 2023
@ahmeddesokyebrahim ahmeddesokyebrahim self-assigned this Dec 26, 2023
@ahmeddesokyebrahim ahmeddesokyebrahim changed the title fix(log-messages): Reduce excessive error and warning logs on Autoware launch fix(log-messages): Reduce excessive log messages in Autoware Dec 26, 2023
@ahmeddesokyebrahim ahmeddesokyebrahim force-pushed the autoware/universe/5539-fix-log-messages branch from 5211e1d to 2827dcb Compare January 13, 2024 00:54
@ahmeddesokyebrahim ahmeddesokyebrahim force-pushed the autoware/universe/5539-fix-log-messages branch 2 times, most recently from d7554cf to 6e84939 Compare January 22, 2024 14:44
@ahmeddesokyebrahim ahmeddesokyebrahim force-pushed the autoware/universe/5539-fix-log-messages branch from 77952a6 to 02b56ea Compare February 2, 2024 11:42
@xmfcx
Copy link
Contributor

xmfcx commented Feb 5, 2024

https://github.com/autowarefoundation/autoware.universe/actions/runs/7756002711/job/21152483741?pr=5971#step:2:23

The PR title and first commit message should start with lowercase letters.

fix(log-messages): reduce excessive log messages

You can drop "in Autoware" as well.

@xmfcx
Copy link
Contributor

xmfcx commented Feb 6, 2024

@ahmeddesokyebrahim can we merge this PR?

@ahmeddesokyebrahim ahmeddesokyebrahim changed the title fix(log-messages): Reduce excessive log messages in Autoware fix(log-messages): reduce excessive log messages Feb 7, 2024
@ahmeddesokyebrahim
Copy link
Contributor Author

https://github.com/autowarefoundation/autoware.universe/actions/runs/7756002711/job/21152483741?pr=5971#step:2:23

The PR title and first commit message should start with lowercase letters.

fix(log-messages): reduce excessive log messages

You can drop "in Autoware" as well.

@xmfcx : Thanks for the comment. This is fixed here and in other PRs as well.

@ahmeddesokyebrahim ahmeddesokyebrahim force-pushed the autoware/universe/5539-fix-log-messages branch 2 times, most recently from 7444c24 to 35d8aff Compare February 7, 2024 01:52
@ahmeddesokyebrahim ahmeddesokyebrahim added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Feb 7, 2024
@ahmeddesokyebrahim ahmeddesokyebrahim marked this pull request as ready for review February 7, 2024 01:58
Ahmed Ebrahim and others added 12 commits February 12, 2024 15:17
…e_report msg is timeout... (isDataHeartbeatTimeout()..."

Signed-off-by: AhmedEbrahim <[email protected]>
…s that appear while starting planning simulator and before setting start location

Signed-off-by: AhmedEbrahim <[email protected]>
…olygon: linestring x must have more than different 3 points! (size is 2). Failed to convert to polygon." and "pedestrian marking x failed conversion."

Signed-off-by: AhmedEbrahim <[email protected]>
…rning messages where it is failed to receive signal and calculate stop point.

Signed-off-by: AhmedEbrahim <[email protected]>
Ahmed Ebrahim and others added 2 commits February 12, 2024 16:12
…icle_state_report msg is timeout... (isDataHeartbeatTimeout()..."

Signed-off-by: AhmedEbrahim <[email protected]>
@ahmeddesokyebrahim ahmeddesokyebrahim force-pushed the autoware/universe/5539-fix-log-messages branch from 9db77c8 to c1bce90 Compare February 12, 2024 14:13
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

Comparison is base (a10f9c4) 14.88% compared to head (c1bce90) 14.88%.

Files Patch % Lines
...et2_map_loader/lanelet2_map_visualization_node.cpp 0.00% 5 Missing ⚠️
...r/src/freespace_planner/freespace_planner_node.cpp 0.00% 2 Missing and 1 partial ⚠️
.../obstacle_avoidance_planner/src/replan_checker.cpp 0.00% 3 Missing ⚠️
planning/path_smoother/src/replan_checker.cpp 0.00% 3 Missing ⚠️
...ion/crosswalk_traffic_light_estimator/src/node.cpp 0.00% 2 Missing ⚠️
...based_prediction/src/map_based_prediction_node.cpp 0.00% 2 Missing ⚠️
...havior_velocity_traffic_light_module/src/scene.cpp 0.00% 2 Missing ⚠️
...nning_simulator/simple_planning_simulator_core.cpp 0.00% 0 Missing and 2 partials ⚠️
...ontrol/control_validator/src/control_validator.cpp 0.00% 1 Missing ⚠️
...nal_controller/src/pid_longitudinal_controller.cpp 66.66% 1 Missing ⚠️
... and 11 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5971      +/-   ##
==========================================
- Coverage   14.88%   14.88%   -0.01%     
==========================================
  Files        1838     1838              
  Lines      126708   126709       +1     
  Branches    38042    38044       +2     
==========================================
  Hits        18865    18865              
- Misses      86544    86545       +1     
  Partials    21299    21299              
Flag Coverage Δ *Carryforward flag
differential 15.93% <5.40%> (?)
total 14.89% <ø> (+<0.01%) ⬆️ Carriedforward from a10f9c4

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ahmeddesokyebrahim
Copy link
Contributor Author

@xmfcx , I see all required actions are passing now.
What is preventing this PR now from being ready for merge ?

@xmfcx xmfcx merged commit 99c8b32 into autowarefoundation:main Feb 12, 2024
31 of 36 checks passed
@xmfcx
Copy link
Contributor

xmfcx commented Feb 12, 2024

What is preventing this PR now from being ready for merge ?

Normally, we would have to get approvals from at least 1 Code Owner from each package.

But I can override it as repo maintainer.

StepTurtle pushed a commit to StepTurtle/autoware.universe that referenced this pull request Feb 28, 2024
0x126 added a commit to tier4/autoware.universe that referenced this pull request Mar 28, 2024
0x126 added a commit to tier4/autoware.universe that referenced this pull request Mar 29, 2024
…5971)

fix(log-messages): reduce excessive log messages (autowarefoundation#5971)

Signed-off-by: AhmedEbrahim <[email protected]>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) component:simulation Virtual environment setups and simulations. (auto-assigned) component:system System design and integration. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants