From e61e461736a28e26b6fcf0ca25d34c47ed29b0fc Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Tue, 12 Dec 2023 11:16:01 -0800 Subject: [PATCH] Revert "[ZK filter] Add per-opcode decoder error metrics (#31138)" (#31245) This is the prerequisite of reverting #30438 in order to fix the ZK proxy filter "Uncaught Exception" issue. Risk Level: Low Testing: Unit test Docs Changes: Revert doc changes in #31138 Release Notes: Revert release notes in #31138 Platform Specific Features: N/A Signed-off-by: Zhewei Hu --- .../zookeeper_proxy/v3/zookeeper_proxy.proto | 2 +- changelogs/current.yaml | 4 - .../_include/zookeeper-filter-proxy.yaml | 1 - .../zookeeper_proxy_filter.rst | 30 -- .../filters/network/zookeeper_proxy/config.cc | 7 +- .../network/zookeeper_proxy/decoder.cc | 239 +++++++-------- .../filters/network/zookeeper_proxy/decoder.h | 11 +- .../filters/network/zookeeper_proxy/filter.cc | 107 +++---- .../filters/network/zookeeper_proxy/filter.h | 45 +-- .../filters/network/zookeeper_proxy/utils.h | 8 +- .../network/zookeeper_proxy/config_test.cc | 5 - .../network/zookeeper_proxy/filter_test.cc | 289 +++++++----------- 12 files changed, 258 insertions(+), 490 deletions(-) diff --git a/api/envoy/extensions/filters/network/zookeeper_proxy/v3/zookeeper_proxy.proto b/api/envoy/extensions/filters/network/zookeeper_proxy/v3/zookeeper_proxy.proto index bb19c752e0c8..a3825f10c9ff 100644 --- a/api/envoy/extensions/filters/network/zookeeper_proxy/v3/zookeeper_proxy.proto +++ b/api/envoy/extensions/filters/network/zookeeper_proxy/v3/zookeeper_proxy.proto @@ -66,7 +66,7 @@ message ZooKeeperProxy { // Whether to emit per opcode response bytes metrics. If not set, it defaults to false. bool enable_per_opcode_response_bytes_metrics = 8; - // Whether to emit per opcode decoder error metrics. If not set, it defaults to false. + // [#not-implemented-hide:] Whether to emit per opcode decoder error metrics. If not set, it defaults to false. bool enable_per_opcode_decoder_error_metrics = 9; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index b691ea12dc54..77faa45c0ed5 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -265,10 +265,6 @@ new_features: Added support to copy headers from the redirect response to the triggered request. See :ref:`response_headers_to_copy`. -- area: zookeeper - change: | - Added support for emitting per opcode decoder error metrics via :ref:`enable_per_opcode_decoder_error_metrics - `. - area: stateful_session change: | Added :ref:`strict mode ` diff --git a/docs/root/configuration/listeners/network_filters/_include/zookeeper-filter-proxy.yaml b/docs/root/configuration/listeners/network_filters/_include/zookeeper-filter-proxy.yaml index 25a534fc79d1..4b079ef6362d 100644 --- a/docs/root/configuration/listeners/network_filters/_include/zookeeper-filter-proxy.yaml +++ b/docs/root/configuration/listeners/network_filters/_include/zookeeper-filter-proxy.yaml @@ -13,7 +13,6 @@ static_resources: stat_prefix: zookeeper enable_per_opcode_request_bytes_metrics: true enable_per_opcode_response_bytes_metrics: true - enable_per_opcode_decoder_error_metrics: true enable_latency_threshold_metrics: true default_latency_threshold: "0.1s" latency_threshold_overrides: diff --git a/docs/root/configuration/listeners/network_filters/zookeeper_proxy_filter.rst b/docs/root/configuration/listeners/network_filters/zookeeper_proxy_filter.rst index bde21b88127f..e98f00795008 100644 --- a/docs/root/configuration/listeners/network_filters/zookeeper_proxy_filter.rst +++ b/docs/root/configuration/listeners/network_filters/zookeeper_proxy_filter.rst @@ -41,8 +41,6 @@ Every configured ZooKeeper proxy filter has statistics rooted at *. *_resp_bytes* are per-opcode metrics and will only be emitted when :ref:`enable_per_opcode_response_bytes_metrics ` is set to ``true``. -*_decoder_error* are per-opcode metrics and will only be emitted when :ref:`enable_per_opcode_decoder_error_metrics ` is set to ``true``. - The following counters are available: .. csv-table:: @@ -50,34 +48,6 @@ The following counters are available: :widths: 1, 1, 2 decoder_error, Counter, Number of times a message wasn't decoded - connect_decoder_error, Counter, Number of times a connect request message wasn't decoded - ping_decoder_error, Counter, Number of times a ping request message wasn't decoded - auth_decoder_error, Counter, Number of times a auth request message wasn't decoded - getdata_decoder_error, Counter, Number of times a getdata request message wasn't decoded - create_decoder_error, Counter, Number of times a create request message wasn't decoded - create2_decoder_error, Counter, Number of times a create2 request message wasn't decoded - createcontainer_decoder_error, Counter, Number of times a createcontainer request message wasn't decoded - createttl_decoder_error, Counter, Number of times a createttl request message wasn't decoded - setdata_decoder_error, Counter, Number of times a setdata request message wasn't decoded - getchildren_decoder_error, Counter, Number of times a getchildren request message wasn't decoded - getchildren2_decoder_error, Counter, Number of times a getchildren2 request message wasn't decoded - getephemerals_decoder_error, Counter, Number of times a getephemerals request message wasn't decoded - getallchildrennumber_decoder_error, Counter, Number of times a getallchildrennumber request message wasn't decoded - delete_decoder_error, Counter, Number of times a delete request message wasn't decoded - exists_decoder_error, Counter, Number of times a exists request message wasn't decoded - getacl_decoder_error, Counter, Number of times a getacl request message wasn't decoded - setacl_decoder_error, Counter, Number of times a setacl request message wasn't decoded - sync_decoder_error, Counter, Number of times a sync request message wasn't decoded - multi_decoder_error, Counter, Number of times a multi request message wasn't decoded - reconfig_decoder_error, Counter, Number of times a reconfig request message wasn't decoded - close_decoder_error, Counter, Number of times a close request message wasn't decoded - setauth_decoder_error, Counter, Number of times a setauth request message wasn't decoded - setwatches_decoder_error, Counter, Number of times a setwatches request message wasn't decoded - setwatches2_decoder_error, Counter, Number of times a setwatches2 request message wasn't decoded - addwatch_decoder_error, Counter, Number of times a addwatch request message wasn't decoded - checkwatches_decoder_error, Counter, Number of times a checkwatches request message wasn't decoded - removewatches_decoder_error, Counter, Number of times a removewatches request message wasn't decoded - check_decoder_error, Counter, Number of times a check request message wasn't decoded request_bytes, Counter, Number of bytes in decoded request messages connect_rq_bytes, Counter, Number of bytes in decoded connect request messages ping_rq_bytes, Counter, Number of bytes in decoded ping request messages diff --git a/source/extensions/filters/network/zookeeper_proxy/config.cc b/source/extensions/filters/network/zookeeper_proxy/config.cc index bfc69806069e..f789a884d9ca 100644 --- a/source/extensions/filters/network/zookeeper_proxy/config.cc +++ b/source/extensions/filters/network/zookeeper_proxy/config.cc @@ -31,8 +31,6 @@ Network::FilterFactoryCb ZooKeeperConfigFactory::createFilterFactoryFromProtoTyp proto_config.enable_per_opcode_request_bytes_metrics(); const bool enable_per_opcode_response_bytes_metrics = proto_config.enable_per_opcode_response_bytes_metrics(); - const bool enable_per_opcode_decoder_error_metrics = - proto_config.enable_per_opcode_decoder_error_metrics(); const bool enable_latency_threshold_metrics = proto_config.enable_latency_threshold_metrics(); const std::chrono::milliseconds default_latency_threshold( PROTOBUF_GET_MS_OR_DEFAULT(proto_config, default_latency_threshold, 100)); @@ -51,9 +49,8 @@ Network::FilterFactoryCb ZooKeeperConfigFactory::createFilterFactoryFromProtoTyp ZooKeeperFilterConfigSharedPtr filter_config(std::make_shared( stat_prefix, max_packet_bytes, enable_per_opcode_request_bytes_metrics, - enable_per_opcode_response_bytes_metrics, enable_per_opcode_decoder_error_metrics, - enable_latency_threshold_metrics, default_latency_threshold, latency_threshold_overrides, - context.scope())); + enable_per_opcode_response_bytes_metrics, enable_latency_threshold_metrics, + default_latency_threshold, latency_threshold_overrides, context.scope())); auto& time_source = context.serverFactoryContext().mainThreadDispatcher().timeSource(); return [filter_config, &time_source](Network::FilterManager& filter_manager) -> void { diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 0fc676fd6ac9..92367e55063a 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -50,18 +50,18 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - len, absl::nullopt, fmt::format("peekInt32 for len: {}", len.status().message())); + len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with len {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, absl::nullopt, fmt::format("ensureMinLength: {}", status.message())); + status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, absl::nullopt, fmt::format("ensureMaxLength: {}", status.message())); + status, fmt::format("ensureMaxLength: {}", status.message())); auto start_time = time_source_.monotonicTime(); @@ -77,7 +77,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // as a regular data request, so we support that as well. const absl::StatusOr xid = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - xid, absl::nullopt, fmt::format("peerInt32 for xid: {}", xid.status().message())); + xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with xid {} at offset {}", xid.value(), offset); @@ -124,7 +124,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // the session alive. const absl::StatusOr oc = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - oc, absl::nullopt, fmt::format("peekInt32 for opcode: {}", oc.status().message())); + oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with opcode {} at offset {}", oc.value(), offset); @@ -140,7 +140,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan case OpCodes::Create2: case OpCodes::CreateContainer: case OpCodes::CreateTtl: - status = parseCreateRequest(data, offset, len.value(), opcode); + status = parseCreateRequest(data, offset, len.value(), static_cast(opcode)); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("parseCreateRequest: {}", status.message())); break; @@ -152,12 +152,12 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan case OpCodes::GetChildren: status = parseGetChildrenRequest(data, offset, len.value(), false); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, fmt::format("parseGetChildrenRequest (get children): {}", status.message())); + status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::GetChildren2: status = parseGetChildrenRequest(data, offset, len.value(), true); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, fmt::format("parseGetChildrenRequest (get children2): {}", status.message())); + status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len.value()); @@ -180,7 +180,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan status, fmt::format("parseSetAclRequest: {}", status.message())); break; case OpCodes::Sync: - status = callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value()), opcode); + status = callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("onSyncRequest: {}", status.message())); break; @@ -225,13 +225,12 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan status, fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); break; case OpCodes::GetEphemerals: - status = callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value()), opcode); + status = callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("onGetEphemeralsRequest: {}", status.message())); break; case OpCodes::GetAllChildrenNumber: - status = callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len.value()), - opcode); + status = callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len.value())); RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("onGetAllChildrenNumberRequest: {}", status.message())); break; @@ -241,7 +240,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan default: ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: unknown opcode {}", enumToSignedInt(opcode)); - callbacks_.onDecodeError(absl::nullopt); + callbacks_.onDecodeError(); return absl::nullopt; } @@ -258,7 +257,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - len, absl::nullopt, fmt::format("peekInt32 for len: {}", len.status().message())); + len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding response with len.value() {} at offset {}", len.value(), offset); @@ -266,15 +265,15 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta absl::Status status = ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, absl::nullopt, fmt::format("ensureMinLength: {}", status.message())); + status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, absl::nullopt, fmt::format("ensureMaxLength: {}", status.message())); + status, fmt::format("ensureMaxLength: {}", status.message())); const absl::StatusOr xid = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - xid, absl::nullopt, fmt::format("peekInt32 for xid: {}", xid.status().message())); + xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding response with xid {} at offset {}", xid.value(), offset); @@ -293,7 +292,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta case XidCodes::SetWatchesXid: latency = fetchControlRequestData(xid.value(), opcode); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - latency, opcode, fmt::format("fetchControlRequestData: {}", latency.status().message())); + latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); break; case XidCodes::WatchXid: // WATCH_XID is generated by the server, no need to fetch opcode and latency here. @@ -301,7 +300,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta default: latency = fetchDataRequestData(xid.value(), opcode); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - latency, opcode, fmt::format("fetchDataRequestData: {}", latency.status().message())); + latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); } // Connect responses are special, they have no full reply header @@ -317,11 +316,11 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Control responses that aren't connect, with XIDs <= 0. const absl::StatusOr zxid = helper_.peekInt64(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - zxid, absl::nullopt, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); + zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); const absl::StatusOr error = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - error, absl::nullopt, fmt::format("peekInt32 for error: {}", error.status().message())); + error, fmt::format("peekInt32 for error: {}", error.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding response with zxid.value() {} and error {} at offset {}", @@ -372,17 +371,17 @@ absl::Status DecoderImpl::ensureMaxLength(const int32_t len) const { absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Connect); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip zxid, timeout, and session id. offset += ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH; // Skip password. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Connect); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, OpCodes::Connect, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -393,17 +392,16 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetAuth); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip opcode + type. offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, OpCodes::SetAuth, - scheme.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); // Skip credential. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetAuth); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onAuthRequest(scheme.value()); @@ -413,15 +411,13 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::GetData); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::GetData, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, OpCodes::GetData, - watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetDataRequest(path.value(), watch.value()); @@ -452,46 +448,43 @@ absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, opcode, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = skipAcls(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); absl::StatusOr flag_data = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, opcode, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); return absl::OkStatus(); } absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetData); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::SetData, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetData); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, OpCodes::SetData, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetRequest(path.value()); @@ -499,23 +492,17 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs } absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, - uint32_t len, const bool v2) { + uint32_t len, const bool two) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - OpCodes opcode = OpCodes::GetChildren; - if (v2) { - opcode = OpCodes::GetChildren2; - } - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, opcode, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, opcode, - watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); - callbacks_.onGetChildrenRequest(path.value(), watch.value(), v2); + callbacks_.onGetChildrenRequest(path.value(), watch.value(), two); return absl::OkStatus(); } @@ -523,15 +510,13 @@ absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Delete); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::Delete, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, OpCodes::Delete, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -541,15 +526,13 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Exists); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::Exists, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, OpCodes::Exists, - watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onExistsRequest(path.value(), watch.value()); @@ -559,11 +542,10 @@ absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::GetAcl); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::GetAcl, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onGetAclRequest(path.value()); @@ -573,18 +555,16 @@ absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetAcl); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::SetAcl, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); status = skipAcls(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetAcl); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr version = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, OpCodes::SetAcl, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -594,8 +574,7 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("zookeeper_proxy: path only request decoding exception {}", status.message())); @@ -605,15 +584,13 @@ absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Check); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::Check, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, OpCodes::Check, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -624,21 +601,18 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of uint32_t len) { // Treat empty transactions as a decoding error, there should be at least 1 header. absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Multi); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, OpCodes::Multi, - opcode.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, OpCodes::Multi, - done.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, OpCodes::Multi, - error.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); if (done.value()) { break; @@ -647,22 +621,22 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Create); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetData); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Check); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Delete); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; default: - callbacks_.onDecodeError(absl::nullopt); + callbacks_.onDecodeError(); return absl::InvalidArgumentError( fmt::format("unknown opcode within a transaction: {}", opcode.value())); } @@ -677,22 +651,22 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Reconfig); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip joining. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Reconfig); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip leaving. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Reconfig); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip new members. status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Reconfig); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, OpCodes::Reconfig, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, config_id.status().message()); callbacks_.onReconfigRequest(); @@ -704,24 +678,23 @@ absl::Status DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_ uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, OpCodes::SetWatches, - zxid.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatchesRequest(); @@ -732,32 +705,31 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, OpCodes::SetWatches2, - zxid.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent recursive watches. status = skipStrings(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::SetWatches2); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatches2Request(); @@ -767,15 +739,13 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::AddWatch); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, OpCodes::AddWatch, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr mode = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, OpCodes::AddWatch, - mode.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); callbacks_.onAddWatchRequest(path.value(), mode.value()); @@ -785,14 +755,13 @@ absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, opcode, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch_type = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, opcode, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { @@ -887,15 +856,15 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy // Peek packet length. len = helper_.peekInt32(data, offset); EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - len, absl::nullopt, fmt::format("peekInt32 for len: {}", len.status().message())); + len, fmt::format("peekInt32 for len: {}", len.status().message())); status = ensureMinLength(len.value(), dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, absl::nullopt); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = ensureMaxLength(len.value()); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, absl::nullopt); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); offset += len.value(); if (offset <= data_len) { @@ -974,19 +943,18 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const std::chrono::milliseconds latency) { absl::Status status = ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Connect); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr timeout = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, OpCodes::Connect, - timeout.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; status = skipString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, OpCodes::Connect); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, OpCodes::Connect, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -998,19 +966,18 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs const uint32_t len, const int64_t zxid, const int32_t error) { absl::Status status = ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); - EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, absl::nullopt); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr event_type = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, absl::nullopt, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, absl::nullopt, + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); - EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, absl::nullopt, - path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onWatchEvent(event_type.value(), client_state.value(), path.value(), zxid, error); diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.h b/source/extensions/filters/network/zookeeper_proxy/decoder.h index 875edd363546..8718a8d9d6b1 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.h +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.h @@ -80,7 +80,7 @@ class DecoderCallbacks { public: virtual ~DecoderCallbacks() = default; - virtual void onDecodeError(const absl::optional opcode) PURE; + virtual void onDecodeError() PURE; virtual void onRequestBytes(const absl::optional opcode, const uint64_t bytes) PURE; virtual void onConnect(bool readonly) PURE; virtual void onPing() PURE; @@ -90,16 +90,13 @@ class DecoderCallbacks { OpCodes opcode) PURE; virtual void onSetRequest(const std::string& path) PURE; virtual void onGetChildrenRequest(const std::string& path, bool watch, bool v2) PURE; - virtual absl::Status onGetEphemeralsRequest(const absl::StatusOr& path, - const OpCodes opcode) PURE; - virtual absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path, - const OpCodes opcode) PURE; + virtual absl::Status onGetEphemeralsRequest(const absl::StatusOr& path) PURE; + virtual absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path) PURE; virtual void onDeleteRequest(const std::string& path, int32_t version) PURE; virtual void onExistsRequest(const std::string& path, bool watch) PURE; virtual void onGetAclRequest(const std::string& path) PURE; virtual void onSetAclRequest(const std::string& path, int32_t version) PURE; - virtual absl::Status onSyncRequest(const absl::StatusOr& path, - const OpCodes opcode) PURE; + virtual absl::Status onSyncRequest(const absl::StatusOr& path) PURE; virtual void onCheckRequest(const std::string& path, int32_t version) PURE; virtual void onMultiRequest() PURE; virtual void onReconfigRequest() PURE; diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc index 29e599ba782b..817fa14077d4 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.cc +++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc @@ -24,7 +24,7 @@ ZooKeeperFilterConfig::ZooKeeperFilterConfig( const std::string& stat_prefix, const uint32_t max_packet_bytes, const bool enable_per_opcode_request_bytes_metrics, const bool enable_per_opcode_response_bytes_metrics, - const bool enable_per_opcode_decoder_error_metrics, const bool enable_latency_threshold_metrics, + const bool enable_latency_threshold_metrics, const std::chrono::milliseconds default_latency_threshold, const LatencyThresholdOverrideList& latency_threshold_overrides, Stats::Scope& scope) : scope_(scope), max_packet_bytes_(max_packet_bytes), stats_(generateStats(stat_prefix, scope)), @@ -35,7 +35,6 @@ ZooKeeperFilterConfig::ZooKeeperFilterConfig( unknown_opcode_latency_(stat_name_set_->add("unknown_opcode_latency")), enable_per_opcode_request_bytes_metrics_(enable_per_opcode_request_bytes_metrics), enable_per_opcode_response_bytes_metrics_(enable_per_opcode_response_bytes_metrics), - enable_per_opcode_decoder_error_metrics_(enable_per_opcode_decoder_error_metrics), enable_latency_threshold_metrics_(enable_latency_threshold_metrics), default_latency_threshold_(default_latency_threshold), latency_threshold_override_map_(parseLatencyThresholdOverrides(latency_threshold_overrides)) { @@ -46,89 +45,79 @@ ZooKeeperFilterConfig::ZooKeeperFilterConfig( {"auth_rq", "digest_rq", "host_rq", "ip_rq", "ping_response_rq", "world_rq", "x509_rq"}); initOpCode(OpCodes::Ping, stats_.ping_resp_, stats_.ping_resp_fast_, stats_.ping_resp_slow_, - stats_.ping_rq_bytes_, stats_.ping_resp_bytes_, stats_.ping_decoder_error_, - "ping_response"); + stats_.ping_rq_bytes_, stats_.ping_resp_bytes_, "ping_response"); initOpCode(OpCodes::SetAuth, stats_.auth_resp_, stats_.auth_resp_fast_, stats_.auth_resp_slow_, - stats_.auth_rq_bytes_, stats_.auth_resp_bytes_, stats_.auth_decoder_error_, - "auth_response"); + stats_.auth_rq_bytes_, stats_.auth_resp_bytes_, "auth_response"); initOpCode(OpCodes::GetData, stats_.getdata_resp_, stats_.getdata_resp_fast_, stats_.getdata_resp_slow_, stats_.getdata_rq_bytes_, stats_.getdata_resp_bytes_, - stats_.getdata_decoder_error_, "getdata_resp"); + "getdata_resp"); initOpCode(OpCodes::Create, stats_.create_resp_, stats_.create_resp_fast_, stats_.create_resp_slow_, stats_.create_rq_bytes_, stats_.create_resp_bytes_, - stats_.create_decoder_error_, "create_resp"); + "create_resp"); initOpCode(OpCodes::Create2, stats_.create2_resp_, stats_.create2_resp_fast_, stats_.create2_resp_slow_, stats_.create2_rq_bytes_, stats_.create2_resp_bytes_, - stats_.create2_decoder_error_, "create2_resp"); + "create2_resp"); initOpCode(OpCodes::CreateContainer, stats_.createcontainer_resp_, stats_.createcontainer_resp_fast_, stats_.createcontainer_resp_slow_, stats_.createcontainer_rq_bytes_, stats_.createcontainer_resp_bytes_, - stats_.createcontainer_decoder_error_, "createcontainer_resp"); + "createcontainer_resp"); initOpCode(OpCodes::CreateTtl, stats_.createttl_resp_, stats_.createttl_resp_fast_, stats_.createttl_resp_slow_, stats_.createttl_rq_bytes_, stats_.createttl_resp_bytes_, - stats_.createttl_decoder_error_, "createttl_resp"); + "createttl_resp"); initOpCode(OpCodes::SetData, stats_.setdata_resp_, stats_.setdata_resp_fast_, stats_.setdata_resp_slow_, stats_.setdata_rq_bytes_, stats_.setdata_resp_bytes_, - stats_.setdata_decoder_error_, "setdata_resp"); + "setdata_resp"); initOpCode(OpCodes::GetChildren, stats_.getchildren_resp_, stats_.getchildren_resp_fast_, stats_.getchildren_resp_slow_, stats_.getchildren_rq_bytes_, - stats_.getchildren_resp_bytes_, stats_.getchildren_decoder_error_, "getchildren_resp"); + stats_.getchildren_resp_bytes_, "getchildren_resp"); initOpCode(OpCodes::GetChildren2, stats_.getchildren2_resp_, stats_.getchildren2_resp_fast_, stats_.getchildren2_resp_slow_, stats_.getchildren2_rq_bytes_, - stats_.getchildren2_resp_bytes_, stats_.getchildren2_decoder_error_, - "getchildren2_resp"); + stats_.getchildren2_resp_bytes_, "getchildren2_resp"); initOpCode(OpCodes::Delete, stats_.delete_resp_, stats_.delete_resp_fast_, stats_.delete_resp_slow_, stats_.delete_rq_bytes_, stats_.delete_resp_bytes_, - stats_.delete_decoder_error_, "delete_resp"); + "delete_resp"); initOpCode(OpCodes::Exists, stats_.exists_resp_, stats_.exists_resp_fast_, stats_.exists_resp_slow_, stats_.exists_rq_bytes_, stats_.exists_resp_bytes_, - stats_.exists_decoder_error_, "exists_resp"); + "exists_resp"); initOpCode(OpCodes::GetAcl, stats_.getacl_resp_, stats_.getacl_resp_fast_, stats_.getacl_resp_slow_, stats_.getacl_rq_bytes_, stats_.getacl_resp_bytes_, - stats_.getacl_decoder_error_, "getacl_resp"); + "getacl_resp"); initOpCode(OpCodes::SetAcl, stats_.setacl_resp_, stats_.setacl_resp_fast_, stats_.setacl_resp_slow_, stats_.setacl_rq_bytes_, stats_.setacl_resp_bytes_, - stats_.setacl_decoder_error_, "setacl_resp"); + "setacl_resp"); initOpCode(OpCodes::Sync, stats_.sync_resp_, stats_.sync_resp_fast_, stats_.sync_resp_slow_, - stats_.sync_rq_bytes_, stats_.sync_resp_bytes_, stats_.sync_decoder_error_, - "sync_resp"); + stats_.sync_rq_bytes_, stats_.sync_resp_bytes_, "sync_resp"); initOpCode(OpCodes::Check, stats_.check_resp_, stats_.check_resp_fast_, stats_.check_resp_slow_, - stats_.check_rq_bytes_, stats_.check_resp_bytes_, stats_.check_decoder_error_, - "check_resp"); + stats_.check_rq_bytes_, stats_.check_resp_bytes_, "check_resp"); initOpCode(OpCodes::Multi, stats_.multi_resp_, stats_.multi_resp_fast_, stats_.multi_resp_slow_, - stats_.multi_rq_bytes_, stats_.multi_resp_bytes_, stats_.multi_decoder_error_, - "multi_resp"); + stats_.multi_rq_bytes_, stats_.multi_resp_bytes_, "multi_resp"); initOpCode(OpCodes::Reconfig, stats_.reconfig_resp_, stats_.reconfig_resp_fast_, stats_.reconfig_resp_slow_, stats_.reconfig_rq_bytes_, stats_.reconfig_resp_bytes_, - stats_.reconfig_decoder_error_, "reconfig_resp"); + "reconfig_resp"); initOpCode(OpCodes::SetWatches, stats_.setwatches_resp_, stats_.setwatches_resp_fast_, stats_.setwatches_resp_slow_, stats_.setwatches_rq_bytes_, - stats_.setwatches_resp_bytes_, stats_.setwatches_decoder_error_, "setwatches_resp"); + stats_.setwatches_resp_bytes_, "setwatches_resp"); initOpCode(OpCodes::SetWatches2, stats_.setwatches2_resp_, stats_.setwatches2_resp_fast_, stats_.setwatches2_resp_slow_, stats_.setwatches2_rq_bytes_, - stats_.setwatches2_resp_bytes_, stats_.setwatches2_decoder_error_, "setwatches2_resp"); + stats_.setwatches2_resp_bytes_, "setwatches2_resp"); initOpCode(OpCodes::AddWatch, stats_.addwatch_resp_, stats_.addwatch_resp_fast_, stats_.addwatch_resp_slow_, stats_.addwatch_rq_bytes_, stats_.addwatch_resp_bytes_, - stats_.addwatch_decoder_error_, "addwatch_resp"); + "addwatch_resp"); initOpCode(OpCodes::CheckWatches, stats_.checkwatches_resp_, stats_.checkwatches_resp_fast_, stats_.checkwatches_resp_slow_, stats_.checkwatches_rq_bytes_, - stats_.checkwatches_resp_bytes_, stats_.checkwatches_decoder_error_, - "checkwatches_resp"); + stats_.checkwatches_resp_bytes_, "checkwatches_resp"); initOpCode(OpCodes::RemoveWatches, stats_.removewatches_resp_, stats_.removewatches_resp_fast_, stats_.removewatches_resp_slow_, stats_.removewatches_rq_bytes_, - stats_.removewatches_resp_bytes_, stats_.removewatches_decoder_error_, - "removewatches_resp"); + stats_.removewatches_resp_bytes_, "removewatches_resp"); initOpCode(OpCodes::GetEphemerals, stats_.getephemerals_resp_, stats_.getephemerals_resp_fast_, stats_.getephemerals_resp_slow_, stats_.getephemerals_rq_bytes_, - stats_.getephemerals_resp_bytes_, stats_.getephemerals_decoder_error_, - "getephemerals_resp"); + stats_.getephemerals_resp_bytes_, "getephemerals_resp"); initOpCode(OpCodes::GetAllChildrenNumber, stats_.getallchildrennumber_resp_, stats_.getallchildrennumber_resp_fast_, stats_.getallchildrennumber_resp_slow_, stats_.getallchildrennumber_rq_bytes_, stats_.getallchildrennumber_resp_bytes_, - stats_.getallchildrennumber_decoder_error_, "getallchildrennumber_resp"); + "getallchildrennumber_resp"); initOpCode(OpCodes::Close, stats_.close_resp_, stats_.close_resp_fast_, stats_.close_resp_slow_, - stats_.close_rq_bytes_, stats_.close_resp_bytes_, stats_.close_decoder_error_, - "close_resp"); + stats_.close_rq_bytes_, stats_.close_resp_bytes_, "close_resp"); } ErrorBudgetResponseType @@ -157,16 +146,13 @@ void ZooKeeperFilterConfig::initOpCode(OpCodes opcode, Stats::Counter& resp_coun Stats::Counter& resp_fast_counter, Stats::Counter& resp_slow_counter, Stats::Counter& rq_bytes_counter, - Stats::Counter& resp_bytes_counter, - Stats::Counter& decoder_error_counter, - absl::string_view name) { + Stats::Counter& resp_bytes_counter, absl::string_view name) { OpCodeInfo& opcode_info = op_code_map_[opcode]; opcode_info.resp_counter_ = &resp_counter; opcode_info.resp_fast_counter_ = &resp_fast_counter; opcode_info.resp_slow_counter_ = &resp_slow_counter; opcode_info.rq_bytes_counter_ = &rq_bytes_counter; opcode_info.resp_bytes_counter_ = &resp_bytes_counter; - opcode_info.decoder_error_counter_ = &decoder_error_counter; opcode_info.opname_ = std::string(name); opcode_info.latency_name_ = stat_name_set_->add(absl::StrCat(name, "_latency")); } @@ -253,18 +239,8 @@ void ZooKeeperFilter::onConnect(const bool readonly) { } } -void ZooKeeperFilter::onDecodeError(const absl::optional opcode) { +void ZooKeeperFilter::onDecodeError() { config_->stats_.decoder_error_.inc(); - - if (config_->enable_per_opcode_decoder_error_metrics_ && opcode.has_value() && - config_->op_code_map_.contains(*opcode)) { - if (*opcode == OpCodes::Connect) { - config_->stats_.connect_decoder_error_.inc(); - } else { - config_->op_code_map_[*opcode].decoder_error_counter_->inc(); - } - } - setDynamicMetadata("opname", "error"); } @@ -388,12 +364,8 @@ void ZooKeeperFilter::onSetAclRequest(const std::string& path, const int32_t ver setDynamicMetadata({{"opname", "setacl"}, {"path", path}, {"version", std::to_string(version)}}); } -absl::Status ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path, - const OpCodes opcode) { - if (!path.ok()) { - onDecodeError(opcode); - return absl::InvalidArgumentError(path.status().message()); - } +absl::Status ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); config_->stats_.sync_rq_.inc(); setDynamicMetadata({{"opname", "sync"}, {"path", path.value()}}); @@ -440,12 +412,8 @@ void ZooKeeperFilter::onAddWatchRequest(const std::string& path, const int32_t m setDynamicMetadata({{"opname", "addwatch"}, {"path", path}, {"mode", std::to_string(mode)}}); } -absl::Status ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path, - const OpCodes opcode) { - if (!path.ok()) { - onDecodeError(opcode); - return absl::InvalidArgumentError(path.status().message()); - } +absl::Status ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); config_->stats_.getephemerals_rq_.inc(); setDynamicMetadata({{"opname", "getephemerals"}, {"path", path.value()}}); @@ -453,12 +421,9 @@ absl::Status ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path, - const OpCodes opcode) { - if (!path.ok()) { - onDecodeError(opcode); - return absl::InvalidArgumentError(path.status().message()); - } +absl::Status +ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); config_->stats_.getallchildrennumber_rq_.inc(); setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path.value()}}); diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.h b/source/extensions/filters/network/zookeeper_proxy/filter.h index 8dfe85e1e91b..94f47506b211 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.h +++ b/source/extensions/filters/network/zookeeper_proxy/filter.h @@ -29,34 +29,6 @@ namespace ZooKeeperProxy { */ #define ALL_ZOOKEEPER_PROXY_STATS(COUNTER) \ COUNTER(decoder_error) \ - COUNTER(connect_decoder_error) \ - COUNTER(ping_decoder_error) \ - COUNTER(auth_decoder_error) \ - COUNTER(getdata_decoder_error) \ - COUNTER(create_decoder_error) \ - COUNTER(create2_decoder_error) \ - COUNTER(createcontainer_decoder_error) \ - COUNTER(createttl_decoder_error) \ - COUNTER(setdata_decoder_error) \ - COUNTER(getchildren_decoder_error) \ - COUNTER(getchildren2_decoder_error) \ - COUNTER(getephemerals_decoder_error) \ - COUNTER(getallchildrennumber_decoder_error) \ - COUNTER(delete_decoder_error) \ - COUNTER(exists_decoder_error) \ - COUNTER(getacl_decoder_error) \ - COUNTER(setacl_decoder_error) \ - COUNTER(sync_decoder_error) \ - COUNTER(multi_decoder_error) \ - COUNTER(reconfig_decoder_error) \ - COUNTER(close_decoder_error) \ - COUNTER(setauth_decoder_error) \ - COUNTER(setwatches_decoder_error) \ - COUNTER(setwatches2_decoder_error) \ - COUNTER(addwatch_decoder_error) \ - COUNTER(checkwatches_decoder_error) \ - COUNTER(removewatches_decoder_error) \ - COUNTER(check_decoder_error) \ COUNTER(request_bytes) \ COUNTER(connect_rq_bytes) \ COUNTER(connect_readonly_rq_bytes) \ @@ -253,7 +225,6 @@ class ZooKeeperFilterConfig { ZooKeeperFilterConfig(const std::string& stat_prefix, const uint32_t max_packet_bytes, const bool enable_per_opcode_request_bytes_metrics, const bool enable_per_opcode_response_bytes_metrics, - const bool enable_per_opcode_decoder_error_metrics, const bool enable_latency_threshold_metrics, const std::chrono::milliseconds default_latency_threshold, const LatencyThresholdOverrideList& latency_threshold_overrides, @@ -276,7 +247,6 @@ class ZooKeeperFilterConfig { Stats::Counter* resp_slow_counter_; Stats::Counter* rq_bytes_counter_; Stats::Counter* resp_bytes_counter_; - Stats::Counter* decoder_error_counter_; std::string opname_; Stats::StatName latency_name_; }; @@ -293,7 +263,6 @@ class ZooKeeperFilterConfig { const Stats::StatName unknown_opcode_latency_; const bool enable_per_opcode_request_bytes_metrics_; const bool enable_per_opcode_response_bytes_metrics_; - const bool enable_per_opcode_decoder_error_metrics_; ErrorBudgetResponseType errorBudgetDecision(const OpCodes opcode, const std::chrono::milliseconds latency) const; @@ -301,8 +270,7 @@ class ZooKeeperFilterConfig { private: void initOpCode(OpCodes opcode, Stats::Counter& resp_counter, Stats::Counter& resp_fast_counter, Stats::Counter& resp_slow_counter, Stats::Counter& rq_bytes_counter, - Stats::Counter& resp_bytes_counter, Stats::Counter& decoder_error_counter, - absl::string_view name); + Stats::Counter& resp_bytes_counter, absl::string_view name); ZooKeeperProxyStats generateStats(const std::string& prefix, Stats::Scope& scope) { return ZooKeeperProxyStats{ALL_ZOOKEEPER_PROXY_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; @@ -368,7 +336,7 @@ class ZooKeeperFilter : public Network::Filter, Network::FilterStatus onWrite(Buffer::Instance& data, bool end_stream) override; // ZooKeeperProxy::DecoderCallback - void onDecodeError(const absl::optional opcode) override; + void onDecodeError() override; void onRequestBytes(const absl::optional opcode, const uint64_t bytes) override; void onConnect(bool readonly) override; void onPing() override; @@ -381,8 +349,7 @@ class ZooKeeperFilter : public Network::Filter, void onExistsRequest(const std::string& path, bool watch) override; void onGetAclRequest(const std::string& path) override; void onSetAclRequest(const std::string& path, int32_t version) override; - absl::Status onSyncRequest(const absl::StatusOr& path, - const OpCodes opcode) override; + absl::Status onSyncRequest(const absl::StatusOr& path) override; void onCheckRequest(const std::string& path, int32_t version) override; void onMultiRequest() override; void onReconfigRequest() override; @@ -391,10 +358,8 @@ class ZooKeeperFilter : public Network::Filter, void onAddWatchRequest(const std::string& path, const int32_t mode) override; void onCheckWatchesRequest(const std::string& path, int32_t type) override; void onRemoveWatchesRequest(const std::string& path, int32_t type) override; - absl::Status onGetEphemeralsRequest(const absl::StatusOr& path, - const OpCodes opcode) override; - absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path, - const OpCodes opcode) override; + absl::Status onGetEphemeralsRequest(const absl::StatusOr& path) override; + absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path) override; void onCloseRequest() override; void onResponseBytes(const absl::optional opcode, const uint64_t bytes) override; void onConnectResponse(int32_t proto_version, int32_t timeout, bool readonly, diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index 59dd5292f114..76b43661dcfb 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -48,9 +48,9 @@ class BufferHelper : public Logger::Loggable { return status; \ } -#define EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status, opcode) \ +#define EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status) \ if (!status.ok()) { \ - callbacks_.onDecodeError(opcode); \ + callbacks_.onDecodeError(); \ return status; \ } @@ -59,9 +59,9 @@ class BufferHelper : public Logger::Loggable { return absl::InvalidArgumentError(message); \ } -#define EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, opcode, message) \ +#define EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ if (!status.ok()) { \ - callbacks_.onDecodeError(opcode); \ + callbacks_.onDecodeError(); \ return absl::InvalidArgumentError(message); \ } } // namespace ZooKeeperProxy diff --git a/test/extensions/filters/network/zookeeper_proxy/config_test.cc b/test/extensions/filters/network/zookeeper_proxy/config_test.cc index f20621bd3afe..30043aaa11c8 100644 --- a/test/extensions/filters/network/zookeeper_proxy/config_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/config_test.cc @@ -26,7 +26,6 @@ stat_prefix: test_prefix max_packet_bytes: 1048576 enable_per_opcode_request_bytes_metrics: true enable_per_opcode_response_bytes_metrics: true -enable_per_opcode_decoder_error_metrics: true enable_latency_threshold_metrics: true default_latency_threshold: "0.1s" latency_threshold_overrides:)EOF"; @@ -184,7 +183,6 @@ stat_prefix: test_prefix EXPECT_EQ(proto_config_.max_packet_bytes().value(), 0); EXPECT_EQ(proto_config_.enable_per_opcode_request_bytes_metrics(), false); EXPECT_EQ(proto_config_.enable_per_opcode_response_bytes_metrics(), false); - EXPECT_EQ(proto_config_.enable_per_opcode_decoder_error_metrics(), false); EXPECT_EQ(proto_config_.enable_latency_threshold_metrics(), false); EXPECT_EQ(proto_config_.default_latency_threshold(), ProtobufWkt::util::TimeUtil::SecondsToDuration(0)); @@ -206,7 +204,6 @@ default_latency_threshold: "0.15s" EXPECT_EQ(proto_config_.max_packet_bytes().value(), 0); EXPECT_EQ(proto_config_.enable_per_opcode_request_bytes_metrics(), false); EXPECT_EQ(proto_config_.enable_per_opcode_response_bytes_metrics(), false); - EXPECT_EQ(proto_config_.enable_per_opcode_decoder_error_metrics(), false); EXPECT_EQ(proto_config_.enable_latency_threshold_metrics(), false); EXPECT_EQ(proto_config_.default_latency_threshold(), ProtobufWkt::util::TimeUtil::MillisecondsToDuration(150)); @@ -230,7 +227,6 @@ stat_prefix: test_prefix EXPECT_EQ(proto_config_.max_packet_bytes().value(), 0); EXPECT_EQ(proto_config_.enable_per_opcode_request_bytes_metrics(), false); EXPECT_EQ(proto_config_.enable_per_opcode_response_bytes_metrics(), false); - EXPECT_EQ(proto_config_.enable_per_opcode_decoder_error_metrics(), false); EXPECT_EQ(proto_config_.enable_latency_threshold_metrics(), false); EXPECT_EQ(proto_config_.default_latency_threshold(), ProtobufWkt::util::TimeUtil::SecondsToDuration(0)); @@ -255,7 +251,6 @@ TEST_F(ZookeeperFilterConfigTest, FullConfig) { EXPECT_EQ(proto_config_.max_packet_bytes().value(), 1048576); EXPECT_EQ(proto_config_.enable_per_opcode_request_bytes_metrics(), true); EXPECT_EQ(proto_config_.enable_per_opcode_response_bytes_metrics(), true); - EXPECT_EQ(proto_config_.enable_per_opcode_decoder_error_metrics(), true); EXPECT_EQ(proto_config_.enable_latency_threshold_metrics(), true); EXPECT_EQ(proto_config_.default_latency_threshold(), ProtobufWkt::util::TimeUtil::MillisecondsToDuration(100)); diff --git a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc index 72f8bba27954..f22f9f14cee4 100644 --- a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc @@ -31,16 +31,14 @@ class ZooKeeperFilterTest : public testing::Test { void initialize( const bool enable_per_opcode_request_bytes_metrics = true, const bool enable_per_opcode_response_bytes_metrics = true, - const bool enable_per_opcode_decoder_error_metrics = true, const bool enable_latency_threshold_metrics = true, const std::chrono::milliseconds default_latency_threshold = std::chrono::milliseconds(100), const LatencyThresholdOverrideList& latency_threshold_overrides = LatencyThresholdOverrideList()) { config_ = std::make_shared( stat_prefix_, 1048576, enable_per_opcode_request_bytes_metrics, - enable_per_opcode_response_bytes_metrics, enable_per_opcode_decoder_error_metrics, - enable_latency_threshold_metrics, default_latency_threshold, latency_threshold_overrides, - scope_); + enable_per_opcode_response_bytes_metrics, enable_latency_threshold_metrics, + default_latency_threshold, latency_threshold_overrides, scope_); filter_ = std::make_unique(config_, time_system_); filter_->initializeReadFilterCallbacks(filter_callbacks_); } @@ -351,24 +349,6 @@ class ZooKeeperFilterTest : public testing::Test { return buffer; } - Buffer::OwnedImpl - encodeTooSmallCreateRequest(const std::string& path, const std::string& data, - const bool txn = false, const int32_t xid = 1000, - const int32_t opcode = enumToSignedInt(OpCodes::Create)) const { - Buffer::OwnedImpl buffer; - - if (!txn) { - buffer.writeBEInt(16 + path.length() + data.length()); - buffer.writeBEInt(xid); - buffer.writeBEInt(opcode); - } - - addString(buffer, path); - addString(buffer, data); - // Deliberately not adding acls and flags to the buffer and change the length accordingly. - return buffer; - } - Buffer::OwnedImpl encodeSetRequest(const std::string& path, const std::string& data, const int32_t version, const bool txn = false) const { Buffer::OwnedImpl buffer; @@ -576,9 +556,6 @@ class ZooKeeperFilterTest : public testing::Test { case OpCodes::CreateTtl: opname = "createttl"; break; - case OpCodes::Create2: - opname = "create2"; - break; default: break; } @@ -588,12 +565,26 @@ class ZooKeeperFilterTest : public testing::Test { {{"bytes", "35"}}}); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(1UL, store_.counter(absl::StrCat("test.zookeeper.", opname, "_rq")).value()); + + switch (opcode) { + case OpCodes::Create: + EXPECT_EQ(1UL, config_->stats().create_rq_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); + break; + case OpCodes::CreateContainer: + EXPECT_EQ(1UL, config_->stats().createcontainer_rq_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.createcontainer_rq_bytes").value()); + break; + case OpCodes::CreateTtl: + EXPECT_EQ(1UL, config_->stats().createttl_rq_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.createttl_rq_bytes").value()); + break; + default: + break; + } + EXPECT_EQ(35UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(35UL, store_.counter(absl::StrCat("test.zookeeper.", opname, "_rq_bytes")).value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, - store_.counter(absl::StrCat("test.zookeeper.", opname, "_decoder_error")).value()); } void testCreateWithNegativeDataLen(CreateFlags flag, const int32_t flag_val, @@ -610,9 +601,6 @@ class ZooKeeperFilterTest : public testing::Test { case OpCodes::CreateTtl: opname = "createttl"; break; - case OpCodes::Create2: - opname = "create2"; - break; default: break; } @@ -622,12 +610,24 @@ class ZooKeeperFilterTest : public testing::Test { {{"bytes", "32"}}}); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(1UL, store_.counter(absl::StrCat("test.zookeeper.", opname, "_rq")).value()); + + switch (opcode) { + case OpCodes::Create: + EXPECT_EQ(1UL, config_->stats().create_rq_.value()); + break; + case OpCodes::CreateContainer: + EXPECT_EQ(1UL, config_->stats().createcontainer_rq_.value()); + break; + case OpCodes::CreateTtl: + EXPECT_EQ(1UL, config_->stats().createttl_rq_.value()); + break; + default: + break; + } + EXPECT_EQ(32UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(32UL, store_.counter(absl::StrCat("test.zookeeper.", opname, "_rq_bytes")).value()); + EXPECT_EQ(32UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, - store_.counter(absl::StrCat("test.zookeeper.", opname, "_decoder_error")).value()); } void testRequest(Buffer::OwnedImpl& data, const std::vector& metadata_values) { @@ -653,8 +653,6 @@ class ZooKeeperFilterTest : public testing::Test { EXPECT_EQ(request_bytes, store_.counter(absl::StrCat("test.zookeeper.", opcode, "_rq_bytes")).value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, - store_.counter(absl::StrCat("test.zookeeper.", opcode, "_decoder_error")).value()); } void testControlRequest(Buffer::OwnedImpl& data, const std::vector& metadata_values, @@ -683,11 +681,11 @@ class ZooKeeperFilterTest : public testing::Test { expectSetDynamicMetadata(metadata_values); Buffer::OwnedImpl data = encodeResponseHeader(xid, zxid, 0); - std::string response = ""; + std::string resp = ""; for (const auto& metadata : metadata_values) { auto it = metadata.find("opname"); if (it != metadata.end()) { - response = it->second; + resp = it->second; } } @@ -695,19 +693,11 @@ class ZooKeeperFilterTest : public testing::Test { // However, its corresponding metric names have `_resp` suffix. std::string long_resp_suffix = "_response"; std::string short_resp_suffix = "_resp"; - std::string resp = response; - size_t pos = response.rfind(long_resp_suffix); + size_t pos = resp.rfind(long_resp_suffix); if (pos != std::string::npos) { resp.replace(pos, long_resp_suffix.length(), short_resp_suffix); } - // Fetch opcode by trimming the `_resp` suffix. - std::string opcode = resp; - pos = opcode.rfind(short_resp_suffix); - if (pos != std::string::npos) { - opcode.replace(pos, short_resp_suffix.length(), ""); - } - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onWrite(data, false)); EXPECT_EQ(1UL * response_count, store_.counter(absl::StrCat("test.zookeeper.", resp)).value()); EXPECT_EQ(1UL * response_count, @@ -717,9 +707,8 @@ class ZooKeeperFilterTest : public testing::Test { EXPECT_EQ(20UL * response_count, store_.counter(absl::StrCat("test.zookeeper.", resp, "_bytes")).value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, - store_.counter(absl::StrCat("test.zookeeper.", opcode, "_decoder_error")).value()); - const auto histogram_name = fmt::format("test.zookeeper.{}_latency", response); + const auto histogram_name = + fmt::format("test.zookeeper.{}_latency", metadata_values[0].find("opname")->second); EXPECT_NE(absl::nullopt, findHistogram(histogram_name)); } @@ -742,7 +731,7 @@ TEST_F(ZooKeeperFilterTest, DisableErrorBudgetCalculation) { std::chrono::milliseconds default_latency_threshold(200); LatencyThresholdOverrideList latency_threshold_overrides; - initialize(true, true, true, false, default_latency_threshold, latency_threshold_overrides); + initialize(true, true, false, default_latency_threshold, latency_threshold_overrides); EXPECT_EQ(config_->errorBudgetDecision(OpCodes::Connect, std::chrono::milliseconds(50)), ErrorBudgetResponseType::None); @@ -765,7 +754,7 @@ TEST_F(ZooKeeperFilterTest, ErrorBudgetDecisionWithDefaultLatencyThresholdConfig std::chrono::milliseconds default_latency_threshold(200); LatencyThresholdOverrideList latency_threshold_overrides; - initialize(true, true, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(true, true, true, default_latency_threshold, latency_threshold_overrides); EXPECT_EQ(config_->errorBudgetDecision(OpCodes::Connect, std::chrono::milliseconds(50)), ErrorBudgetResponseType::Fast); @@ -792,7 +781,7 @@ TEST_F(ZooKeeperFilterTest, ErrorBudgetDecisionWithMultiLatencyThresholdConfig) threshold_override->set_opcode(LatencyThresholdOverride::Multi); threshold_override->mutable_threshold()->set_nanos(200000000); // 200 milliseconds - initialize(true, true, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(true, true, true, default_latency_threshold, latency_threshold_overrides); EXPECT_EQ(config_->errorBudgetDecision(OpCodes::Connect, std::chrono::milliseconds(50)), ErrorBudgetResponseType::Fast); @@ -822,7 +811,7 @@ TEST_F(ZooKeeperFilterTest, ErrorBudgetDecisionWithDefaultAndOtherLatencyThresho threshold_override->set_opcode(LatencyThresholdOverride::Create); threshold_override->mutable_threshold()->set_nanos(200000000); // 200 milliseconds - initialize(true, true, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(true, true, true, default_latency_threshold, latency_threshold_overrides); EXPECT_EQ(config_->errorBudgetDecision(OpCodes::Connect, std::chrono::milliseconds(150)), ErrorBudgetResponseType::Fast); @@ -851,17 +840,16 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeRequestAndResponseBytesMetrics) { std::chrono::milliseconds default_latency_threshold(100); LatencyThresholdOverrideList latency_threshold_overrides; - initialize(false, false, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(false, false, true, default_latency_threshold, latency_threshold_overrides); Buffer::OwnedImpl data = encodeConnect(); expectSetDynamicMetadata({{{"opname", "connect"}}, {{"bytes", "32"}}}); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(1UL, config_->stats().connect_rq_.value()); + EXPECT_EQ(1UL, store_.counter("test.zookeeper.connect_rq").value()); EXPECT_EQ(32UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().connect_rq_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.connect_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); data = encodeConnectResponse(); expectSetDynamicMetadata({{{"opname", "connect_response"}, @@ -874,9 +862,8 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeRequestAndResponseBytesMetrics) { EXPECT_EQ(1UL, config_->stats().connect_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().connect_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.connect_response_latency")); } @@ -884,17 +871,16 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeRequestBytesMetrics) { std::chrono::milliseconds default_latency_threshold(100); LatencyThresholdOverrideList latency_threshold_overrides; - initialize(false, true, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(false, true, true, default_latency_threshold, latency_threshold_overrides); Buffer::OwnedImpl data = encodeConnect(); expectSetDynamicMetadata({{{"opname", "connect"}}, {{"bytes", "32"}}}); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(1UL, config_->stats().connect_rq_.value()); + EXPECT_EQ(1UL, store_.counter("test.zookeeper.connect_rq").value()); EXPECT_EQ(32UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().connect_rq_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.connect_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); data = encodeConnectResponse(); expectSetDynamicMetadata({{{"opname", "connect_response"}, @@ -907,9 +893,8 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeRequestBytesMetrics) { EXPECT_EQ(1UL, config_->stats().connect_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().connect_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.connect_response_latency")); } @@ -917,17 +902,16 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeResponseBytesMetrics) { std::chrono::milliseconds default_latency_threshold(100); LatencyThresholdOverrideList latency_threshold_overrides; - initialize(true, false, true, true, default_latency_threshold, latency_threshold_overrides); + initialize(true, false, true, default_latency_threshold, latency_threshold_overrides); Buffer::OwnedImpl data = encodeConnect(); expectSetDynamicMetadata({{{"opname", "connect"}}, {{"bytes", "32"}}}); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(1UL, config_->stats().connect_rq_.value()); + EXPECT_EQ(1UL, store_.counter("test.zookeeper.connect_rq").value()); EXPECT_EQ(32UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(32UL, config_->stats().connect_rq_bytes_.value()); + EXPECT_EQ(32UL, store_.counter("test.zookeeper.connect_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); data = encodeConnectResponse(); expectSetDynamicMetadata({{{"opname", "connect_response"}, @@ -940,9 +924,8 @@ TEST_F(ZooKeeperFilterTest, DisablePerOpcodeResponseBytesMetrics) { EXPECT_EQ(1UL, config_->stats().connect_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().connect_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.connect_response_latency")); } @@ -964,9 +947,8 @@ TEST_F(ZooKeeperFilterTest, Connect) { EXPECT_EQ(1UL, config_->stats().connect_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().connect_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.connect_response_latency")); } @@ -989,9 +971,8 @@ TEST_F(ZooKeeperFilterTest, ConnectReadonly) { EXPECT_EQ(1UL, config_->stats().connect_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().connect_resp_slow_.value()); EXPECT_EQ(25UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(25UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(25UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().connect_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.connect_response_latency")); } @@ -1127,39 +1108,14 @@ TEST_F(ZooKeeperFilterTest, CreateRequestTTLSequential) { } TEST_F(ZooKeeperFilterTest, CreateRequest2) { - testCreate(CreateFlags::Persistent, 0, OpCodes::Create2); - testResponse({{{"opname", "create2_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); -} - -TEST_F(ZooKeeperFilterTest, TooSmallCreateRequest) { initialize(); Buffer::OwnedImpl data = - encodeTooSmallCreateRequest("/foo", "bar", false, 1000, enumToSignedInt(OpCodes::Create)); - - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(0UL, config_->stats().create_rq_.value()); - EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().create_rq_bytes_.value()); - EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(1UL, config_->stats().create_decoder_error_.value()); -} - -TEST_F(ZooKeeperFilterTest, TooSmallCreateRequestWithDisabledPerOpcodeDecoderErrorMetrics) { - std::chrono::milliseconds default_latency_threshold(100); - LatencyThresholdOverrideList latency_threshold_overrides; - - initialize(true, true, false, true, default_latency_threshold, latency_threshold_overrides); - - Buffer::OwnedImpl data = - encodeTooSmallCreateRequest("/foo", "bar", false, 1000, enumToSignedInt(OpCodes::Create)); + encodeCreateRequest("/foo", "bar", 0, false, 1000, enumToSignedInt(OpCodes::Create2)); - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(0UL, config_->stats().create_rq_.value()); - EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().create_rq_bytes_.value()); - EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); + testRequest(data, {{{"opname", "create2"}, {"path", "/foo"}, {"create_type", "persistent"}}, + {{"bytes", "35"}}}); + testResponse({{{"opname", "create2_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } TEST_F(ZooKeeperFilterTest, SetRequest) { @@ -1273,9 +1229,8 @@ TEST_F(ZooKeeperFilterTest, CheckRequest) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().check_rq_.value()); EXPECT_EQ(24UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().check_rq_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.check_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().check_decoder_error_.value()); testResponse({{{"opname", "check_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } @@ -1305,13 +1260,12 @@ TEST_F(ZooKeeperFilterTest, MultiRequest) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().multi_rq_.value()); EXPECT_EQ(200UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(200UL, config_->stats().multi_rq_bytes_.value()); + EXPECT_EQ(200UL, store_.counter("test.zookeeper.multi_rq_bytes").value()); EXPECT_EQ(3UL, config_->stats().create_rq_.value()); EXPECT_EQ(1UL, config_->stats().setdata_rq_.value()); EXPECT_EQ(1UL, config_->stats().check_rq_.value()); EXPECT_EQ(2UL, config_->stats().delete_rq_.value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().multi_decoder_error_.value()); testResponse({{{"opname", "multi_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } @@ -1432,7 +1386,7 @@ TEST_F(ZooKeeperFilterTest, WatchEvent) { // WATCH_XID is generated by the server, it has no corresponding opcode. // Below expectation makes sure that WATCH_XID does not return the default opcode (which is // connect). - EXPECT_EQ(0UL, config_->stats().connect_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.connect_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); } @@ -1461,9 +1415,8 @@ TEST_F(ZooKeeperFilterTest, OneRequestWithMultipleOnDataCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(0UL, config_->stats().create_rq_.value()); EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1477,9 +1430,8 @@ TEST_F(ZooKeeperFilterTest, OneRequestWithMultipleOnDataCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().create_rq_.value()); EXPECT_EQ(35UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(35UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Response. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); @@ -1498,9 +1450,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithOneOnDataCall) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(2UL, config_->stats().create_rq_.value()); EXPECT_EQ(71UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(71UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(71UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); @@ -1520,9 +1471,8 @@ TEST_F(ZooKeeperFilterTest, MultipleControlRequestsWithOneOnDataCall) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(2UL, store_.counter("test.zookeeper.auth.digest_rq").value()); EXPECT_EQ(72UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(72UL, config_->stats().auth_rq_bytes_.value()); + EXPECT_EQ(72UL, store_.counter("test.zookeeper.auth_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().auth_decoder_error_.value()); // Responses. testResponse({{{"opname", "auth_response"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1544,11 +1494,9 @@ TEST_F(ZooKeeperFilterTest, MixedControlAndDataRequestsWithOneOnDataCall) { EXPECT_EQ(1UL, store_.counter("test.zookeeper.auth.digest_rq").value()); EXPECT_EQ(1UL, config_->stats().create_rq_.value()); EXPECT_EQ(71UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(36UL, config_->stats().auth_rq_bytes_.value()); - EXPECT_EQ(35UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(36UL, store_.counter("test.zookeeper.auth_rq_bytes").value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().auth_decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "auth_response"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1562,11 +1510,9 @@ TEST_F(ZooKeeperFilterTest, MixedControlAndDataRequestsWithOneOnDataCall) { EXPECT_EQ(1UL, config_->stats().create_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().create_resp_slow_.value()); EXPECT_EQ(40UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(20UL, config_->stats().auth_resp_bytes_.value()); - EXPECT_EQ(20UL, config_->stats().create_resp_bytes_.value()); + EXPECT_EQ(20UL, store_.counter("test.zookeeper.auth_resp_bytes").value()); + EXPECT_EQ(20UL, store_.counter("test.zookeeper.create_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().auth_decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); EXPECT_NE(absl::nullopt, findHistogram("test.zookeeper.create_resp_latency")); } @@ -1582,9 +1528,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(0UL, config_->stats().create_rq_.value()); EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1602,9 +1547,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().create_rq_.value()); EXPECT_EQ(35UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(35UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1620,9 +1564,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(2UL, config_->stats().create_rq_.value()); EXPECT_EQ(71UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(71UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(71UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1643,9 +1586,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls2) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(0UL, config_->stats().create_rq_.value()); EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1662,9 +1604,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls2) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(3UL, config_->stats().create_rq_.value()); EXPECT_EQ(108UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(108UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(108UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1690,9 +1631,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls3) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(2UL, config_->stats().create_rq_.value()); EXPECT_EQ(71UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(71UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(71UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1706,9 +1646,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls3) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(3UL, config_->stats().create_rq_.value()); EXPECT_EQ(108UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(108UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(108UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1734,9 +1673,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls4) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().create_rq_.value()); EXPECT_EQ(35UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(35UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1748,9 +1686,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls4) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(1UL, config_->stats().create_rq_.value()); EXPECT_EQ(35UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(35UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(35UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. data.drain(data.length()); @@ -1764,9 +1701,8 @@ TEST_F(ZooKeeperFilterTest, MultipleRequestsWithMultipleOnDataCalls4) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(3UL, config_->stats().create_rq_.value()); EXPECT_EQ(108UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(108UL, config_->stats().create_rq_bytes_.value()); + EXPECT_EQ(108UL, store_.counter("test.zookeeper.create_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().create_decoder_error_.value()); // Responses. testResponse({{{"opname", "create_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}, @@ -1788,9 +1724,8 @@ TEST_F(ZooKeeperFilterTest, OneResponseWithMultipleOnWriteCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(1UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(21UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(21UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(21UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponseWithPartialData(1000, 2000, 0); @@ -1800,9 +1735,8 @@ TEST_F(ZooKeeperFilterTest, OneResponseWithMultipleOnWriteCalls) { EXPECT_EQ(0UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(0UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -1815,9 +1749,8 @@ TEST_F(ZooKeeperFilterTest, OneResponseWithMultipleOnWriteCalls) { EXPECT_EQ(1UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } // |RESP1|RESP2| @@ -1832,9 +1765,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithOneOnWriteCall) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(2UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(42UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(42UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(42UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponse(1000, 2000, 0, "/foo"); @@ -1845,9 +1777,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithOneOnWriteCall) { EXPECT_EQ(2UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(48UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(48UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(48UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } // |RESP1 --------|RESP2 ------------| @@ -1863,9 +1794,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(2UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(42UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(42UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(42UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponseWithPartialData(1000, 2000, 0); @@ -1875,9 +1805,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls) { EXPECT_EQ(0UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(0UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -1894,9 +1823,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls) { EXPECT_EQ(1UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -1910,9 +1838,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls) { EXPECT_EQ(2UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(50UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(50UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(50UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } // |RESP1 ------|RESP2|RESP3| @@ -1930,9 +1857,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls2) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(3UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(63UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(63UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(63UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponseWithPartialData(1000, 2000, 0); @@ -1942,9 +1868,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls2) { EXPECT_EQ(0UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(0UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -1960,9 +1885,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls2) { EXPECT_EQ(3UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(72UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(72UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(72UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } // |RESP1|RESP2|RESP3 ---------| @@ -1980,9 +1904,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls3) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(3UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(63UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(63UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(63UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponse(1000, 2000, 0, "abcd"); @@ -1994,9 +1917,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls3) { EXPECT_EQ(2UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(48UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(48UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(48UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -2009,9 +1931,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls3) { EXPECT_EQ(3UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(72UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(72UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(72UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } // |RESP1|RESP2 ------------------|RESP3| @@ -2029,9 +1950,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls4) { EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(rq_data, false)); EXPECT_EQ(3UL, config_->stats().getdata_rq_.value()); EXPECT_EQ(63UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(63UL, config_->stats().getdata_rq_bytes_.value()); + EXPECT_EQ(63UL, store_.counter("test.zookeeper.getdata_rq_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Response (onWrite1). Buffer::OwnedImpl resp_data = encodeResponse(1000, 2000, 0, "abcd"); @@ -2042,9 +1962,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls4) { EXPECT_EQ(1UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -2057,9 +1976,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls4) { EXPECT_EQ(1UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(24UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(24UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(24UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); // Mock the buffer is drained by the tcp_proxy filter. resp_data.drain(resp_data.length()); @@ -2074,9 +1992,8 @@ TEST_F(ZooKeeperFilterTest, MultipleResponsesWithMultipleOnWriteCalls4) { EXPECT_EQ(3UL, config_->stats().getdata_resp_fast_.value()); EXPECT_EQ(0UL, config_->stats().getdata_resp_slow_.value()); EXPECT_EQ(72UL, config_->stats().response_bytes_.value()); - EXPECT_EQ(72UL, config_->stats().getdata_resp_bytes_.value()); + EXPECT_EQ(72UL, store_.counter("test.zookeeper.getdata_resp_bytes").value()); EXPECT_EQ(0UL, config_->stats().decoder_error_.value()); - EXPECT_EQ(0UL, config_->stats().getdata_decoder_error_.value()); } } // namespace ZooKeeperProxy