The "Publish" action will fail sometimes due to the issue I mentioned. (See Line #38 in PublishMessageEndpoint and #107 in MessageOperator) . In addition, if the action is failed, the target message WILL BE REMOVED and users will LOSE DATA FOREVER.

Root Cause:

Reasons for "Publishing" fail :

• The messageCache doesn't contain the fingerprint information since the request may be handled by different instances.

Reasons for data loss:

• Virgil ack & remove the message before publishing the message, but not take the "REVERT" if the publishing behavior failed.

Solution:
For issue #1, let's call the function "getMessages()" again if messageCache doesn't contain the target fingerprint.

For issue #2, let's switch the process in Publish API to do "publish target message" before "ack & remove target message"

Currently, it seems that Virgil closes the connection to make "unacked" messages be back to the "READY" state at the end of "getMessage" & "ackCertainMessage" function. If any exception is thrown during the process, the message "consumed" by Virgil will become "unacked" and never recovered back. It would be helpful to make Virgil close the connection anyway as the last step. (even though any exception is thrown)

Solution options:

1. Add try-catch for the mentioned functions
2. Move the destroy call to the finally function

Sometimes, when you click the "Republish" button on one specific message, the request will fail even though the message is correct and publishable (they can be re-published successfully with some attempt).

With some local debugging, it seems that the problem comes from the method ackCertainMessage(). During the process of this method, sometimes the target message may become "unacked" when it calls the function "getQueueSize()". In this case, the method will FAIL to ack the target message anymore, which leads to the FAILURE response.

To fix the issue, I suggest to re-design the method "ackCertainMessage()" to make sure that messages WILL NOT BE ACKED unexpected.

I think it would be a useful feature to add a search that allowed users to search for messages that contained certain header/value combos or text within body.

This may have some performance implications if the DLQ is too large, so it may need to get truncated if any issues.

Sometimes, the clients require to NOT initialize the Virgil configurations due to their specific usages (like integration tests).
Making clients enable/disable Virgil's initialization in a flexible way would be really helpful for all users.

Solution:
just a one-line change on the file VirgilAutoConfiguration.java

Please change the Class to Public

Within the MessageOperator, purged queues can return null response. We should add a null check for this.

https://github.com/indeedeng/virgil-spring-boot-starter/blob/master/src/main/java/com/indeed/virgil/spring/boot/starter/services/MessageOperator.java#L165

This for-loop could be optimized to stop processing messages once we have ack'd the correct message.

We should depend on the runtime for the reactor library to allow folks to decide whether to use jetty, netty, or (another library) for reactor.

This is causing some of our users to have to exclude our reactor-netty library

Currently Virgil uses its own incantation of @ConfigurationProperties (RabbitSettings) to load the Rabbit configuration. This does not support addresses that are perfectly valid in Spring Boot Rabbit auto-configuration. The end result is that a SpringBoot app using standard rabbit addresses is not able to use those same properties to configure Virgil.

Namely Virgil can not handle an addresses field prefixed w/ "amqp://" nor one that does not specify a port (aka uses the default port). All that Virgil does is send the info down into Rabbit - so really its Rabbit that is not lenient. However, SpringBoot has already solved this with its RabbitProperties config props class that it uses.

Virgil should support the same configuration options that Spring Boot supports.

Acceptance Criteria:

• Standard Spring Boot supported rabbit addresses property can be used.

Without a limit being passed into /virgil/get-dlq-message, it fails to return messages to the dashboard.

http://localhost:8081/private/virgil/get-dlq-messages?limit=1 will succeed while http://localhost:8081/private/virgil/get-dlq-messages fails.

Acceptance Criteria:

• Frontend code should pass a default limit 200
• If no limit is received at endpoint, a default limit of 200 should be used.

Notes:
In the future, I can see this being a configurable value on the frontend similiar to how you can change the results on a page during pagination.

republish and drop button don't work, while the drop all button works. Please help to fix it. thanks

The DefaultMessageConverter uses the ObjectMapper to generate the md5 fingerprint of the target RabbitMQ message. Therefore, all fields inside the target message (spring-amqp) should be Serializable.

This message class contains a field MessageProperties, which contains an attribute, "headers" in <String, Object> type.

If there is any exception stored in the message, the attribute header will store the stack trace messages as one Map Entry "stacktrace"(String) -> "details...." (ByteArrayLongString), in which case that "ByteArrayLongString" is Non-Serializable.

In this case, the ObjectMapper will fail to generate md5 fingerprint and throws an exception like :

No serializer found for class java.io.DataInputStream and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) (through reference chain: org.springframework.amqp.core.Message[\"messageProperties\"]->org.springframework.amqp.core.MessageProperties[\"headers\"]->java.util.HashMap[\"x-exception-stacktrace\"]->com.rabbitmq.client.impl.LongStringHelper$ByteArrayLongString[\"stream\"])","path":"/private/virgil/get-dlq-messages"}

Solution:
To solve this issue, we should configure the ObjectMapper as SerializationFeature.FAIL_ON_EMPTY_BEANS = false, which will ignore all Non-Serializable fields during the process.

The default branch name for repositories created on GitHub is now main. See github guidance

In the UI, the list of messages in the DLQ once a message has been dropped or republished, all following messages' actions cannot be issued successfully. Only the message that are displayed before the acted upon message can be successfully acted upon.

Hello,

The OSSmetadata file has the shield notation as "experimental". Is this the correct notation, or should it be updated to "active"?

With the current implementation of Virgil, it's possible that one message will be lost when the service tries to republish it. According to the comment, if the process crashed in between, or ack success but publish failed, the target message will be missed.

It's very dangerous that any message will be missed unexpectedly during the republishing processing.

Solution:
Please optimize the function to NOT remove the message from Dead Letter Queue until the republishment success.

The frontend UI is passing the parameter as fingerprint but the backend controllers are expecting a messageId named parameter. The mismatch is causing a 400.

{"timestamp":"2021-02-27T03:28:30.525+00:00","status":400,"error":"Bad Request","message":"","path":"/actuator/private/virgil/drop-message"}

Currently, Virgil is limited to supporting a single DLQ. A user would want to manage multiple DLQs from the same interface without having to create multiple interfaces.

Potential Configuration changes to consider

Multiple Queues with Single Binder

virgil:
  queues:
    dlq1:
      readName: 'dlq1'
      readBinderName: binder1
      republishName: 'originalQueue1'
      republishBinderName: binder1
      republishBindingRoutingKey: 'key1'
    dlq2:
      readName: 'dlq2'
      readBinderName: binder1
      republishName: 'originalQueue2'
      republishBinderName: binder1
      republishBindingRoutingKey: 'key2'
  binders:
    binder1:
      name: 'exchange'
      type: rabbit
      rabbitProperties:
        addresses: 
        username: 
        password: 
        virtual-host: 

Multiple Queues with Multiple Binders

virgil:
  queues:
    dlq1:
      readName: 'dlq1'
      readBinderName: binder1
      republishName: 'originalQueue1'
      republishBinderName: binder1
      republishBindingRoutingKey: 'key1'
    dlq2:
      readName: 'dlq2'
      readBinderName: binder2
      republishName: 'originalQueue2'
      republishBinderName: binder2
      republishBindingRoutingKey: 'key2'
  binders:
    binder1:
      name: 'exchange1'
      type: rabbit
      rabbitProperties:
        addresses: ${secret.store.addresses}
        username: ${secret.store.username}
        password: ${secret.store.password}
        virtual-host: ${secret.store.virtual-host}
    binder2:
      name: 'exchange2'
      type: rabbit
      rabbitProperties:
        addresses: ${secret.store.addresses}
        username: ${secret.store.username}
        password: ${secret.store.password}
        virtual-host: ${secret.store.virtual-host}

Acceptance Criteria:

• Modify config to support multiple queues and multiple binders
• UI would need to be able to have a way of swapping between each DLQ