Skip to content

[feat] Add producer interceptor - #169

Merged
RobertIndie merged 13 commits into
apache:mainfrom
RobertIndie:interceptors
Feb 20, 2023
Merged

[feat] Add producer interceptor#169
RobertIndie merged 13 commits into
apache:mainfrom
RobertIndie:interceptors

Conversation

@RobertIndie

@RobertIndie RobertIndie commented Jan 6, 2023

Copy link
Copy Markdown
Member

Master Issue: #150

Motivation

This is the producer interceptor implementation of #150.

Modifications

  • Add ProducerInterceptor interface
  • Add intercept and getInterceptors in ProducerConfiguration.
  • Triggering the interceptor when beforeSend, onSendAcknowledgement, onPartitionsChange and close.

Verifying this change

  • Make sure that the change passes the CI checks.

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 Jan 6, 2023
@RobertIndie RobertIndie added this to the 3.2.0 milestone Jan 6, 2023
@RobertIndie RobertIndie self-assigned this Jan 6, 2023
Comment thread include/pulsar/ProducerConfiguration.h Outdated
Comment thread include/pulsar/ProducerInterceptor.h Outdated
Comment thread lib/PartitionedProducerImpl.cc Outdated
Comment thread lib/ProducerImpl.cc Outdated

@BewareMyPower BewareMyPower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests should check what latch.wait returns and the side effects made by the different interceptors.

Comment thread tests/InterceptorsTest.cc Outdated
Comment thread include/pulsar/Producer.h Outdated
Comment thread include/pulsar/ProducerConfiguration.h Outdated
Comment thread include/pulsar/ProducerConfiguration.h Outdated
Comment thread include/pulsar/ProducerInterceptor.h
Comment thread include/pulsar/ProducerInterceptor.h Outdated
Comment thread include/pulsar/Producer.h Outdated
* Use param test for InterceptorsTest
* Check latch
* Add tests for partitioned topic
* Improve dependency of Producer.h
* Improve interceptors insertion in ProducerConfiguration
* Add close interceptors calls
@BewareMyPower

Copy link
Copy Markdown
Contributor

Could you solve the conflicts?

@RobertIndie

Copy link
Copy Markdown
Member Author

Could you solve the conflicts?

Done. PTAL again. Thanks.

Comment thread lib/ProducerInterceptors.h
Comment thread lib/ProducerImplBase.h

@BewareMyPower BewareMyPower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left two comments about the coding style. These should not be blockers so I dismissed my requested changes before.

@BewareMyPower

Copy link
Copy Markdown
Contributor

@RobertIndie The main branch is broken due to a recent upgrade of macos-12 runner image. The fix is here: #199

@RobertIndie
RobertIndie merged commit 86c915b into apache:main Feb 20, 2023
@RobertIndie RobertIndie mentioned this pull request Mar 16, 2023
4 tasks
@Anonymitaet

Anonymitaet commented Apr 17, 2023

Copy link
Copy Markdown
Member

Hi @RobertIndie, thanks for introducing this great feature!

As we discussed just now, for the doc side:

✅ Pulsar feature matrix

I've ticked the boxes of "Interceptors" for C++ producer and consumer.

✅ Pulsar client docs

Could you please provide the following tech inputs? TIA!

The following docs are only added to NEXT since this feature is server-independent.

image

image

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.

4 participants