feat(pubsub): support kafka #7032

bzp2010 · 2022-05-12T03:11:31Z

Description

Provide kafka implementation on pubsub framework.

Split from #6995

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

bzp2010 · 2022-05-12T20:21:25Z

Update

There are still some test cases that have not been debug passed yet.

membphis

LGTM

membphis · 2022-05-15T12:45:04Z

t/admin/upstream5.t

+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';


t/admin/upstream5.t we need a meaningful name

@membphis This is just because the previous upstreamX test contains too many lines, and it does not point to the upstream scheme test of "Kafka", future upstream-related tests can also be added to this file (this test was moved here from t/node), which only tests whether the upstream in the Admin API can be created properly.

spacewander · 2022-05-15T13:19:10Z

apisix/pubsub/kafka.lua

+-- string to int64 cdata numbers.
+local function pb_convert_to_int64(src)
+    if type(src) == "string" then
+        return C.atoll(ffi.cast("char *", src) + 1)


Let's check src length to avoid out of bound

spacewander · 2022-05-15T13:20:09Z

docs/en/latest/pubsub.md

@@ -42,6 +42,10 @@ In Apache APISIX, the most common scenario is handling north-south traffic from

 Currently, Apache APISIX supports WebSocket communication with the client, which can be any application that supports WebSocket, with Protocol Buffer as the serialization mechanism, see the [protocol definition](../../../apisix/pubsub.proto).


Let's update the path of definition. We should use absolute path as it's not in website

spacewander · 2022-05-15T13:20:49Z

docs/en/latest/pubsub/kafka.md

+title: Apache Kafka
+keywords:
+  - APISIX
+  - Pub-Sub


Could we use PubSub like pubsub in other places?

All Pub-Sub replaced to PubSub

spacewander · 2022-05-15T13:23:13Z

docs/en/latest/pubsub/kafka.md

+
+### Limitations
+
+- Offsets need to be managed manually


After rendering, these two lines are merged together...

spacewander · 2022-05-15T13:23:41Z

docs/en/latest/pubsub/kafka.md

+
+### Prepare
+
+First, it is necessary to compile the [communication protocol](../../../../apisix/pubsub.proto) as a language-specific SDK using the `protoc`, which provides the command and response definitions to connect to Kafka via APISIX using the WebSocket.


Let's update the path of definition. We should use absolute path as it's not in website

spacewander · 2022-05-15T13:27:06Z

apisix/include/apisix/model/pubsub.proto

