From 885790b371eb3b855f3db1a5044a7c182cf09e97 Mon Sep 17 00:00:00 2001 From: Ken Rowland Date: Thu, 31 Oct 2024 15:36:11 -0400 Subject: [PATCH] Addressed review comments --- helm/examples/metrics/README.md | 14 +++--- .../metrics/elasticsearch_metrics.yaml | 25 +++++++---- system/metrics/sinks/elastic/elasticSink.cpp | 43 +++++++++++++------ system/metrics/sinks/elastic/elasticSink.hpp | 3 +- 4 files changed, 58 insertions(+), 27 deletions(-) diff --git a/helm/examples/metrics/README.md b/helm/examples/metrics/README.md index e0833f192c5..b290199e68c 100644 --- a/helm/examples/metrics/README.md +++ b/helm/examples/metrics/README.md @@ -250,7 +250,9 @@ Make a copy and modify as needed for your installation. ##### Configuration Settings The ElasticSearch sink defines the following settings: -* elasticHost - The host name or IP address of the ElasticSearch server. (required) +* hostProtocol - The protocol used to connect to the ElasticSearch server. (default: https) +* hostName - The host name or IP address of the ElasticSearch server. (required) +* hostPort - The port number of the ElasticSearch server. (default: 9200) * indexName - The name of the index to which metrics are reported. (required) * countMetricSuffix - The suffix used to identify count metrics. (default: count) * gaugeMetricSuffix - The suffix used to identify gauge metrics. (default: gauge) @@ -271,11 +273,11 @@ settings are shown): - - + + + + + diff --git a/helm/examples/metrics/elasticsearch_metrics.yaml b/helm/examples/metrics/elasticsearch_metrics.yaml index 8a7bd6af19b..0ade007606e 100644 --- a/helm/examples/metrics/elasticsearch_metrics.yaml +++ b/helm/examples/metrics/elasticsearch_metrics.yaml @@ -3,11 +3,15 @@ # Settings: # type - sink type (must be elastic for ElasticSearch support) # name - name for the sink instance -# settings.elasticHost - url for the ElasticSearch instance -# settings.indexName - ElasticSearch index name where metrics are indexed -# settings.countMetricSuffix - suffix for count metrics -# settings.gaugeMetricSuffix - suffix for gauge metrics -# settings.histogramMetricSuffix - suffix for histogram metrics +# settings.countMetricSuffix - suffix for count metrics (default: count) +# settings.gaugeMetricSuffix - suffix for gauge metrics (default: gauge) +# settings.histogramMetricSuffix - suffix for histogram metrics (default: histogram) +# settings.host - ElasticSearch host settings +# settings.host.protocol - protocol to use, http or https (default) +# settings.host.name - host name +# settings.host.port - port number (default 9200) +# settings.index - ElasticSearch index settings +# settings.index.name - index name # # If not overridden, the following suffixes are used by default: # countMetricSuffix: count @@ -18,10 +22,15 @@ global: metrics: sinks: - type: elastic - name: elasticsink + name: myelasticsink settings: - elasticHost: http://localhost:9200, - indexName: hpccmetrics, countMetricSuffix: count, gaugeMetricSuffix: gauge, histogramMetricSuffix: histogram + host: + protocol: https + name: hostname + port: 9200 + index: + name: hpccmetrics + diff --git a/system/metrics/sinks/elastic/elasticSink.cpp b/system/metrics/sinks/elastic/elasticSink.cpp index d3978907b09..0680c97d5bf 100644 --- a/system/metrics/sinks/elastic/elasticSink.cpp +++ b/system/metrics/sinks/elastic/elasticSink.cpp @@ -12,7 +12,6 @@ ############################################################################## */ #include "elasticSink.hpp" - #include "nlohmann/json.hpp" //including cpp-httplib single header file REST client @@ -43,13 +42,38 @@ ElasticMetricSink::ElasticMetricSink(const char *name, const IPropertyTree *pSet PeriodicMetricSink(name, "elastic", pSettingsTree) { ignoreZeroMetrics = pSettingsTree->getPropBool("@ignoreZeroMetrics", true); - pSettingsTree->getProp("@elasticHost", elasticHost); - pSettingsTree->getProp("@indexName", indexName); + + StringBuffer hostProtocol("https"); + StringBuffer hostName; + StringBuffer hostPort("9200"); + + Owned pHostConfigTree = pSettingsTree->getPropTree("host"); + pHostConfigTree->getProp("@protocol", hostProtocol); + pHostConfigTree->getProp("@name", hostName); + pHostConfigTree->getProp("@port", hostPort); + if (!hostName.isEmpty() && !hostPort.isEmpty() && !hostProtocol.isEmpty()) + { + elasticHostUrl.append(hostProtocol).append("://").append(hostName).append(":").append(hostPort); + } + else + { + WARNLOG("ElasticMetricSink: Host configuration is invalid"); + } + + Owned pIndexConfigTree = pSettingsTree->getPropTree("index"); + pSettingsTree->getProp("@name", indexName); + if (indexName.isEmpty()) + { + WARNLOG("ElasticMetricSink: Index configuration is invalid"); + } + + // Both a host url and an index name are required + configurationValid = !elasticHostUrl.isEmpty() && !indexName.isEmpty(); // Initialize standard suffixes - countMetricSuffix.append("_count"); - gaugeMetricSuffix.append("_gauge"); - histogramMetricSuffix.append("_histogram"); + countMetricSuffix.append("count"); + gaugeMetricSuffix.append("gauge"); + histogramMetricSuffix.append("histogram"); // See if any are overridden pSettingsTree->getProp("@countMetricSuffix", countMetricSuffix); @@ -60,11 +84,6 @@ ElasticMetricSink::ElasticMetricSink(const char *name, const IPropertyTree *pSet void ElasticMetricSink::prepareToStartCollecting() { - if (elasticHost.isEmpty()) - throw MakeStringException(-1, "ElasticMetricSink: ElasticSearch host missing"); - - if (indexName.isEmpty()) - throw MakeStringException(-1, "ElasticMetricSink: Index name missing"); } @@ -77,6 +96,6 @@ void ElasticMetricSink::doCollection() void ElasticMetricSink::collectingHasStopped() { - ; + } diff --git a/system/metrics/sinks/elastic/elasticSink.hpp b/system/metrics/sinks/elastic/elasticSink.hpp index c1120d8228f..c435ed04abb 100644 --- a/system/metrics/sinks/elastic/elasticSink.hpp +++ b/system/metrics/sinks/elastic/elasticSink.hpp @@ -38,8 +38,9 @@ class ELASTICSINK_API ElasticMetricSink : public hpccMetrics::PeriodicMetricSink protected: StringBuffer indexName; bool ignoreZeroMetrics = false; - StringBuffer elasticHost; + StringBuffer elasticHostUrl; StringBuffer countMetricSuffix; StringBuffer gaugeMetricSuffix; StringBuffer histogramMetricSuffix; + bool configurationValid = false; };