From 3287c8b4b6b6dc35f166e64e314e505a430765b8 Mon Sep 17 00:00:00 2001 From: wbpcode Date: Tue, 13 Aug 2024 14:27:54 +0800 Subject: [PATCH 1/2] remove senseless dynamic config to simplify code base Signed-off-by: wbpcode --- source/BUILD | 4 -- source/cds_impl.cc | 86 ----------------------- source/cds_impl.h | 103 ---------------------------- source/dynamic_config.cc | 93 ------------------------- source/dynamic_config.h | 45 ------------ source/tracer_impl.cc | 24 ------- source/tracer_impl.h | 6 +- test/dynamic_config_test.cc | 133 ------------------------------------ 8 files changed, 1 insertion(+), 493 deletions(-) delete mode 100644 source/cds_impl.cc delete mode 100644 source/cds_impl.h delete mode 100644 source/dynamic_config.cc delete mode 100644 source/dynamic_config.h delete mode 100644 test/dynamic_config_test.cc diff --git a/source/BUILD b/source/BUILD index a30fb3c..c45126e 100644 --- a/source/BUILD +++ b/source/BUILD @@ -3,16 +3,12 @@ load("@rules_cc//cc:defs.bzl", "cc_library") cc_library( name = "cpp2sky_lib", srcs = [ - "cds_impl.cc", - "dynamic_config.cc", "grpc_async_client_impl.cc", "propagation_impl.cc", "tracer_impl.cc", "tracing_context_impl.cc", ], hdrs = [ - "cds_impl.h", - "dynamic_config.h", "grpc_async_client_impl.h", "propagation_impl.h", "tracer_impl.h", diff --git a/source/cds_impl.cc b/source/cds_impl.cc deleted file mode 100644 index dda7f7c..0000000 --- a/source/cds_impl.cc +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2021 SkyAPM - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 - -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "cds_impl.h" - -#include - -#include "spdlog/spdlog.h" - -namespace cpp2sky { - -using namespace spdlog; - -GrpcAsyncConfigDiscoveryServiceClient::GrpcAsyncConfigDiscoveryServiceClient( - const std::string& address, grpc::CompletionQueue& cq, - UnaryStreamBuilderPtr builder, - std::shared_ptr cred) - : builder_(std::move(builder)), - cq_(cq), - stub_(grpc::CreateChannel(address, cred)) {} - -GrpcAsyncConfigDiscoveryServiceClient:: - ~GrpcAsyncConfigDiscoveryServiceClient() { - resetStream(); -} - -void GrpcAsyncConfigDiscoveryServiceClient::sendMessage(CdsRequest request) { - resetStream(); - stream_ = builder_->create(*this, request); - info("[CDS] Stream {} had created", fmt::ptr(stream_.get())); -} - -void GrpcAsyncConfigDiscoveryServiceClient::resetStream() { - if (stream_) { - info("[CDS] Stream {} has destroyed", fmt::ptr(this)); - stream_.reset(); - } -} - -GrpcAsyncConfigDiscoveryServiceStream::GrpcAsyncConfigDiscoveryServiceStream( - AsyncClient& parent, CdsRequest request, - DynamicConfig& config) - : client_(parent), config_(config) { - sendMessage(request); -} - -void GrpcAsyncConfigDiscoveryServiceStream::sendMessage(CdsRequest request) { - response_reader_ = client_.stub().PrepareUnaryCall( - &ctx_, "/skywalking.v3.ConfigurationDiscoveryService/fetchConfigurations", - request, &client_.completionQueue()); - response_reader_->StartCall(); - response_reader_->Finish(&commands_, &status_, - reinterpret_cast(&read_done_)); -} - -void GrpcAsyncConfigDiscoveryServiceStream::onReadDone() { - info("[CDS] Stream {} read finished with gRPC status {}", fmt::ptr(this), - static_cast(status_.error_code())); - - if (status_.ok()) { - config_.onConfigChange(commands_); - } - - // Stream which finished to read done won't be destroyed here. - // But it will be destroyed when new stream created. -} - -AsyncStreamSharedPtr -GrpcAsyncConfigDiscoveryServiceStreamBuilder::create( - AsyncClient& client, CdsRequest request) { - return std::make_shared( - client, request, config_); -} - -} // namespace cpp2sky diff --git a/source/cds_impl.h b/source/cds_impl.h deleted file mode 100644 index 2611e41..0000000 --- a/source/cds_impl.h +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright 2021 SkyAPM - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 - -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#pragma once - -#include "cpp2sky/internal/async_client.h" -#include "cpp2sky/internal/stream_builder.h" -#include "language-agent/ConfigurationDiscoveryService.grpc.pb.h" -#include "language-agent/ConfigurationDiscoveryService.pb.h" -#include "source/dynamic_config.h" - -namespace cpp2sky { - -using CdsRequest = skywalking::v3::ConfigurationSyncRequest; -using CdsResponse = skywalking::v3::Commands; - -class GrpcAsyncConfigDiscoveryServiceStream; - -class GrpcAsyncConfigDiscoveryServiceClient final - : public AsyncClient { - public: - explicit GrpcAsyncConfigDiscoveryServiceClient( - const std::string& address, grpc::CompletionQueue& cq, - UnaryStreamBuilderPtr builder, - std::shared_ptr cred); - ~GrpcAsyncConfigDiscoveryServiceClient(); - - void sendMessage(CdsRequest request) override; - void startStream() override {} - CircularBuffer& pendingMessages() override { assert(false); } - grpc::CompletionQueue& completionQueue() override { return cq_; } - grpc::TemplatedGenericStub& stub() override { - return stub_; - } - - private: - void resetStream(); - - std::string address_; - UnaryStreamBuilderPtr builder_; - grpc::CompletionQueue& cq_; - grpc::TemplatedGenericStub stub_; - AsyncStreamSharedPtr stream_; -}; - -class GrpcAsyncConfigDiscoveryServiceStream final - : public AsyncStream, - public AsyncStreamCallback { - public: - explicit GrpcAsyncConfigDiscoveryServiceStream( - AsyncClient& parent, CdsRequest request, - DynamicConfig& config); - - // AsyncStream - void sendMessage(CdsRequest request) override; - - // AsyncStreamCallback - void onReady() override {} - void onIdle() override {} - void onWriteDone() override {} - void onReadDone() override; - void onStreamFinish() override { delete this; } - - private: - AsyncClient& client_; - std::unique_ptr> - response_reader_; - CdsRequest request_; - CdsResponse commands_; - grpc::Status status_; - grpc::ClientContext ctx_; - DynamicConfig& config_; - - StreamCallbackTag read_done_{StreamState::ReadDone, this}; -}; - -class GrpcAsyncConfigDiscoveryServiceStreamBuilder final - : public UnaryStreamBuilder { - public: - explicit GrpcAsyncConfigDiscoveryServiceStreamBuilder(DynamicConfig& config) - : config_(config) {} - - // AsyncStreamFactory - AsyncStreamSharedPtr create( - AsyncClient& client, - CdsRequest request) override; - - private: - DynamicConfig& config_; -}; - -} // namespace cpp2sky diff --git a/source/dynamic_config.cc b/source/dynamic_config.cc deleted file mode 100644 index 7c9903a..0000000 --- a/source/dynamic_config.cc +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2021 SkyAPM - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 - -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dynamic_config.h" - -#include - -#include "absl/strings/str_split.h" -#include "absl/strings/strip.h" -#include "spdlog/spdlog.h" - -namespace cpp2sky { - -namespace { // well known fields on response commands. -static constexpr absl::string_view UUID_FIELD = "UUID"; -static constexpr absl::string_view SERIAL_NUMBER_FIELD = "SerialNumber"; -static constexpr absl::string_view IGNORE_SUFFIX = "ignore_suffix"; -} // namespace - -using namespace spdlog; - -DynamicConfig::DynamicConfig(TracerConfig& config) : config_(config) { - target_fields_.emplace(IGNORE_SUFFIX.data()); - - ignore_fields_.emplace(UUID_FIELD.data()); - ignore_fields_.emplace(SERIAL_NUMBER_FIELD.data()); -} - -void DynamicConfig::onConfigChange(skywalking::v3::Commands commands) { - if (commands.commands_size() <= 0) { - return; - } - const auto top_command = commands.commands(0); - - std::string uuid; - for (const auto& target : top_command.args()) { - if (target.key() == UUID_FIELD.data()) { - uuid = target.value(); - } - } - - if (uuid.empty() || uuid_ == uuid) { - info("UUID not changed changed from {}", uuid_); - return; - } - - std::unique_lock lck(mux_); - bool config_changed = false; - for (const auto& target : top_command.args()) { - if (ignore_fields_.find(target.key()) != ignore_fields_.end() || - target_fields_.find(target.key()) == target_fields_.end()) { - continue; - } - - if (target.key() == IGNORE_SUFFIX) { - std::string will_destroy_suffixes; - for (const auto& current_suffix : - config_.ignore_operation_name_suffix()) { - will_destroy_suffixes += current_suffix + ","; - } - will_destroy_suffixes = - std::string(absl::StripSuffix(will_destroy_suffixes, ",")); - - config_.clear_ignore_operation_name_suffix(); - - for (const auto& next_suffix : absl::StrSplit(target.value(), ",")) { - *config_.add_ignore_operation_name_suffix() = std::string(next_suffix); - } - - info("{} updated from {} to {}", IGNORE_SUFFIX.data(), - will_destroy_suffixes, target.value()); - } - - config_changed = true; - } - - if (config_changed) { - uuid_ = uuid; - } -} - -} // namespace cpp2sky diff --git a/source/dynamic_config.h b/source/dynamic_config.h deleted file mode 100644 index 215673d..0000000 --- a/source/dynamic_config.h +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2021 SkyAPM - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 - -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#pragma once - -#include -#include - -#include "absl/strings/string_view.h" -#include "cpp2sky/config.pb.h" -#include "language-agent/ConfigurationDiscoveryService.pb.h" - -namespace cpp2sky { - -class DynamicConfig { - public: - DynamicConfig(TracerConfig& config); - - void onConfigChange(skywalking::v3::Commands commands); - const TracerConfig& tracerConfig() const& { - std::unique_lock lck(mux_); - return config_; - } - const std::string& uuid() const { return uuid_; } - - private: - mutable std::mutex mux_; - TracerConfig& config_; - std::string uuid_; - std::set target_fields_; - std::set ignore_fields_; -}; - -} // namespace cpp2sky diff --git a/source/tracer_impl.cc b/source/tracer_impl.cc index 524d5e6..6957f30 100644 --- a/source/tracer_impl.cc +++ b/source/tracer_impl.cc @@ -17,7 +17,6 @@ #include #include -#include "cds_impl.h" #include "cpp2sky/exception.h" #include "language-agent/ConfigurationDiscoveryService.pb.h" #include "matchers/suffix_matcher.h" @@ -45,7 +44,6 @@ TracerImpl::TracerImpl( TracerImpl::~TracerImpl() { reporter_client_.reset(); - cds_client_.reset(); cq_.Shutdown(); evloop_thread_.join(); } @@ -78,11 +76,6 @@ void TracerImpl::run() { void* got_tag; bool ok = false; while (true) { - // TODO(shikugawa): cleanup evloop handler. - if (cds_timer_ != nullptr && cds_timer_->check()) { - cdsRequest(); - } - grpc::CompletionQueue::NextStatus status = cq_.AsyncNext( &got_tag, &ok, gpr_time_from_nanos(0, GPR_CLOCK_REALTIME)); switch (status) { @@ -97,13 +90,6 @@ void TracerImpl::run() { } } -void TracerImpl::cdsRequest() { - skywalking::v3::ConfigurationSyncRequest request; - request.set_service(config_.tracerConfig().service_name()); - request.set_uuid(config_.uuid()); - cds_client_->sendMessage(request); -} - void TracerImpl::init(TracerConfig& config, std::shared_ptr cred) { spdlog::set_level(spdlog::level::warn); @@ -123,16 +109,6 @@ void TracerImpl::init(TracerConfig& config, op_name_matchers_.emplace_back(absl::make_unique( std::vector(config.ignore_operation_name_suffix().begin(), config.ignore_operation_name_suffix().end()))); - - if (config_.tracerConfig().cds_request_interval() != 0) { - cds_client_ = absl::make_unique( - config.address(), cq_, - absl::make_unique( - config_), - cred); - cds_timer_ = - absl::make_unique(config_.tracerConfig().cds_request_interval()); - } } TracerPtr createInsecureGrpcTracer(TracerConfig& cfg) { diff --git a/source/tracer_impl.h b/source/tracer_impl.h index e394d69..4f63f1f 100644 --- a/source/tracer_impl.h +++ b/source/tracer_impl.h @@ -20,7 +20,6 @@ #include "cpp2sky/internal/matcher.h" #include "cpp2sky/tracer.h" #include "language-agent/ConfigurationDiscoveryService.pb.h" -#include "source/cds_impl.h" #include "source/grpc_async_client_impl.h" #include "source/tracing_context_impl.h" #include "source/utils/timer.h" @@ -51,12 +50,9 @@ class TracerImpl : public Tracer { void init(TracerConfig& config, std::shared_ptr cred); void run(); - void cdsRequest(); - std::unique_ptr cds_timer_; - DynamicConfig config_; + TracerConfig config_; AsyncClientPtr reporter_client_; - AsyncClientPtr cds_client_; grpc::CompletionQueue cq_; std::thread evloop_thread_; TracingContextFactory segment_factory_; diff --git a/test/dynamic_config_test.cc b/test/dynamic_config_test.cc deleted file mode 100644 index 8486a27..0000000 --- a/test/dynamic_config_test.cc +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2020 SkyAPM - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 - -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "source/dynamic_config.h" - -#include -#include - -#include "absl/memory/memory.h" -#include "common/Common.pb.h" - -namespace cpp2sky { - -using testing::_; - -skywalking::v3::Commands setCommands(std::string uuid_value, - std::string suffixes) { - skywalking::v3::Commands commands; - auto* command = commands.add_commands(); - auto* uuid = command->add_args(); - uuid->set_key("UUID"); - uuid->set_value(uuid_value); - auto* suffix = command->add_args(); - suffix->set_key("ignore_suffix"); - suffix->set_value(suffixes); - return commands; -} - -class DynamicConfigTest : public testing::Test { - public: - void setup() { - init_cfg_.set_service_name("mesh"); - init_cfg_.set_instance_name("instance_default"); - - dcfg_ = absl::make_unique(init_cfg_); - } - - void update(skywalking::v3::Commands& commands) { - prev_cfg_ = dcfg_->tracerConfig(); - prev_uuid_ = dcfg_->uuid(); - dcfg_->onConfigChange(commands); - } - - void shouldChanged() { - EXPECT_NE(dcfg_->tracerConfig().DebugString(), prev_cfg_.DebugString()); - EXPECT_NE(dcfg_->uuid(), prev_uuid_); - } - - void shouldNotChanged() { - EXPECT_EQ(dcfg_->tracerConfig().DebugString(), prev_cfg_.DebugString()); - EXPECT_EQ(dcfg_->uuid(), prev_uuid_); - } - - TracerConfig init_cfg_; - TracerConfig prev_cfg_; - std::string prev_uuid_; - std::unique_ptr dcfg_; -}; - -TEST_F(DynamicConfigTest, ConfigChange) { - setup(); - - skywalking::v3::Commands commands1 = setCommands("uuid", "ignore1"); - update(commands1); - shouldChanged(); - - // config not changed - update(commands1); - shouldNotChanged(); - - // Config updated - skywalking::v3::Commands commands2 = setCommands("uuid2", "ignore2"); - update(commands2); - shouldChanged(); - - // UUID not set - { - skywalking::v3::Commands commands3; - auto* command = commands3.add_commands(); - auto* suffixes = command->add_args(); - suffixes->set_key("ignore_suffix"); - suffixes->set_value("ignore3"); - - update(commands3); - shouldNotChanged(); - } - - // UUID updated but suffixes name doesn't changed with unknown field - { - skywalking::v3::Commands commands4; - auto* command = commands4.add_commands(); - auto* uuid = command->add_args(); - uuid->set_key("UUID"); - uuid->set_value("uuid3"); - auto* unknown = command->add_args(); - unknown->set_key("unknown"); - unknown->set_value("suffixes-updated-3"); - - update(commands4); - shouldNotChanged(); - } - - // UUID upgraded and suffixes name changed, with unknown field addition - { - skywalking::v3::Commands commands5; - auto* command = commands5.add_commands(); - auto* uuid = command->add_args(); - uuid->set_key("UUID"); - uuid->set_value("uuid4"); - auto* unknown = command->add_args(); - unknown->set_key("unknown"); - unknown->set_value("suffixes-updated-3"); - auto* suffixes = command->add_args(); - suffixes->set_key("ignore_suffix"); - suffixes->set_value("ignore-3"); - - update(commands5); - shouldChanged(); - } -} - -} // namespace cpp2sky From c7279ca9948e6519f670304014341ade63134a78 Mon Sep 17 00:00:00 2001 From: wbpcode Date: Tue, 13 Aug 2024 14:40:51 +0800 Subject: [PATCH 2/2] fix ci Signed-off-by: wbpcode --- .github/workflows/main.yml | 10 +++++----- test/BUILD | 14 -------------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 01b2dcd..efef18d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -40,12 +40,12 @@ jobs: - name: Install cmake dependencies and run cmake compile run: | sudo apt update - sudo apt -y install cmake + sudo apt -y install cmake sudo git clone -b v9.1.0 https://github.com/apache/skywalking-data-collect-protocol.git ./3rdparty/skywalking-data-collect-protocol sudo git clone -b v1.46.6 https://github.com/grpc/grpc.git --recursive - sudo cmake -S ./grpc -B ./grpc/build + sudo cmake -S ./grpc -B ./grpc/build sudo cmake --build ./grpc/build --parallel 8 --target install - sudo cmake -S . -B ./build + sudo cmake -S . -B ./build sudo cmake --build ./build - name: Install lcov and genhtml and link llvm run: | @@ -70,7 +70,7 @@ jobs: - uses: actions/checkout@v3 - name: Prepare service container run: | - docker-compose -f test/e2e/docker/docker-compose.e2e.yml up -d + docker compose -f test/e2e/docker/docker-compose.e2e.yml up -d - name: Run e2e run: | pip3 install --upgrade pip @@ -84,7 +84,7 @@ jobs: - uses: actions/checkout@v3 - name: Prepare service container run: | - docker-compose -f test/e2e/docker/docker-compose.e2e-python.yml up -d + docker compose -f test/e2e/docker/docker-compose.e2e-python.yml up -d - name: Run e2e run: | pip3 install --upgrade pip diff --git a/test/BUILD b/test/BUILD index eb9b7ff..1a3c133 100644 --- a/test/BUILD +++ b/test/BUILD @@ -68,20 +68,6 @@ cc_test( ], ) -cc_test( - name = "dynamic_config_test", - srcs = [ - "dynamic_config_test.cc", - ], - visibility = ["//visibility:public"], - deps = [ - ":mocks", - "//source:cpp2sky_lib", - "@com_google_absl//absl/memory", - "@com_google_googletest//:gtest_main", - ], -) - cc_test( name = "tracer_test", srcs = [