Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4ec6b33
WIP: Kafka codec
adamkotwasinski Sep 25, 2018
aa51c6b
Fix compile error after rebases
adamkotwasinski Nov 3, 2018
074f868
Remove all request types except OffsetCommit v0..v1 (for review)
adamkotwasinski Nov 14, 2018
f341df7
Remove bytes buffers (for review - unused in these requests)
adamkotwasinski Nov 14, 2018
12ece4f
Apply review fixes
adamkotwasinski Nov 14, 2018
431a64c
Apply review fixes
adamkotwasinski Nov 15, 2018
ac5f851
Introduce composite deserializers for 2, 3, 4 delegates
adamkotwasinski Nov 19, 2018
8cb67ee
Review fixes:
adamkotwasinski Nov 20, 2018
7e3f54c
Apply review fixes:
adamkotwasinski Nov 27, 2018
d0534c3
Introduce generated code for Kafka requests:
adamkotwasinski Jan 10, 2019
db7763e
Add python tool for generating Kafka request classes; keep all versio…
adamkotwasinski Nov 27, 2018
00aac5b
In case of parse errors, consume rest of request properly; apply clan…
adamkotwasinski Feb 21, 2019
255f120
Merge remote-tracking branch 'envoy/master' into mergetest
adamkotwasinski Feb 25, 2019
bbf0e08
Fix formatting and clang-tidy
adamkotwasinski Feb 26, 2019
76cc44f
Fix spelling
adamkotwasinski Feb 26, 2019
6f86d76
Fixes after review: string_view used instead of raw pointers; documen…
adamkotwasinski Mar 11, 2019
23c1b9d
Merge remote-tracking branch 'envoy/master' into HEAD
adamkotwasinski Mar 14, 2019
f369bfa
Fixes: spelling, clang-tidy, documentation, cleaning up internal API;…
adamkotwasinski Mar 19, 2019
28641b4
Download whole Kafka specification; test fixes
adamkotwasinski Mar 19, 2019
3b9e324
Activate kafka tests in builds; review fixes: documentation, formatting
adamkotwasinski Mar 22, 2019
89c1a59
Attempt to point to Kafka codec in extension build; fix buffer overfl…
adamkotwasinski Mar 22, 2019
acdbcb3
Explicitly provide type of test values in generated Kafka tests
adamkotwasinski Mar 22, 2019
f5f5f32
Reorganize test code
adamkotwasinski Mar 25, 2019
20c7bf1
Create separate test class for each of Kafka tests; add missing forma…
adamkotwasinski Mar 26, 2019
81c97c8
Put Kafka tests in dedicated namespaces to avoid duplicate mock class…
adamkotwasinski Mar 26, 2019
130d2ac
Merge remote-tracking branch 'envoy/master' into codec
adamkotwasinski Mar 27, 2019
d47c2c2
Kick CI
adamkotwasinski Mar 28, 2019
145f28d
Kick CI
adamkotwasinski Mar 28, 2019
11d9288
Kick CI
adamkotwasinski Mar 28, 2019
ebe2134
Merge remote-tracking branch 'envoy/master' into codec
adamkotwasinski Apr 15, 2019
7f1abca
Put generated files in directories named 'external', so they do not g…
adamkotwasinski Apr 25, 2019
36f76c7
Add missing test to NullableArrayDeserializer
adamkotwasinski Apr 25, 2019
98c7752
Refactoring:
adamkotwasinski Apr 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/common/common/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace Logger {
FUNCTION(http) \
FUNCTION(http2) \
FUNCTION(hystrix) \
FUNCTION(kafka) \
FUNCTION(lua) \
FUNCTION(main) \
FUNCTION(misc) \
Expand Down
86 changes: 86 additions & 0 deletions source/extensions/filters/network/kafka/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
licenses(["notice"]) # Apache 2

# Kafka network filter.
# Public docs: docs/root/configuration/network_filters/kafka_filter.rst

load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
)

envoy_package()

envoy_cc_library(
name = "kafka_request_codec_lib",
srcs = ["request_codec.cc"],
hdrs = [
"codec.h",
"request_codec.h",
],
deps = [
":kafka_request_lib",
"//source/common/buffer:buffer_lib",
],
)

envoy_cc_library(
name = "kafka_request_lib",
srcs = [
"generated/kafka_request_resolver.cc",
"kafka_request.cc",
],
hdrs = [
"generated/requests.h",
"kafka_request.h",
],
deps = [
":parser_lib",
":serialization_lib",
"//source/common/common:assert_lib",
"//source/common/common:minimal_logger_lib",
],
)

