Add Dead Letter Queue and update existing queue configurations to use it

PaulKBaumann commented 1 month ago

What was the problem?

Messages that fail to process are lost.

Associated tickets or Slack threads:

https://app.zenhub.com/workspaces/vro-team-6557e67173391c000e1409f3/issues/gh/department-of-veterans-affairs/abd-vro/2758

How does this fix it?[^1]

When bip-api starts up and creates/configures queues, it will now also create a dead-letter queue. Messages that fail to process will be placed on this queue

How to test this PR

Build and launch the bip-api service in Docker
Go to the RabbitMQ console and verify that the dead-letter queue was created
Place some junk data on any of the queues. bip-api will fail to process it and you can see the message end up in the dead-letter queue.
after implementing it, we discovered that coordinating changes for EPMerge and BIP are needed
When we deploy this it will need to be planned to ensure we do not take down the EPMerge and BIP

[^1]: Pull-Requests guidelines. If PR is significant, update Current Software State wiki page. [^secrel]: To check if a PR will succeed in the SecRel workflow, test PRs in the SecRel pipeline.

github-actions[bot] commented 1 month ago

Test Results

118 tests ±0 118 :white_check_mark: ±0 22s :stopwatch: +3s 34 suites ±0 0 :zzz: ±0 34 files ±0 0 :x: ±0

Results for commit d7dcc061. ± Comparison against base commit edd1144b.

:recycle: This comment has been updated with latest results.

lisac commented 1 month ago

I don't why the CI checks haven't completed. Triggered a run manually, running in https://github.com/department-of-veterans-affairs/abd-vro/actions/runs/10115124790 .

lisac commented 1 month ago

the lint check is now passing - ran it manually here: https://github.com/department-of-veterans-affairs/abd-vro/actions/runs/10115460344

@PaulKBaumann : as a next step, do you mind updating your branch with what's on the default branch? I'm curious about the conflict that GH is flagging around MessageQueueConfiguration.java. Thanks!

tangent: I don't know why the CI checks appear to be stuck. My hunch: our tooling is getting confused by the # in the branch name, or we need to refine the filters in https://github.com/department-of-veterans-affairs/abd-vro/blob/ec7090930cf5d15b42f194717d64dc4c9c2f8d5b/.github/workflows/continuous-integration.yml#L7. The latter is on my mind after reading this. I think the fix is out of scope for this PR.

nelsestu commented 1 month ago

The main thing I want to encourage in this PR is that we modify the message queue configuration loading to support missing configuration settings. One problem that we frequently encounter with our services is that we fail to make them resilient to misconfiguration or missing configuration. Have a look at RabbitMQ Config- startup configuration will be difficult to diagnose without more graceful handling. For example: Add error handling or logging for the creation of the dead letter queue and its bindings. Consider adding validation or setting default values where config wouldn't actually be needed.

PaulKBaumann commented 1 month ago

@PaulKBaumann : as a next step, do you mind updating your branch with what's on the default branch? I'm curious about the conflict that GH is flagging around MessageQueueConfiguration.java. Thanks!

It's merged now. It was the Camel library removal changes that were conflicting.

github-actions[bot] commented 1 month ago

JaCoCo Test Coverage

Overall Project	71.79% `-2.25%`	:x:
Files changed	0%	:x:

File	Coverage
MessageQueueConfiguration.java	0% `-24.73%`	:x:

PaulKBaumann commented 1 month ago

I added some default values for the RabbitMQ properties and logged the relevant values when creating the dead letter queue and binding.

department-of-veterans-affairs / abd-vro