@@ -55,16 +73,18 @@ message CmdEmpty {}
 message PubSubReq {
    int64 sequence = 1;
    oneof req {
-        CmdEmpty cmd_empty = 31;
-        CmdPing cmd_ping = 32;
+        CmdEmpty cmd_empty                       = 31;


We can remove cmd_empty which is test-only? Using cmd_kafka_fetch in pubsub.t is enough.

@spacewander This would make the pubsub module test the relevant code that relies on kafka, and I'm not sure if I should do that.

What about adding a comment to show that this cmd is test-only?

After rechecking, I found that CmdEmpty has added a test-only flag.

apisix/apisix/include/apisix/model/pubsub.proto

Lines 35 to 39 in d955009

/**

* An empty command, a placeholder for testing purposes only

*/

message CmdEmpty {}

t/pubsub/kafka.t

apisix/pubsub/kafka.lua

docs/en/latest/pubsub/kafka.md

Co-authored-by: soulbird <zhaothree@gmail.com>

bzp2010 added 4 commits May 13, 2022 03:05

feat: extend pubsub proto

4d3e996

feat: add kafka upstream scheme

cb5e29b

chore: update kafka dependency

0db7413

feat: add kafka support

ac25c0b

bzp2010 force-pushed the feat-pubsub-kafka branch from a8eca0a to ac25c0b Compare May 12, 2022 20:15

chore: install pubsub.kafka

b2af519

bzp2010 added 4 commits May 13, 2022 11:49

test: add basic cases

d1f7280

docs: add documentation

fb1f202

fix: typo

797935d

fix: ensure pre-produce kafka messages

141aaa6

bzp2010 self-assigned this May 13, 2022

bzp2010 requested review from spacewander, membphis, shuaijinchao and tzssangglass May 13, 2022 05:25

bzp2010 marked this pull request as ready for review May 13, 2022 05:25

test: add more cases coverage

b7bebcf

bzp2010 changed the title ~~feat: support kafka pubsub~~ feat(pubsub): support kafka May 13, 2022

fix: lint

b9d3ace

bzp2010 mentioned this pull request May 13, 2022

feat: support kafka consumer for pubsub scenario #6995

Closed

5 tasks

test: add kafka upstream case

8ed6673

bzp2010 mentioned this pull request May 13, 2022

feat(pubsub): support kafka tls and sasl/plain auth #7046

Merged

5 tasks

membphis previously approved these changes May 15, 2022

View reviewed changes

spacewander reviewed May 15, 2022

View reviewed changes

bzp2010 added 4 commits May 15, 2022 21:52

chore: check int64 convert bound

78b7f88

chore: ensure pubsub proto use absolute link

3353581

fix: replace all Pub-Sub to PubSub

c65dac2

docs: fix style

aec71b7

bzp2010 dismissed membphis’s stale review via aec71b7 May 15, 2022 13:53

docs: fix style

438d982

bzp2010 requested review from membphis and spacewander May 15, 2022 13:56

tzssangglass reviewed May 16, 2022

View reviewed changes

apisix/pubsub/kafka.lua Show resolved Hide resolved

apisix/pubsub/kafka.lua Show resolved Hide resolved

apisix/pubsub/kafka.lua Show resolved Hide resolved

docs/en/latest/pubsub/kafka.md Outdated Show resolved Hide resolved

ci(traffic-split): improve ci stability (apache#7055)

bc475da

Co-authored-by: soulbird <zhaothree@gmail.com>

bzp2010 requested a review from tzssangglass May 16, 2022 07:16

tzssangglass previously approved these changes May 16, 2022

View reviewed changes

fix: review

e5b258f

bzp2010 dismissed tzssangglass’s stale review via e5b258f May 16, 2022 08:04

bzp2010 requested a review from tzssangglass May 16, 2022 08:07

tzssangglass approved these changes May 16, 2022

View reviewed changes

Merge master

db49fcb

spacewander approved these changes May 16, 2022

View reviewed changes

spacewander merged commit db26b6c into apache:master May 16, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022

feat(pubsub): support kafka (apache#7032)

90d0839

spacewander pushed a commit that referenced this pull request Jun 30, 2022

feat(pubsub): support kafka (#7032)

6993631

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pubsub): support kafka #7032

feat(pubsub): support kafka #7032

bzp2010 commented May 12, 2022

bzp2010 commented May 12, 2022

membphis left a comment

membphis May 15, 2022

bzp2010 May 15, 2022 •

edited

Loading

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 15, 2022

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 15, 2022

bzp2010 May 15, 2022

spacewander May 16, 2022

bzp2010 May 16, 2022

		@@ -42,6 +42,10 @@ In Apache APISIX, the most common scenario is handling north-south traffic from

		Currently, Apache APISIX supports WebSocket communication with the client, which can be any application that supports WebSocket, with Protocol Buffer as the serialization mechanism, see the [protocol definition](../../../apisix/pubsub.proto).


		### Prepare

		First, it is necessary to compile the [communication protocol](../../../../apisix/pubsub.proto) as a language-specific SDK using the `protoc`, which provides the command and response definitions to connect to Kafka via APISIX using the WebSocket.

	/**
	* An empty command, a placeholder for testing purposes only
	*/
	message CmdEmpty {}

feat(pubsub): support kafka #7032

feat(pubsub): support kafka #7032

Conversation

bzp2010 commented May 12, 2022

Description

Checklist

bzp2010 commented May 12, 2022

Update

membphis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bzp2010 May 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bzp2010 May 15, 2022 •

edited

Loading