envoy_cc_library(
name = "parser_lib",
hdrs = ["parser.h"],
deps = [
":kafka_protocol_lib",
":message_lib",
"//source/common/common:minimal_logger_lib",
],
)

envoy_cc_library(
name = "message_lib",
hdrs = [
"message.h",
],
deps = [
],
)

envoy_cc_library(
name = "serialization_lib",
hdrs = [
"generated/serialization_composite.h",
"serialization.h",
],
deps = [
":kafka_protocol_lib",
"//include/envoy/buffer:buffer_interface",
"//source/common/common:byte_order_lib",
],
)

envoy_cc_library(
name = "kafka_protocol_lib",
hdrs = [
"kafka_types.h",
],
external_deps = ["abseil_optional"],
deps = [
"//source/common/common:macros",
],
)
44 changes: 44 additions & 0 deletions source/extensions/filters/network/kafka/codec.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#pragma once

#include "envoy/buffer/buffer.h"
#include "envoy/common/pure.h"

namespace Envoy {
namespace Extensions {
namespace NetworkFilters {
namespace Kafka {

/**
* Kafka message decoder
* @tparam MessageType message type (Kafka request or Kafka response)
*/
template <typename MessageType> class MessageDecoder {
public:
virtual ~MessageDecoder() = default;

/**
* Processes given buffer attempting to decode messages of type MessageType container within
* @param data buffer instance
*/
virtual void onData(Buffer::Instance& data) PURE;
Comment thread
mattklein123 marked this conversation as resolved.
};

/**
* Kafka message decoder

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

comments out of date

* @tparam MessageType message type (Kafka request or Kafka response)
*/
template <typename MessageType> class MessageEncoder {
public:
virtual ~MessageEncoder() = default;

/**
* Encodes given message
* @param message message to be encoded
*/
virtual void encode(const MessageType& message) PURE;
};

} // namespace Kafka
} // namespace NetworkFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// DO NOT EDIT - THIS FILE WAS GENERATED

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Optimally we would not check in the generated code. Can we run the generator during the build so that everything is hermetic and reproducible? I think I would like to get that setup and then I will commit to a full and expedient review? WDYT @adamkotwasinski?

/wait

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mattklein123 Actual decision is yours, as then you are requiring to have jvm & gradle installed (as you need to get Kafka's Java library to get the protocol data, so you need Java - there is no BNF with Kafka spec, the BNF was created from Java code).

Erlang https://github.com/klarna/kafka_protocol#code-generation chose the other way - the generated files are committed.

It's up to you if you want to pull java&related tooling into your build ecosystem - I'm fine either way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We use Bazel. It's there. Yes I would prefer to make it hermetic. I think this can likely be done as an external genrule Bazel target.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You may be interested in apache/kafka#5893
Where we added a real protocol-generator to Apache Kafka. It will take us a while to get all our request/response pairs, but it will be great if we'll generate from same definitions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so with request/resp spec in json, we can remove java from ecosystem
I'll make rewrite the existing generator to python and invoke it as part of build

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@gwenshap FYI - if this gets merged in, we will depend on the fact that spec files present at https://github.com/apache/kafka/tree/2.2/clients/src/main/resources/common/message are named *Request.json (and in future *Response.json too)

// clang-format off
#include "extensions/filters/network/kafka/generated/requests.h"
#include "extensions/filters/network/kafka/kafka_request.h"
#include "extensions/filters/network/kafka/parser.h"

namespace Envoy {
namespace Extensions {
namespace NetworkFilters {
namespace Kafka {

const RequestParserResolver RequestParserResolver::INSTANCE;

ParserSharedPtr RequestParserResolver::createParser(int16_t api_key, int16_t api_version,
RequestContextSharedPtr context) const {

if (8 == api_key && 0 == api_version) {
return std::make_shared<OffsetCommitRequestV0Parser>(context);
}
if (8 == api_key && 1 == api_version) {
return std::make_shared<OffsetCommitRequestV1Parser>(context);
}
if (8 == api_key && 2 == api_version) {
return std::make_shared<OffsetCommitRequestV2Parser>(context);
}
if (8 == api_key && 3 == api_version) {
return std::make_shared<OffsetCommitRequestV3Parser>(context);
}
return std::make_shared<SentinelParser>(context);
}

} // namespace Kafka
} // namespace NetworkFilters
} // namespace Extensions
} // namespace Envoy
// clang-format on
Loading