Cleanup code after the first round of comments.

General improvements to code and documentation. Fixing comments by:
- Mattia Codato
- Johannes Schmidt

Author: w1ll-i-code

doc/14-features.md

@@ -440,9 +440,16 @@ where you either have the Elasticsearch HTTP API, or a TLS secured HTTP proxy,
 or Logstash for additional filtering.
 
 
-#### Elasticsearch Writer <a id="elasticsearch-writer"></a>
+#### Elasticsearch Datastream Writer <a id="elasticsearch-datastream-writer"></a>
+
+> **Note**
+>
+> This is a newer alternative to the Elasticsearch Writer above. The Elasticsearch Datastream Writer uses
+> Elasticsearch's datastream feature and follows the Elastic Common Schema (ECS), providing better performance
+> and data organization. Use this writer for new installations. The original Elasticsearch Writer is still
+> available for backward compatibility.
 
-This feature sends check results with performance data to to an [Elasticsearch](https://www.elastic.co/products/elasticsearch) instance or cluster.
+This feature sends check results with performance data to an [Elasticsearch](https://www.elastic.co/products/elasticsearch) instance or cluster.
 
 > **Note**
 >
@@ -455,7 +462,7 @@ icinga2 feature enable elasticsearchdatastream
 ```
 
 The default configuration expects an Elasticsearch instance running on `localhost` on port `9200`
- and writes to an index called `icinga2`.
+ and writes to datastreams with the pattern `metrics-icinga2.<check>-<namespace>`.
 
 More configuration details can be found [here](09-object-types.md#objecttype-elasticsearchdatastreamwriter).
 
@@ -470,26 +477,6 @@ configuration parameter to further separate documents, e.g. by environment like
 The `datastream_namespace` can also be used to separate documents e.g. by hostgroups or zones, by using the
 `filter` function to filter the check results and use several writers with different namespaces.
 
-Metric values are stored like this:
-
-```
-check_result.perfdata.<perfdata-label>.value
-```
-
-The following characters are escaped in perfdata labels:
-
-  Character   | Escaped character
-  ------------|--------------------------
-  whitespace  | _
-  \           | _
-  /           | _
-  ::          | .
-
-Note that perfdata labels may contain dots (`.`) allowing to
-add more subsequent levels inside the tree.
-`::` adds support for [multi performance labels](https://github.com/flackem/check_multi/blob/next/doc/configuration/performance.md)
-and is therefore replaced by `.`.
-
 Icinga 2 automatically adds the following threshold metrics
 if existing:
 
@@ -545,10 +532,10 @@ object ElasticsearchDatastreamWriter "elasticsearchdatastream" {
 }
 ```
 
-#### Elasticsearch in Cluster HA Zones <a id="elasticsearch-writer-cluster-ha"></a>
+#### Elasticsearch Datastream Writer in Cluster HA Zones <a id="elasticsearch-datastream-writer-cluster-ha"></a>
 
-The Elasticsearch feature supports [high availability](06-distributed-monitoring.md#distributed-monitoring-high-availability-features)
-in cluster zones since 2.11.
+The Elasticsearch Datastream Writer feature supports [high availability](06-distributed-monitoring.md#distributed-monitoring-high-availability-features)
+in cluster zones.
 
 By default, all endpoints in a zone will activate the feature and start
 writing events to the Elasticsearch HTTP API. In HA enabled scenarios,

lib/perfdata/elasticsearchdatastreamwriter.cpp

@@ -2,6 +2,7 @@
 
 #include <boost/algorithm/string.hpp>
 #include <boost/asio/ssl/context.hpp>
+#include <boost/asio/ssl/stream.hpp>
 #include <boost/beast/core/flat_buffer.hpp>
 #include <boost/beast/http/field.hpp>
 #include <boost/beast/http/message.hpp>
@@ -43,6 +44,11 @@ using namespace icinga;
 namespace beast = boost::beast;
 namespace http = beast::http;
 
+static void ExceptionHandler(const boost::exception_ptr& exp);
+static Array::Ptr ExtractTemplateTags(const MacroProcessor::ResolverList& resolvers, const Array::Ptr& tags_tmpl,
+	const Checkable::Ptr& checkable, const CheckResult::Ptr& cr);
+static Dictionary::Ptr ExtractTemplateLabels(const MacroProcessor::ResolverList& resolvers, const Dictionary::Ptr& labels_tmpl,
+	const Checkable::Ptr& checkable, const CheckResult::Ptr& cr);
 
 REGISTER_TYPE(ElasticsearchDatastreamWriter);
 
@@ -87,7 +93,7 @@ void ElasticsearchDatastreamWriter::Resume()
 	Log(LogInformation, "ElasticsearchDatastreamWriter")
 		<< "'" << GetName() << "' resumed.";
 	m_Paused = false;
-	m_WorkQueue.SetExceptionCallback([this](boost::exception_ptr exp) { ExceptionHandler(std::move(exp)); });
+	m_WorkQueue.SetExceptionCallback([](boost::exception_ptr exp) { ExceptionHandler(exp); });
 
 	if (GetManageIndexTemplate()) {
 		// Ensure index template exists/is updated as the first item on the work queue.
@@ -167,9 +173,9 @@ void ElasticsearchDatastreamWriter::ManageIndexTemplate() {
 			Dictionary::Ptr jsonResponse = TrySend(component_template_url, "{\"template\":{}}");
 			Log(LogInformation, "ElasticsearchDatastreamWriter")
 				<< "Successfully installed component template 'icinga2@custom'.";
-		} catch (const StatusCodeException es) {
+		} catch (const StatusCodeException& es) {
 		  int status_code = es.GetStatusCode();
-		  if (status_code == 400) {
+		  if (status_code == static_cast<int>(http::status::bad_request)) {
 			Log(LogInformation, "ElasticsearchDatastreamWriter")
 				<< "Component template 'metrics-icinga2@custom' already exists, skipping creation.";
 			// Continue to install/update index template
@@ -207,7 +213,7 @@ void ElasticsearchDatastreamWriter::ManageIndexTemplate() {
 	}
 }
 
-Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractPerfData(const Checkable::Ptr checkable, const Array::Ptr& perfdata) {
+Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractPerfData(const Checkable::Ptr& checkable, const Array::Ptr& perfdata) {
 	Dictionary::Ptr pd_fields = new Dictionary();
 	if (!perfdata)
 		return pd_fields;
@@ -231,7 +237,7 @@ Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractPerfData(const Checkable::
 
 		Dictionary::Ptr metric = new Dictionary();
 		metric->Set("value", pdv->GetValue());
-		metric->Set("counter", pdv->GetCounter());
+		if (pdv->GetCounter()) metric->Set("counter", pdv->GetCounter());
 
 		if (!pdv->GetMin().IsEmpty()) metric->Set("min", pdv->GetMin());
 		if (!pdv->GetMax().IsEmpty()) metric->Set("max", pdv->GetMax());
@@ -249,8 +255,8 @@ Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractPerfData(const Checkable::
 }
 
 // user-defined tags, as specified in the ECS specification: https://www.elastic.co/docs/reference/ecs/ecs-base#field-tags
-Array::Ptr ElasticsearchDatastreamWriter::ExtractTemplateTags(const MacroProcessor::ResolverList resolvers,
-	const Array::Ptr tag_tmpl, const Checkable::Ptr& checkable, const CheckResult::Ptr& cr)
+static Array::Ptr ExtractTemplateTags(const MacroProcessor::ResolverList& resolvers,
+	const Array::Ptr& tag_tmpl, const Checkable::Ptr& checkable, const CheckResult::Ptr& cr)
 {
 	if (tag_tmpl == nullptr) {
 		return nullptr;
@@ -275,8 +281,8 @@ Array::Ptr ElasticsearchDatastreamWriter::ExtractTemplateTags(const MacroProcess
 }
 
 // user-defined labels, as specified in the ECS specification: https://www.elastic.co/docs/reference/ecs/ecs-base#field-labels
-Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractTemplateLabels(const MacroProcessor::ResolverList resolvers,
-	const Dictionary::Ptr labels_tmpl, const Checkable::Ptr& checkable, const CheckResult::Ptr& cr)
+static Dictionary::Ptr ExtractTemplateLabels(const MacroProcessor::ResolverList& resolvers,
+	const Dictionary::Ptr& labels_tmpl, const Checkable::Ptr& checkable, const CheckResult::Ptr& cr)
 {
 	if (labels_tmpl == nullptr) {
 		return nullptr;
@@ -301,7 +307,7 @@ Dictionary::Ptr ElasticsearchDatastreamWriter::ExtractTemplateLabels(const Macro
 	return labels;
 }
 
-String ElasticsearchDatastreamWriter::ExtractDatastreamNamespace(MacroProcessor::ResolverList resolvers,
+String ElasticsearchDatastreamWriter::ExtractDatastreamNamespace(const MacroProcessor::ResolverList& resolvers,
 	const Checkable::Ptr& checkable, const CheckResult::Ptr& cr)
 {
 	String namespace_tmpl = GetDatastreamNamespace();
@@ -365,20 +371,21 @@ void ElasticsearchDatastreamWriter::CheckResultHandler(const Checkable::Ptr& che
 		{ "version", "8.0.0" }
 	});
 
-	String datastream_name = "metrics-icinga2." + checkable->GetCheckCommandRaw() + "-" + GetDatastreamNamespace();
+	String datastream_namespace = ExtractDatastreamNamespace(resolvers, checkable, cr);
+	String datastream_name = "metrics-icinga2." + checkable->GetCheckCommandRaw() + "-" + datastream_namespace;
 	Dictionary::Ptr data_stream = new Dictionary({
 		{"type", "metrics"},
 		{"dataset", "icinga2." + checkable->GetCheckCommandRaw()},
-		{"namespace", ExtractDatastreamNamespace(resolvers, checkable, cr)}
+		{"namespace", datastream_namespace}
 	});
 
 	Dictionary::Ptr ecs_host = new Dictionary({
 		{"name", host->GetDisplayName()},
 		{"hostname", host->GetName()},
-		{"zone", host->GetZone()->GetZoneName()},
 		{"soft_state", host->GetState()},
 		{"hard_state", host->GetLastHardState()}
 	});
+	if (!host->GetZoneName().IsEmpty()) ecs_host->Set("zone", host->GetZoneName());
 
 	char _addr[16];
 	if (!host->GetAddress().IsEmpty() && inet_pton(AF_INET, host->GetAddress().CStr(), _addr) == 1) {
@@ -394,10 +401,10 @@ void ElasticsearchDatastreamWriter::CheckResultHandler(const Checkable::Ptr& che
 	   	ecs_service = new Dictionary({
 	   		{"name", service->GetName()},
 			{"display_name", service->GetDisplayName()},
-			{"zone", host->GetZone()->GetZoneName()},
 			{"soft_state", service->GetState()},
 			{"hard_state", service->GetLastHardState()}
 		});
+		if (!service->GetZoneName().IsEmpty()) ecs_service->Set("zone", service->GetZoneName());
 	}
 
 	Dictionary::Ptr ecs_agent = new Dictionary({
@@ -407,10 +414,10 @@ void ElasticsearchDatastreamWriter::CheckResultHandler(const Checkable::Ptr& che
 	Endpoint::Ptr endpoint = checkable->GetCommandEndpoint();
 	if (endpoint) {
 		ecs_agent->Set("id", endpoint->GetName());
-		ecs_agent->Set("version", FormatIcingaVersion(endpoint->GetIcingaVersion()));
+		ecs_agent->Set("version", endpoint->GetIcingaVersionString());
 	} else {
 		ecs_agent->Set("id", IcingaApplication::GetInstance()->GetName());
-		ecs_agent->Set("version", FormatIcingaVersion((unsigned long) IcingaApplication::GetInstance()->GetVersion()));
+		ecs_agent->Set("version", IcingaApplication::GetAppSpecVersion());
 	}
 
 	Dictionary::Ptr ecs_event = new Dictionary({
@@ -432,25 +439,38 @@ void ElasticsearchDatastreamWriter::CheckResultHandler(const Checkable::Ptr& che
 		{"active", cr->GetActive()}
 	});
 
-	Dictionary::Ptr perf_data = ExtractPerfData(checkable, cr->GetPerformanceData());
-
 	Dictionary::Ptr document = new Dictionary({
 		{"@timestamp", FormatTimestamp(cr->GetScheduleEnd())},
 		{ "ecs", ecs_metadata },
 		{ "data_stream", data_stream },
 		{ "host", ecs_host },
-		{ "service", ecs_service },
 		{ "agent", ecs_agent },
 		{ "event", ecs_event },
 		{ "check", check_result },
 		{ "message", cr->GetOutput() },
-		{ "perf_data", perf_data },
-		{ "tags", ExtractTemplateTags(
-			resolvers, service ? GetServiceTagsTemplate() : GetHostTagsTemplate(),  checkable, cr) },
-		{ "labels", ExtractTemplateLabels(
-			resolvers, service ? GetServiceLabelsTemplate() : GetHostLabelsTemplate(), checkable, cr) }
 	});
 
+	Dictionary::Ptr perfdata = ExtractPerfData(checkable, cr->GetPerformanceData());
+	if (perfdata->GetLength() != 0) {
+        document->Set("perfdata", perfdata);
+    }
+
+	Array::Ptr ecs_tags = ExtractTemplateTags(
+	    resolvers, service ? GetServiceTagsTemplate() : GetHostTagsTemplate(),  checkable, cr);
+	if (ecs_tags && ecs_tags->GetLength() != 0 ) {
+        document->Set("tags", ecs_tags);
+    }
+
+	Dictionary::Ptr ecs_labels = ExtractTemplateLabels(
+        resolvers, service ? GetServiceLabelsTemplate() : GetHostLabelsTemplate(), checkable, cr);
+	if (ecs_labels && ecs_labels->GetLength() != 0 ) {
+	        document->Set("labels", ecs_labels);
+    }
+
+	if (ecs_service && ecs_service->GetLength() != 0 ) {
+	        document->Set("service", ecs_service);
+    }
+
 	EcsDocument::Ptr workqueue_document = new EcsDocument(
 		datastream_name,
 		document
@@ -550,7 +570,7 @@ Value ElasticsearchDatastreamWriter::TrySend(Url::Ptr url, String body) {
 	m_FlushTimer->Reschedule(-1);
 
 	http::request<http::string_body> request (http::verb::post, std::string(url->Format(true)), 10);
-	request.set(http::field::user_agent, "Icinga/" + Application::GetAppVersion());
+	request.set(http::field::user_agent, "Icinga/" + Application::GetAppSpecVersion());
 	request.set(http::field::host, url->GetHost() + ":" + url->GetPort());
 
 	/* Specify required headers by Elasticsearch. */
@@ -676,7 +696,7 @@ OptionalTlsStream ElasticsearchDatastreamWriter::Connect()
 		auto& tlsStream (stream.first->next_layer());
 
 		try {
-			tlsStream.handshake(tlsStream.client);
+			tlsStream.handshake(boost::asio::ssl::stream_base::client);
 		} catch (const std::exception&) {
 			Log(LogWarning, "ElasticsearchDatastreamWriter")
 				<< "TLS handshake with host '" << GetHost() << "' on port " << GetPort() << " failed.";
@@ -705,12 +725,12 @@ void ElasticsearchDatastreamWriter::AssertOnWorkQueue()
 	ASSERT(m_WorkQueue.IsWorkerThread());
 }
 
-void ElasticsearchDatastreamWriter::ExceptionHandler(boost::exception_ptr exp)
+static void ExceptionHandler(const boost::exception_ptr& exp)
 {
 	Log(LogCritical, "ElasticsearchDatastreamWriter", "Exception during Elastic operation: Verify that your backend is operational!");
 
 	Log(LogDebug, "ElasticsearchDatastreamWriter")
-		<< "Exception during Elasticsearch operation: " << DiagnosticInformation(std::move(exp));
+		<< "Exception during Elasticsearch operation: " << DiagnosticInformation(exp);
 }
 
 String ElasticsearchDatastreamWriter::FormatTimestamp(double ts)
@@ -731,16 +751,7 @@ String ElasticsearchDatastreamWriter::FormatTimestamp(double ts)
 	return Utility::FormatDateTime("%Y-%m-%dT%H:%M:%S", ts) + "." + Convert::ToString(milliSeconds) + Utility::FormatDateTime("%z", ts);
 }
 
-String ElasticsearchDatastreamWriter::FormatIcingaVersion(unsigned long version) {
-	auto bugfix = version % 100;
-	version /= 100;
-	auto minor = version % 100;
-	auto major = version / 100;
-	return String() + std::to_string(major) + "." + std::to_string(minor) + "." + std::to_string(bugfix);
-}
-
-
-void ElasticsearchDatastreamWriter::ValidateTagsTemplate(Array::Ptr tags) {
+void ElasticsearchDatastreamWriter::ValidateTagsTemplate(const Array::Ptr& tags) {
 	ObjectLock olock(tags);
 	for (const Value& tag : tags) {
 		if (!MacroProcessor::ValidateMacroString(tag)) {
@@ -752,7 +763,7 @@ void ElasticsearchDatastreamWriter::ValidateTagsTemplate(Array::Ptr tags) {
 void ElasticsearchDatastreamWriter::ValidateHostTagsTemplate(const Lazy<Array::Ptr>& lvalue, const ValidationUtils& utils)
 {
 	ObjectImpl<ElasticsearchDatastreamWriter>::ValidateHostTagsTemplate(lvalue, utils);
-	Array::Ptr tags = lvalue();
+	auto& tags = lvalue();
 	if (tags) {
 		ValidateTagsTemplate(tags);
 	}
@@ -761,13 +772,13 @@ void ElasticsearchDatastreamWriter::ValidateHostTagsTemplate(const Lazy<Array::P
 void ElasticsearchDatastreamWriter::ValidateServiceTagsTemplate(const Lazy<Array::Ptr>& lvalue, const ValidationUtils& utils)
 {
 	ObjectImpl<ElasticsearchDatastreamWriter>::ValidateServiceTagsTemplate(lvalue, utils);
-	Array::Ptr tags = lvalue();
+	auto& tags = lvalue();
 	if (tags) {
 		ValidateTagsTemplate(tags);
 	}
 }
 
-void ElasticsearchDatastreamWriter::ValidateLabelsTemplate(Dictionary::Ptr labels) {
+void ElasticsearchDatastreamWriter::ValidateLabelsTemplate(const Dictionary::Ptr& labels) {
 	ObjectLock olock(labels);
 	for (const Dictionary::Pair& kv : labels) {
 		if (!MacroProcessor::ValidateMacroString(kv.second)) {
@@ -779,7 +790,7 @@ void ElasticsearchDatastreamWriter::ValidateLabelsTemplate(Dictionary::Ptr label
 void ElasticsearchDatastreamWriter::ValidateHostLabelsTemplate(const Lazy<Dictionary::Ptr>& lvalue, const ValidationUtils& utils)
 {
 	ObjectImpl<ElasticsearchDatastreamWriter>::ValidateHostLabelsTemplate(lvalue, utils);
-	Dictionary::Ptr labels = lvalue();
+	auto& labels = lvalue();
 	if (labels) {
 		ValidateLabelsTemplate(labels);
 	}
@@ -788,7 +799,7 @@ void ElasticsearchDatastreamWriter::ValidateHostLabelsTemplate(const Lazy<Dictio
 void ElasticsearchDatastreamWriter::ValidateServiceLabelsTemplate(const Lazy<Dictionary::Ptr>& lvalue, const ValidationUtils& utils)
 {
 	ObjectImpl<ElasticsearchDatastreamWriter>::ValidateServiceLabelsTemplate(lvalue, utils);
-	Dictionary::Ptr labels = lvalue();
+	auto& labels = lvalue();
 	if (labels) {
 		ValidateLabelsTemplate(labels);
 	}
@@ -816,7 +827,6 @@ void ElasticsearchDatastreamWriter::ValidateFilter(const Lazy<Value> &lvalue, co
 	m_CompiledFilter = std::move(fexpr);
 }
 
-EcsDocument::EcsDocument(String index, Dictionary::Ptr document) {
-	SetIndex(index, true);
-	SetDocument(document, true);
+EcsDocument::EcsDocument(String index, Dictionary::Ptr document)
+	: m_Index(std::move(index)), m_Document(std::move(document)) {
 }