Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kenrowland committed Jan 7, 2025
1 parent 4c1ef09 commit 06e88e2
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
4 changes: 2 additions & 2 deletions helm/examples/metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ To create an index with dynamic mapping, use the following object, or other mean

The object names beginning with "hpcc_metric_" represent the mappings for the three affected hpcc metrics types.
The _match_ value for each defines the expected suffix for each HPCC metric value, based on type, when metrics
are reported and indexed. The object names above are the default values useed by the sink. The index can be
are reported and indexed. The object names above are the default values used by the sink. The index can be
configured with different object names (for environment flexibility), but the object names must be provided to the
sink as additional configuration settings (defined below).

Expand Down Expand Up @@ -326,7 +326,7 @@ Add the following to the environment.xml configuration file (note that some valu
<sinks name="myelasticsink" type="elastic">
<settings period="30" ignoreZeroMetrics="1">
<host domain="<domainname>" port="<port>" protocol="http|https"
certificateFilePath="/home/krowland/elastic/cert/http_ca.crt">
certificateFilePath="<path to cert file>">
<authentication type="basic" username="<username>" password="<password>"/>
</host>
<index name="<index>" countSuffixMappingName="<name>" histogramSuffixMappingName="<name>" gaugeSuffixMappingName="<name>/>
Expand Down
4 changes: 1 addition & 3 deletions helm/examples/metrics/elasticsearch_metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ global:
- type: elastic
name: myelasticsink
settings:

host:
protocol: https
domain: domain
Expand All @@ -47,5 +46,4 @@ global:
name: hpccmetrics
countSuffixMappingName: name
gaugeSuffixMappingName: name
histogramSuffixMappingName: name

histogramSuffixMappingName: name
40 changes: 29 additions & 11 deletions system/metrics/sinks/elastic/elasticSink.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*##############################################################################
HPCC SYSTEMS software Copyright (C) 2025 HPCC Systems®.
HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®.
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
Expand Down Expand Up @@ -38,6 +38,10 @@ ElasticMetricSink::ElasticMetricSink(const char *name, const IPropertyTree *pSet
configurationValid = true;
PROGLOG("ElasticMetricSink: Loaded and configured");
}
else
{
PROGLOG("ElasticMetricSink: Configuration invalid, sink not collecting");
}
}


Expand Down Expand Up @@ -127,7 +131,6 @@ bool ElasticMetricSink::getHostConfig(const IPropertyTree *pSettingsTree)
return false;
}
decrypt(password, encryptedPassword.str()); //MD5 encrypted in config
password.clear().append("Blm8huMeiLC4DkX1N1UO");
}
}
return true;
Expand Down Expand Up @@ -175,9 +178,16 @@ bool ElasticMetricSink::getDynamicMappingSuffixesFromIndex(const IPropertyTree *
std::string endpoint;
endpoint.append("/").append(indexName.str()).append("/_mapping");
httplib::Result res = pClient->Get(endpoint.c_str(), elasticHeaders);

if (res == nullptr)
{
WARNLOG("ElasticMetricSink: Unable to connect to ElasticSearch host %s", elasticHostUrl.str());
return false;
}

if (res->status != 200)
{
WARNLOG("ElasticMetricSink: Unable to retrieve index mapping for index %s", indexName.str());
WARNLOG("ElasticMetricSink: Error response status = %d, unable to retrieve mapping for index '%s'", res->status, indexName.str());
return false;
}

Expand All @@ -186,21 +196,21 @@ bool ElasticMetricSink::getDynamicMappingSuffixesFromIndex(const IPropertyTree *
auto indexConfig = data[indexName.str()];
if (indexConfig.is_null())
{
WARNLOG("ElasticMetricSink: Mapping section for Index '%s' does not exist", indexName.str());
WARNLOG("ElasticMetricSink: Unable to load configuration for Index '%s'", indexName.str());
return false;
}

auto mappings = indexConfig["mappings"];
if (mappings.is_null())
{
WARNLOG("ElasticMetricSink: Index %s does not have a 'mappings' section", indexName.str());
WARNLOG("ElasticMetricSink: Required 'mappings' section does not exist in Index '%s'", indexName.str());
return false;
}

auto dynamicTemplates = mappings["dynamic_templates"];
if (dynamicTemplates.is_null())
{
WARNLOG("ElasticMetricSink: Index %s does not have a 'dynamic_templates' section", indexName.str());
WARNLOG("ElasticMetricSink: Required 'dynamic_templates' section does not exist in Index '%s'", indexName.str());
return false;
}

Expand All @@ -215,9 +225,9 @@ bool ElasticMetricSink::getDynamicMappingSuffixesFromIndex(const IPropertyTree *
{
auto const &mappingName = it.first;
auto kvPairs = it.second;
auto const &matchValue = kvPairs["match"];
if (mappingName == countSuffixMappingName.str())
{
auto const &matchValue = kvPairs["match"];
if (!convertPatternToSuffix(matchValue.get<std::string>().c_str(), countMetricSuffix))
{
WARNLOG("ElasticMetricSink: Invalid count suffix pattern");
Expand All @@ -226,7 +236,6 @@ bool ElasticMetricSink::getDynamicMappingSuffixesFromIndex(const IPropertyTree *
}
else if (mappingName == histogramSuffixMappingName.str())
{
auto const &matchValue = kvPairs["match"];
if (!convertPatternToSuffix(matchValue.get<std::string>().c_str(), histogramMetricSuffix))
{
WARNLOG("ElasticMetricSink: Invalid histogram suffix pattern");
Expand All @@ -235,7 +244,6 @@ bool ElasticMetricSink::getDynamicMappingSuffixesFromIndex(const IPropertyTree *
}
else if (mappingName == gaugeSuffixMappingName.str())
{
auto const &matchValue = kvPairs["match"];
if (!convertPatternToSuffix(matchValue.get<std::string>().c_str(), gaugeMetricSuffix))
{
WARNLOG("ElasticMetricSink: Invalid gauge suffix pattern");
Expand Down Expand Up @@ -291,6 +299,10 @@ void ElasticMetricSink::intializeElasticClient()
// Add cert path if needed
if (!certificateFilePath.isEmpty())
pClient->set_ca_cert_path(certificateFilePath.str());

pClient->set_connection_timeout(5);
pClient->set_read_timeout(5);
pClient->set_write_timeout(5);
}


Expand Down Expand Up @@ -318,9 +330,15 @@ bool ElasticMetricSink::validateIndex()
endpoint.append("/").append(indexName.str());
auto res = pClient->Get(endpoint.c_str(), elasticHeaders);

if (res->status != 200)
if (res == nullptr)
{
WARNLOG("ElasticMetricSink: Unable to connect to ElasticSearch host %s", elasticHostUrl.str());
return false;
}

else if (res->status != 200)
{
WARNLOG("ElasticMetricSink: Index %s does not exist", indexName.str());
WARNLOG("ElasticMetricSink: Error response status = %d accessing Index '%s'", res->status, indexName.str());
return false;
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion system/metrics/sinks/elastic/elasticSink.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*##############################################################################
HPCC SYSTEMS software Copyright (C) 2025 HPCC Systems®.
HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®.
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
Expand Down

0 comments on commit 06e88e2

Please sign in to comment.