Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Sep 18, 2025

What do these changes do?

This pull request introduces improvements to logging and error handling in both the RabbitMQ and Redis modules.

  • Redis: adds monitoring for lock and semaphore holding times.
    The changes aim to provide more actionable and structured logs for troubleshooting, and to help detect and warn about potential performance bottlenecks related to lock/semaphore usage. Minor test and logging adjustments are also included. It is expected to see some warnings appear when the expected time a lock is held is exceeded. Some further adjustments will be needed to silence the warnings that are unnecessary.
  • Rabbit: use structured logs for connections/channel closing callbacks and unhandled exceptions in message handling. Also handle the ChannelClosed issue during a message similarly as in aio-pika itself.

Lock and Semaphore Monitoring:

  • Added DEFAULT_EXPECTED_LOCK_OVERALL_TIME constant and logic to warn if Redis locks or semaphores are held longer than expected, helping to identify performance issues or code that holds resources too long.

These changes improve observability, make troubleshooting easier, and help proactively detect resource contention issues.

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the Cheops milestone Sep 18, 2025
@sanderegg sanderegg requested a review from Copilot September 18, 2025 09:15
@sanderegg sanderegg self-assigned this Sep 18, 2025
@sanderegg sanderegg added the a:services-library issues on packages/service-libs label Sep 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the RabbitMQ client where exceptions in message handlers could crash the consumer if the channel is closed. The changes improve error handling and add better logging for troubleshooting message processing issues.

  • Wraps the entire message processing logic in a log_catch context to prevent consumer crashes
  • Enhances exception logging with structured troubleshooting information
  • Adds type assertion for connection type validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.79%. Comparing base (8003f66) to head (817588b).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8382      +/-   ##
==========================================
+ Coverage   87.38%   87.79%   +0.40%     
==========================================
  Files        1951     1523     -428     
  Lines       75948    63261   -12687     
  Branches     1337      674     -663     
==========================================
- Hits        66364    55537   -10827     
+ Misses       9185     7489    -1696     
+ Partials      399      235     -164     
Flag Coverage Δ
integrationtests 63.95% <100.00%> (+3.57%) ⬆️
unittests 86.20% <83.78%> (-0.39%) ⬇️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library 72.40% <83.33%> (-0.01%) ⬇️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 84.99% <ø> (ø)
agent 93.53% <ø> (ø)
api_server 91.96% <ø> (ø)
autoscaling 95.78% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.37% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 91.01% <100.00%> (+5.58%) ⬆️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.46% <ø> (ø)
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.62% <ø> (ø)
resource_usage_tracker 92.29% <ø> (+0.05%) ⬆️
storage 86.45% <ø> (-0.09%) ⬇️
webclient ∅ <ø> (∅)
webserver 87.97% <ø> (-0.01%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8003f66...817588b. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Sep 18, 2025

🧪 CI Insights

Here's what we observed from your CI run for 817588b.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI integration-tests Base branch is broken, but retries were needed. Could be early signs of flakiness 👀 Broken 1 View View

@sanderegg sanderegg force-pushed the maintenance/rabbitmq-catch-issue-when-message-raise branch from 16daa68 to d0af8ef Compare September 18, 2025 12:44
@sanderegg sanderegg requested a review from Copilot September 18, 2025 12:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@sanderegg sanderegg requested a review from Copilot September 18, 2025 13:09
@sanderegg sanderegg marked this pull request as ready for review September 18, 2025 13:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@sonarqubecloud
Copy link

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Thanks

@sanderegg sanderegg merged commit af7ba09 into ITISFoundation:master Sep 18, 2025
146 of 148 checks passed
@sanderegg sanderegg deleted the maintenance/rabbitmq-catch-issue-when-message-raise branch September 18, 2025 14:55
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 19, 2025
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:services-library issues on packages/service-libs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants