PR comments applied

dynatrace-oss · Oct 25, 2023 · 662f7b6 · 662f7b6
1 parent a7569bc
commit 662f7b6
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 31 deletions.
diff --git a/ebpfdiscoverysrv/src/main.cpp b/ebpfdiscoverysrv/src/main.cpp
@@ -41,7 +41,7 @@ static po::options_description getProgramOptions() {
       ("log-dir", po::value<std::filesystem::path>()->default_value(""), "Log files directory")
       ("log-no-stdout", po::value<bool>()->default_value(false), "Disable logging to stdout")
       ("version", "Display program version")
-	  ("interval", po::value<int>()->default_value(60), "Services reporting time interval (in seconds)")
+      ("interval", po::value<int>()->default_value(60), "Services reporting time interval (in seconds)")
   ;
 	// clang-format on
 
@@ -112,11 +112,11 @@ static void initLibbpf() {
 }
 
 void servicesProvidingLoop(ebpfdiscovery::Discovery& discoveryInstance, std::chrono::seconds interval) {
-	while (true) {
-		if (auto services = discoveryInstance.getServices(); !services.empty()) {
-			auto servicesProto = proto::Translator::internalToProto(services);
+	while (programStatus == ProgramStatus::Running) {
+		if (auto services = discoveryInstance.popServices(); !services.empty()) {
+			auto servicesProto = proto::internalToProto(services);
 			LOG_DEBUG("Services list:\n{}\n", servicesProto.DebugString());
-			auto servicesJson = proto::Translator::protoToJson(servicesProto);
+			auto servicesJson = proto::protoToJson(servicesProto);
 			std::cout << servicesJson << std::endl;
 		}
 		std::this_thread::sleep_for(interval);
@@ -193,6 +193,9 @@ int main(int argc, char** argv) {
 	if (unixSignalThread.joinable()) {
 		unixSignalThread.join();
 	}
+	if (servicesProvider.joinable()) {
+		servicesProvider.join();
+	}
 	instance.stop();
 	instance.wait();
 

diff --git a/libebpfdiscovery/headers/ebpfdiscovery/Discovery.h b/libebpfdiscovery/headers/ebpfdiscovery/Discovery.h
@@ -36,7 +36,7 @@ class Discovery {
 	void stop();
 	void wait();
 
-	std::vector<service::Service> getServices();
+	std::vector<service::Service> popServices();
 
 private:
 	using SavedSessionsCacheType = LRUCache<DiscoverySavedSessionKey, Session, DiscoverySavedSessionKeyHash>;

diff --git a/libebpfdiscovery/src/Discovery.cpp b/libebpfdiscovery/src/Discovery.cpp
@@ -177,8 +177,8 @@ void Discovery::handleNewRequest(const Session& session, const DiscoverySessionM
 	serviceAggregator.newRequest(request, meta);
 }
 
-std::vector<service::Service> Discovery::getServices() {
-	return serviceAggregator.getServices();
+std::vector<service::Service> Discovery::popServices() {
+	return serviceAggregator.popServices();
 }
 
 void Discovery::handleCloseEvent(DiscoveryEvent& event) {

diff --git a/libebpfdiscoveryproto/headers/ebpfdiscoveryproto/Translator.h b/libebpfdiscoveryproto/headers/ebpfdiscoveryproto/Translator.h
@@ -9,11 +9,8 @@
 
 namespace proto {
 
-class Translator {
-public:
-	static ServicesList internalToProto(const std::vector<service::Service>& internalServices);
+ServicesList internalToProto(const std::vector<service::Service>& internalServices);
 
-	static std::string protoToJson(const ServicesList& protoServices);
-};
+std::string protoToJson(const ServicesList& protoServices);
 
 } // namespace proto
diff --git a/libebpfdiscoveryproto/src/Translator.cpp b/libebpfdiscoveryproto/src/Translator.cpp
@@ -4,7 +4,7 @@
 
 namespace proto {
 
-ServicesList Translator::internalToProto(const std::vector<service::Service>& internalServices) {
+ServicesList internalToProto(const std::vector<service::Service>& internalServices) {
 	ServicesList external;
 	for (const auto& i : internalServices) {
 		const auto service{external.add_service()};
@@ -17,7 +17,7 @@ ServicesList Translator::internalToProto(const std::vector<service::Service>& in
 	return external;
 }
 
-std::string Translator::protoToJson(const ServicesList& protoServices) {
+std::string protoToJson(const ServicesList& protoServices) {
 	std::string json;
 	google::protobuf::util::JsonPrintOptions options;
 	options.add_whitespace = false;

diff --git a/libebpfdiscoveryproto/test/TranslatorTest.cpp b/libebpfdiscoveryproto/test/TranslatorTest.cpp
@@ -7,7 +7,6 @@
 using namespace proto;
 
 struct ProtobufTranslatorTest : public testing::Test {
-	Translator translator;
 	const std::string expectedServicesJson{
 			"{\"service\":[{\"pid\":1,\"endpoint\":\"/endpoint/"
 			"1\",\"internalClientsNumber\":1,\"externalClientsNumber\":2},{\"pid\":2,\"endpoint\":\"/endpoint/"
@@ -26,7 +25,7 @@ struct ProtobufTranslatorTest : public testing::Test {
 };
 
 TEST_F(ProtobufTranslatorTest, successfulTranslation) {
-	auto proto = translator.internalToProto(internalServices);
-	auto json = translator.protoToJson(proto);
+	auto proto = internalToProto(internalServices);
+	auto json = protoToJson(proto);
 	EXPECT_EQ(json, expectedServicesJson);
 }
diff --git a/libservice/headers/service/Aggregator.h b/libservice/headers/service/Aggregator.h
@@ -28,7 +28,7 @@ class Aggregator {
 public:
 	Aggregator(ebpfdiscovery::IpAddressCheckerInerface& ipChecker);
 
-	std::vector<Service> getServices();
+	std::vector<Service> popServices();
 
 	void newRequest(const httpparser::HttpRequest& request, const DiscoverySessionMeta& meta);
 

diff --git a/libservice/src/Aggregator.cpp b/libservice/src/Aggregator.cpp
@@ -53,7 +53,7 @@ void Aggregator::newRequest(const httpparser::HttpRequest& request, const Discov
 	services[key] = std::move(newService);
 }
 
-std::vector<Service> Aggregator::getServices() {
+std::vector<Service> Aggregator::popServices() {
 	locked = true;
 
 	std::vector<Service> ret;

diff --git a/libservice/test/AggregatorTest.cpp b/libservice/test/AggregatorTest.cpp
@@ -16,7 +16,7 @@ class IpAddressCheckerMock : public IpAddressCheckerInerface {
 };
 
 struct ServiceAggregatorTest : public testing::Test {
-	std::pair<httpparser::HttpRequest, DiscoverySessionMeta> getRequest(
+	std::pair<httpparser::HttpRequest, DiscoverySessionMeta> makeRequest(
 			int pid, std::string host, std::string url, std::optional<__u8> flags = std::nullopt) {
 		httpparser::HttpRequest request;
 		request.host = host;
@@ -34,50 +34,50 @@ struct ServiceAggregatorTest : public testing::Test {
 };
 
 TEST_F(ServiceAggregatorTest, aggregate) {
-	EXPECT_EQ(aggregator.getServices().size(), 0);
+	EXPECT_EQ(aggregator.popServices().size(), 0);
 	// Service 1
 	{
-		const auto [request, meta]{getRequest(100, "host", "/url", DISCOVERY_SESSION_FLAGS_IPV4)};
+		const auto [request, meta]{makeRequest(100, "host", "/url", DISCOVERY_SESSION_FLAGS_IPV4)};
 		EXPECT_CALL(ipCheckerMock, isAddressExternalLocal).WillOnce(testing::Return(true));
 		aggregator.newRequest(request, meta);
 	}
 	{
-		auto [request, meta] = getRequest(100, "host", "/url");
+		auto [request, meta] = makeRequest(100, "host", "/url");
 		aggregator.newRequest(request, meta);
 	}
 	// Service 2
 	{
-		auto [request, meta] = getRequest(100, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
+		auto [request, meta] = makeRequest(100, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
 		EXPECT_CALL(ipCheckerMock, isAddressExternalLocal).WillOnce(testing::Return(false));
 		aggregator.newRequest(request, meta);
 	}
 	// Service 3
 	{
-		auto [request, meta] = getRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
+		auto [request, meta] = makeRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
 		EXPECT_CALL(ipCheckerMock, isAddressExternalLocal).WillOnce(testing::Return(true));
 		aggregator.newRequest(request, meta);
 	}
 	{
-		auto [request, meta] = getRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
+		auto [request, meta] = makeRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
 		EXPECT_CALL(ipCheckerMock, isAddressExternalLocal).WillOnce(testing::Return(false));
 		aggregator.newRequest(request, meta);
 	}
 	{
-		auto [request, meta] = getRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
+		auto [request, meta] = makeRequest(200, "host", "/url2", DISCOVERY_SESSION_FLAGS_IPV4);
 		EXPECT_CALL(ipCheckerMock, isAddressExternalLocal).WillOnce(testing::Return(true));
 		aggregator.newRequest(request, meta);
 	}
 
 	{
-		auto services = aggregator.getServices();
+		auto services = aggregator.popServices();
 		EXPECT_EQ(services.size(), 3);
-		
+
 		Service expectedService1{.pid{100}, .endpoint{"host/url"}, .internalClientsNumber{0}, .externalClientsNumber{2}};
 		Service expectedService2{.pid{100}, .endpoint{"host/url2"}, .internalClientsNumber{1}, .externalClientsNumber{0}};
 		Service expectedService3{.pid{200}, .endpoint{"host/url2"}, .internalClientsNumber{1}, .externalClientsNumber{2}};
 		EXPECT_THAT(services, testing::Contains(expectedService1));
 		EXPECT_THAT(services, testing::Contains(expectedService2));
 		EXPECT_THAT(services, testing::Contains(expectedService3));
 	}
-	EXPECT_EQ(aggregator.getServices().size(), 0);
+	EXPECT_EQ(aggregator.popServices().size(), 0);
 }