-
Notifications
You must be signed in to change notification settings - Fork 303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HPCC-32734 Add ElasticSearch metric sink configuration #19247
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,3 +177,110 @@ HPCC metrics can be easily routed to Azure Insights for application level monito | |
11/5/2021, 9:02:00.000 PM prometheus esp_requests_active 0 {"app":"eclservices","namespace":"default","pod_name":"eclservices-778477d679-vgpj2"} | ||
11/5/2021, 9:02:00.000 PM prometheus esp_requests_active 3 {"app":"eclservices","namespace":"default","pod_name":"eclservices-778477d679-vgpj2"} | ||
``` | ||
### ElasticSearch Support | ||
HPCC metrics can be routed to ElasticSearch for advanced metrics processing and | ||
alerting. This process involves two requirements: enabling the ElasticSearch metric | ||
sink, and configuring the index in ElasticSearch to receive the metrics. | ||
|
||
Since the metrics configuration is common across all HPCC components, the ElasticSearch | ||
sink will report metrics from all components to the same index. Therefore, the | ||
index must be configured to receive metrics from all components. | ||
|
||
#### Index Configuration | ||
The index must be created in ElasticSearch before metrics can be reported to it. The name is passed | ||
to the sink as a configuration setting. The index must be created with the following settings: | ||
|
||
##### Dynamic Mapping | ||
The index must be created with dynamic mapping enabled. Dynamic mapping allows framework metric | ||
data types to be stored in the index in their native types. Without dynamic mapping, the ElasticSearch | ||
default mapping does not properly map value to unsigned 64-bit integers. | ||
|
||
To create an index with dynamic mapping, use the following object when creating the index: | ||
```code json | ||
{ | ||
"mappings": { | ||
"dynamic_templates": [ | ||
{ | ||
"hpcc_metric_count_to_unsigned_long": { | ||
"match": "*_count", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this cause problems if other, non-HPCC, components are publishing values with these suffixes to elastic stack? |
||
"mapping": { | ||
"type": "unsigned_long" | ||
} | ||
} | ||
}, | ||
{ | ||
"hpcc_metric_gauge_to_unsigned_long": { | ||
"match": "*_gauge", | ||
"mapping": { | ||
"type": "unsigned_long" | ||
} | ||
} | ||
}, | ||
{ | ||
"hpcc_metric_histogram_to_histogram": { | ||
"match": "*_histogram", | ||
"mapping": { | ||
"type": "histogram" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
} | ||
``` | ||
|
||
Note that there may be other means for adding the required dynamic mapping to the index. | ||
|
||
#### Enabling ElasticSearch Metrics Sink for Kubernetes | ||
To enable reporting of metrics to ElasticSearch, add the metric configuration settings to | ||
the helm chart. | ||
|
||
The provided HPCC helm chart provides all global settings from its values.yml file to all components. | ||
To enable metrics reporting, either include the metrics configuration as a permanent part of | ||
your HPCC helm chart values.yml file, or add it as command line settings at chart installation. | ||
To enable the ElasticSearch sink on the command line, use the following to add the ElasticSearch | ||
settings: | ||
|
||
```code | ||
helm install mycluster ./hpcc -f <path>/elasticsearch_metrics.yml | ||
``` | ||
An example _yml_ file can be found in the repository at helm/examples/metrics/elasticsearch_metrics.yml. | ||
Make a copy and modify as needed for your installation. | ||
|
||
##### Configuration Settings | ||
The ElasticSearch sink defines the following settings: | ||
|
||
* 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) | ||
* histogramMetricSuffix - The suffix used to identify histogram metrics. (default: histogram) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Does the elastic stack support mtls? |
||
|
||
Standard periodic metric sink settings are also available. | ||
|
||
#### Enabling ElasticSearch Metrics Sink for Bare Metal | ||
To enable reporting of metrics to ElasticSearch, add the metric configuration settings to | ||
the environment configuration file (enviroment.xml). These settings must be added manually | ||
since there is no support in the config manager. | ||
|
||
Add the following to the environment configuration file (note only the required | ||
settings are shown): | ||
|
||
```code xml | ||
|
||
<Environment> | ||
<Software> | ||
<metrics name="mymetricsconfig"> | ||
<sinks name="myelasticsink" type="elastic"> | ||
<settings period="30" ignoreZeroMetrics="1"> | ||
<host name="<hostname>" port="<port>" protocol="http|htps"/> | ||
<index name="<index>"/> | ||
<settings/> | ||
</sinks> | ||
</metrics> | ||
</Software> | ||
</Environment> | ||
``` | ||
See section above for additional settings that can be added to the ElasticSearch sink. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# | ||
# Defines an elastic sink for reporting metrics to an ElasticSearch instance | ||
# Settings: | ||
# type - sink type (must be elastic for ElasticSearch support) | ||
# name - name for the sink instance | ||
# settings.countMetricSuffix - suffix for count metrics (default: count) | ||
rpastrana marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# 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 | ||
# gaugeMetricSuffix: gauge | ||
# histogramMetricSuffix: histogram | ||
|
||
global: | ||
metrics: | ||
sinks: | ||
- type: elastic | ||
name: myelasticsink | ||
settings: | ||
countMetricSuffix: count, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume the comma here and next line are include by mistake. |
||
gaugeMetricSuffix: gauge, | ||
histogramMetricSuffix: histogram | ||
host: | ||
protocol: https | ||
name: hostname | ||
port: 9200 | ||
index: | ||
name: hpccmetrics | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,6 @@ | |
############################################################################## */ | ||
|
||
#include "elasticSink.hpp" | ||
|
||
#include "nlohmann/json.hpp" | ||
|
||
//including cpp-httplib single header file REST client | ||
|
@@ -43,8 +42,60 @@ 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 hostName; | ||
StringBuffer hostProtocol; | ||
StringBuffer hostPort; | ||
|
||
Owned<IPropertyTree> pHostConfigTree = pSettingsTree->getPropTree("host"); | ||
if (pHostConfigTree) | ||
{ | ||
pHostConfigTree->getProp("@name", hostName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor future point: If the tree is guaranteed to stay active then more common is to use queryProp() and avoid cloning the strings. In this case the code is only executed once, so it doesn't matter.
|
||
|
||
if (!pHostConfigTree->getProp("@protocol", hostProtocol)) | ||
{ | ||
hostProtocol.append("https"); | ||
} | ||
|
||
if (!pHostConfigTree->getProp("@port", hostPort)) | ||
{ | ||
hostPort.append("9200"); | ||
} | ||
} | ||
|
||
if (!hostName.isEmpty() && !hostPort.isEmpty() && !hostProtocol.isEmpty()) | ||
{ | ||
elasticHostUrl.append(hostProtocol).append("://").append(hostName).append(":").append(hostPort); | ||
} | ||
else | ||
{ | ||
WARNLOG("ElasticMetricSink: Host configuration missing or invalid"); | ||
} | ||
|
||
Owned<IPropertyTree> pIndexConfigTree = pSettingsTree->getPropTree("index"); | ||
if (pIndexConfigTree) | ||
{ | ||
pSettingsTree->getProp("@name", indexName); | ||
} | ||
|
||
if (indexName.isEmpty()) | ||
{ | ||
WARNLOG("ElasticMetricSink: Index configuration missing or invalid"); | ||
} | ||
|
||
|
||
// Both a host url and an index name are required | ||
configurationValid = !elasticHostUrl.isEmpty() && !indexName.isEmpty(); | ||
|
||
// Initialize standard suffixes | ||
countMetricSuffix.append("count"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it likely that anyone is going to want to change these? Under what situation? |
||
gaugeMetricSuffix.append("gauge"); | ||
histogramMetricSuffix.append("histogram"); | ||
|
||
// See if any are overridden | ||
pSettingsTree->getProp("@countMetricSuffix", countMetricSuffix); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code does not work. getProp() appends to the existing value, it does not replace it. I am not sure why this code uses a different pattern from the code above that gets the hostname. |
||
pSettingsTree->getProp("@gaugeMetricSuffix", gaugeMetricSuffix); | ||
pSettingsTree->getProp("@histogramMetricSuffix", histogramMetricSuffix); | ||
} | ||
|
||
|
||
|
@@ -62,6 +113,6 @@ void ElasticMetricSink::doCollection() | |
|
||
void ElasticMetricSink::collectingHasStopped() | ||
{ | ||
; | ||
|
||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this write up seems very useful.
Is the goal still to handle the creation of the indices programmatically in later versions of the sink?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently no plans to create the index from the sink. If that was something we added, there are several considerations that must be taken into account. These are
I felt like just telling the sink what index to use allows the policy of the ElasticSearch instance to remain there instead of pushing it down to the sink.
If we decide to push that responsibility to the sink, we can add it as an improvement addressing the concerns from above