Skip to content

[feat] Add consumer interceptor - #210

Merged
BewareMyPower merged 12 commits into
apache:mainfrom
RobertIndie:consumer-interceptor
Mar 16, 2023
Merged

[feat] Add consumer interceptor #210
BewareMyPower merged 12 commits into
apache:mainfrom
RobertIndie:consumer-interceptor

Conversation

@RobertIndie

Copy link
Copy Markdown
Member

Master Issue: #150

Motivation

This is the consumer interceptor implementation of #150.

There are some trigger events that are not implemented due to the current PR is big enough:

  • onNegativeAcksSend
  • onAckTimeoutSend
  • onPartitionsChange

I will add them in another PR.

Modifications

  • Add ProducerInterceptor interface
  • Add intercept and getInterceptors in ProducerConfiguration.
  • Triggering the interceptor when beforeConsume, onAcknowledge, onAcknowledgeCumulative and close.

Verifying this change

This change added tests.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@RobertIndie RobertIndie added the enhancement New feature or request label Mar 10, 2023
@RobertIndie RobertIndie added this to the 3.2.0 milestone Mar 10, 2023
@RobertIndie RobertIndie self-assigned this Mar 10, 2023
Comment thread lib/ConsumerInterceptors.cc
Comment thread lib/ConsumerInterceptors.cc Outdated
@BewareMyPower

BewareMyPower commented Mar 16, 2023

Copy link
Copy Markdown
Contributor

I don't think we need to implement the close method, as well as the close method of producer interceptor. I checked the code in Java client, the close method is actually never used. (Please correct me if I was wrong)

@RobertIndie

Copy link
Copy Markdown
Member Author

I don't think we need to implement the close method, as well as the close method of producer interceptor. I checked the code in Java client, the close method is actually never used. (Please correct me if I was wrong)

As we discussed here: #169 (comment), we think it's a missing implementation for the java client. It doesn't make sense to expose the close method but it doesn't do anything

@BewareMyPower
BewareMyPower merged commit 998c73d into apache:main Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants