-
Notifications
You must be signed in to change notification settings - Fork 3
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(service): enable health-check for servicebus #1338
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs (1)
195-195
: LGTM! Health check configuration is properly implemented.The change to use "dependencies" tag for ServiceBus health check aligns well with the existing health check strategy, where Redis and Postgres checks are similarly tagged. This provides consistent categorization of external dependency health checks.
Consider adding a comment explaining the health check tag strategy to help future maintainers understand the categorization approach.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs (2)
Line range hint
205-211
: Environment-specific ServiceBus configuration is well structured.The code cleanly handles the transition between development (in-memory) and production (Azure ServiceBus) configurations, making local development and testing easier while maintaining production reliability.
Line range hint
196-204
: ServiceBus endpoint configuration looks good but verify error handling.The addition of dead letter queue configuration enhances message reliability. However, it's important to verify the error handling behavior.
Let's verify if there are any existing error handlers that might conflict with the dead letter queue configuration:
✅ Verification successful
Let's examine the specific error handling configurations in these files to ensure there are no conflicts.
Dead letter queue configuration complements existing retry mechanisms
The verification shows that the dead letter queue configuration works harmoniously with the existing error handling setup:
- The
DomainEventConsumer
implements a tiered retry strategy:
- Immediate retries with increasing intervals (100ms to 1000ms)
- Delayed redelivery attempts (1min to 15min)
- The new dead letter queue configuration in
InfrastructureExtensions.cs
serves as the final fallback after retries are exhaustedThis is a well-structured error handling approach that provides multiple recovery opportunities before messages are moved to the dead letter queue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing error handling configurations rg -l "ConfigureDeadLetterQueue|UseMessageRetry|UseExceptionHandler"Length of output: 216
Script:
#!/bin/bash # Check the content of files with error handling configurations echo "=== InfrastructureExtensions.cs ===" rg -A 5 -B 5 "ConfigureDeadLetterQueue|UseMessageRetry|UseExceptionHandler" src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs echo -e "\n=== DomainEventConsumer.cs ===" rg -A 5 -B 5 "ConfigureDeadLetterQueue|UseMessageRetry|UseExceptionHandler" src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.csLength of output: 1568
🤖 I have created a release *beep* *boop* --- ## [1.26.3](v1.26.2...v1.26.3) (2024-10-23) ### Bug Fixes * Fix XACML attribute id for system users ([#1340](#1340)) ([4257729](4257729)) * **service:** enable health-check for servicebus ([#1338](#1338)) ([480f5e3](480f5e3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Related Issue(s)
Verification
Documentation
docs
-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
Bug Fixes