Add dltHandlerMethod field to @RetryableTopic annotation matching RetryTopicConfigurationBuilder

[JooHyukKim]

Motivation

Currently, there is no way to configure dltHandlerMethod via @RetryableTopic even though @RetryableTopic is sort of the method/class level version of RetryTopicConfigurationBuilder.
I am not sure if the dltHandlerMethod configuration was left out intentionally or not, but it would help my use case below and hopefully others' too.

Use Case

In my application, there are many consumer beans configured as below.

@Component
class MyFirstConsumer(
    private val someService: SomeService,
    private val dltHandler: CustomDeadLetterTopicHandler,
) {

    @RetryableTopicPreset(
        retryTopicSuffix = ".first-consumer.retry",
        dltTopicSuffix = ".first-consumer.dlt",
    )
    @KafkaListenerPreset(
        id = "first-consumer-id",
        topics = ["my-first-topic"],
        groupId = "my-first-consumer-group",
    )
    fun onMessage(
        @Header(KafkaHeaders.RECEIVED_TOPIC) topicName: String,
        message: CustomMessage,
    ) {
        someService.doSomething(message)
    }

    @DltHandler
    fun handleDlt(
        @Header(KafkaHeaders.RECEIVED_TOPIC) topic: String,
        payload: String,
    ) {
        dltHandler.handleDlt(topic, payload)
    }
}

Key points are....

1. We are using the two annotations, @RetryableTopicPreset and @KafkaListenerPreset, that respectively extend the @RetryableTopic and @KafkaListener annotations. This approach minimizes boilerplate configuration values by pre-setting commonly used configurations within these annotations.
2. @DltHandler method is implemented exactly the same in all consumers. This because we take human-intervention approach to fix whatever led a message to DLT.
3. As to why we do not declaring global RetryTopicConfiguration bean, we often find the need to change configurations per usage, per case or classes and these preset annotations allow us to easily configure things. RetryTopicConfiguration is too global.

Note

• I will start writing documentation(.adoc) when the PR reaches positive agreement state.

Thanks

[artembilan]

Well, thanks for looking into this!

I wonder if, according to the annotation-driven model, it would be more natural to have something like this instead:

@DltHandler
public void handleFailed(Object message) {
}

in the same class where we have that @RetryableTopic.

See similar model with @Retryable and @Recover in Spring Retry.

[JooHyukKim]

I guess it can be subjective as to what natural is 🤔.

[JooHyukKim]

Looking into how @RetryableTopic is implemented, @RetryableTopic is really is just annotation-representation of RetryTopicConfigurationBuilder which gets constructed using the RetryTopicConfigurationBuilder itself during startup.

This is why it also sort of felt natural (unfortunately 😅) to make current proprosal (PR). We are trying to resolve this asymmetry between the two approaches.

• With RetryTopicConfiguration, we only need a RetryTopicConfigurationBuilder to configure both in one go, builder-style
• With @RetryableTopic, we are restricted to same-class, same method.

[JooHyukKim]

But still, maintainer's design perspective also counts alot, so lemme know what you think, @artembilan! ✌🏼✌🏼

[artembilan]

As you said before, the RetryTopicConfiguration is global. The @RetryableTopic is for specific service, therefore it feels logical that DLT handler must be that service-specific as well.
So, when you encapsulate logic for your @RetryableTopic, you definitely might go the way to encapsulate DLT over there as well.
Therefore, in most cases, your new annotation would point to the same service and method in there.
Let's just stick with best practice how to design this kind of services!

[JooHyukKim]

Hmmm. yes I said the RetryTopicConfiguration is global. At the same time, I also believe that @RetryableTopic being for specific service is of capability/choice, not as restriction? That much of freedom feels reasonable knowing that DLT can already be configured via RetryTopicConfigurationBuilder.

And as per encapsulation of DLT, we have RetryTopicConfigurationBuilder which already allows configuration of DLT, meaning retry and DLT's can be associated?

This PR is not introducing a new dependency between two isolated concepts -between Retry and DLT, but already existing capability of one-go-configuration via RetryTopicConfiguration.

[artembilan]

Well, again: with an annotation we already have a scope of the method where it is declared, so comparing RetryTopicConfigurationBuilder and annotation configuration is not really one-to-one.
Therefore, if we go the way of annotation DLT, then I'd prefer to have it in a scope where its @RetryableTopic counterpart is declared.
Plus we already have a positive experience with the patter we have in Spring Retry.
Or even see here @KafkaHandler and its isDefault() attribute.
Another example is a @RestController with its @..Mapping and @ExceptionHandler.

So, the general point is: if we have a service and give some its method a specific @KafkaListener aspect, the other related aspects must be declared in this service as well.

[JooHyukKim]

Thank you for the insightful explanation🙏🏽🙏🏽

Took me a while to realize the differences between the annotation and builder way, and how it all fits overall design of Spring libraries.

It also makes sense to consider the positive feedback for the current state implementation👍🏼. Thanks again @artembilan!