ignore_suffix as skywalking CDS target (#66)

Author: Shikugawa

README.md

@@ -136,19 +136,27 @@ tracer->report(std::move(tracing_context));
 #### Skywalking CDS
 
 C++ agent implements Skywalking CDS feature it allows to change bootstrap config dynamically from the response of sync request, invoked from this periodically.
+Dynamically configurable values are described in description of properties on `docs/README.md'.
 
 ```cpp
 TracerConfig config;
 // If you set this value as zero, CDS request won't occur.
 config.set_cds_request_interval(5); // CDS request interval should be 5sec
 ```
 
-Currently, Configurable values dynamically are like below.
+If you are using Consul KVS as backend, we could put configuration value through HTTP request.
 
-| Config Key | Value Description |
-|:----:|:----:|
-|instance_name| Instance name of this agent |
+```yaml
+configurations:
+  service_name:
+    ignore_suffix: '/ignore, /hoge'
+```
+
+After setup configurations, try to put values with 
 
+```
+curl --request PUT --data-binary "@./config.yaml" http://localhost:8500/v1/kv/configuration-discovery.default.agentConfigurations
+```
 
 ## Security
 

cpp2sky/config.proto

@@ -9,27 +9,35 @@ enum Protocol {
 
 message TracerConfig {
   // Tracer protocol.
+  // | Static config only
   Protocol protocol = 1;
 
   // Service name
+  // | Static config only
   string service_name = 2;
 
   // Instance name
+  // | Static config only
   string instance_name = 3;
 
   // OAP address.
+  // | Static config only
   string address = 4;
 
   // OAP token.
+  // | Static config only
   string token = 5;
 
   // The size of buffer it stores pending messages.
+  // | Static config only
   uint32 delayed_buffer_size = 6;
 
   // If the operation name of the first span is included in this set, 
   // this segment should be ignored.
+  // | This value can be changed with SkyWalking CDS.
   repeated string ignore_operation_name_suffix = 8;
 
   // CDS sync request interval. If this value is zero, CDS feature will be disabled.
+  // | Static config only
   uint32 cds_request_interval = 7;
 }

docs/README.md

@@ -5,14 +5,14 @@
 
 | Field | Description | Type | C++ |
 | ----- | ----------- | ---- | --- |
-| protocol | Tracer protocol. | Protocol | |
-| service_name | Service name | string | string |
-| instance_name | Instance name | string | string |
-| address | OAP address. | string | string |
-| token | OAP token. | string | string |
-| delayed_buffer_size | The size of buffer it stores pending messages. | uint32 | uint32 |
-| ignore_operation_name_suffix | If the operation name of the first span is included in this set, this segment should be ignored. | (slice of) string | (slice of) string |
-| cds_request_interval | CDS sync request interval. If this value is zero, CDS feature will be disabled. | uint32 | uint32 |
+| protocol | Tracer protocol. | Static config only | Protocol | |
+| service_name | Service name | Static config only | string | string |
+| instance_name | Instance name | Static config only | string | string |
+| address | OAP address. | Static config only | string | string |
+| token | OAP token. | Static config only | string | string |
+| delayed_buffer_size | The size of buffer it stores pending messages. | Static config only | uint32 | uint32 |
+| ignore_operation_name_suffix | If the operation name of the first span is included in this set, this segment should be ignored. | This value can be changed with SkyWalking CDS. | (slice of) string | (slice of) string |
+| cds_request_interval | CDS sync request interval. If this value is zero, CDS feature will be disabled. | Static config only | uint32 | uint32 |
 
 
 

source/BUILD

@@ -25,6 +25,7 @@ cc_library(
     "@skywalking_data_collect_protocol//language-agent:configuration_discovery_service_cc_grpc",
     "@com_github_grpc_grpc//:grpc++",
     "@com_github_gabime_spdlog//:spdlog",
+    "@com_github_abseil-cpp//absl/strings:strings",
     "//cpp2sky/internal:async_client_interface",
     "//cpp2sky/internal:stream_builder_interface",
     "//cpp2sky/internal:matcher_interface",
@@ -50,6 +51,7 @@ cc_library(
     "@skywalking_data_collect_protocol//language-agent:tracing_protocol_cc_grpc",
     "@com_github_grpc_grpc//:grpc++",
     "@com_github_gabime_spdlog//:spdlog",
+    "@com_github_abseil-cpp//absl/strings:strings",
     "//cpp2sky/internal:async_client_interface",
     "//cpp2sky/internal:stream_builder_interface",
     "//cpp2sky/internal:matcher_interface",
@@ -75,6 +77,7 @@ cc_library(
   deps = [
     "@skywalking_data_collect_protocol//language-agent:configuration_discovery_service_cc_proto",
     "@skywalking_data_collect_protocol//language-agent:tracing_protocol_cc_proto",
+    "@com_github_abseil-cpp//absl/strings:strings",
     "//cpp2sky:config_cc_proto",
     "//cpp2sky:cpp2sky_interface",
     "//source/utils:util_lib",

source/dynamic_config.cc

@@ -14,20 +14,24 @@
 
 #include "dynamic_config.h"
 
+#include <string>
+
+#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 std::string_view UUID_FIELD = "UUID";
 static constexpr std::string_view SERIAL_NUMBER_FIELD = "SerialNumber";
-static constexpr std::string_view INSTANCE_FIELD = "instance_name";
+static constexpr std::string_view IGNORE_SUFFIX = "ignore_suffix";
 }  // namespace
 
 using namespace spdlog;
 
 DynamicConfig::DynamicConfig(TracerConfig& config) : config_(config) {
-  target_fields_.emplace(INSTANCE_FIELD.data());
+  target_fields_.emplace(IGNORE_SUFFIX.data());
 
   ignore_fields_.emplace(UUID_FIELD.data());
   ignore_fields_.emplace(SERIAL_NUMBER_FIELD.data());
@@ -59,13 +63,24 @@ void DynamicConfig::onConfigChange(skywalking::v3::Commands commands) {
       continue;
     }
 
-    const auto* reflection = config_.GetReflection();
-    info("{} updated from {} to {}", INSTANCE_FIELD.data(),
-         config_.instance_name(), target.value());
-    reflection->SetString(
-        static_cast<google::protobuf::Message*>(&config_),
-        config_.GetDescriptor()->FindFieldByName(INSTANCE_FIELD.data()),
-        target.value());
+    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 = 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() = next_suffix;
+      }
+
+      info("{} updated from {} to {}", IGNORE_SUFFIX.data(),
+           will_destroy_suffixes, target.value());
+    }
+
     config_changed = true;
   }
 

test/cds/config.yml

@@ -24,15 +24,15 @@ namespace cpp2sky {
 using testing::_;
 
 skywalking::v3::Commands setCommands(std::string uuid_value,
-                                     std::string instance_name) {
+                                     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* instance = command->add_args();
-  instance->set_key("instance_name");
-  instance->set_value(instance_name);
+  auto* suffix = command->add_args();
+  suffix->set_key("ignore_suffix");
+  suffix->set_value(suffixes);
   return commands;
 }
 
@@ -70,7 +70,7 @@ class DynamicConfigTest : public testing::Test {
 TEST_F(DynamicConfigTest, ConfigChange) {
   setup();
 
-  skywalking::v3::Commands commands1 = setCommands("uuid", "instance-updated");
+  skywalking::v3::Commands commands1 = setCommands("uuid", "ignore1");
   update(commands1);
   shouldChanged();
 
@@ -79,24 +79,23 @@ TEST_F(DynamicConfigTest, ConfigChange) {
   shouldNotChanged();
 
   // Config updated
-  skywalking::v3::Commands commands2 =
-      setCommands("uuid2", "instance-updated-2");
+  skywalking::v3::Commands commands2 = setCommands("uuid2", "ignore2");
   update(commands2);
   shouldChanged();
 
   // UUID not set
   {
     skywalking::v3::Commands commands3;
     auto* command = commands3.add_commands();
-    auto* instance = command->add_args();
-    instance->set_key("instance_name");
-    instance->set_value("instance-updated-3");
+    auto* suffixes = command->add_args();
+    suffixes->set_key("ignore_suffix");
+    suffixes->set_value("ignore3");
 
     update(commands3);
     shouldNotChanged();
   }
 
-  // UUID updated but instance name doesn't changed with unknown field
+  // UUID updated but suffixes name doesn't changed with unknown field
   {
     skywalking::v3::Commands commands4;
     auto* command = commands4.add_commands();
@@ -105,13 +104,13 @@ TEST_F(DynamicConfigTest, ConfigChange) {
     uuid->set_value("uuid3");
     auto* unknown = command->add_args();
     unknown->set_key("unknown");
-    unknown->set_value("instance-updated-3");
+    unknown->set_value("suffixes-updated-3");
 
     update(commands4);
     shouldNotChanged();
   }
 
-  // UUID upgraded and instance name changed, with unknown field addition
+  // UUID upgraded and suffixes name changed, with unknown field addition
   {
     skywalking::v3::Commands commands5;
     auto* command = commands5.add_commands();
@@ -120,10 +119,10 @@ TEST_F(DynamicConfigTest, ConfigChange) {
     uuid->set_value("uuid4");
     auto* unknown = command->add_args();
     unknown->set_key("unknown");
-    unknown->set_value("instance-updated-3");
-    auto* instance = command->add_args();
-    instance->set_key("instance_name");
-    instance->set_value("instance-updated-3");
+    unknown->set_value("suffixes-updated-3");
+    auto* suffixes = command->add_args();
+    suffixes->set_key("ignore_suffix");
+    suffixes->set_value("ignore-3");
 
     update(commands5);
     shouldChanged();