From da16120052b29b9c2068be9d77a56acf05766c70 Mon Sep 17 00:00:00 2001 From: Andrew Gizas Date: Mon, 15 Apr 2024 11:32:08 +0300 Subject: [PATCH] Logshints (#83) Continuation of https://github.com/elastic/elastic-agent-autodiscover/pull/81 We enhance the autodiscovery checks for supported hints to check multiple data_streams (in case of agent) and multiple metrcisets (in case of beats) --- CHANGELOG.md | 9 ++++ utils/hints.go | 116 +++++++++++++++++++++++++++++++++++++++++--- utils/hints_test.go | 82 +++++++++++++++++++++++++++++-- 3 files changed, 195 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 247ac580f..4e8ab4b2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,3 +53,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). [0.6.9]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.8...v0.6.9 + +## [0.6.11] + +### Changed + +- Enhance GenerateHints function to check supported list of hints for multiple datastreams and metricsets + + +[0.6.10]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.10...v0.6.11 diff --git a/utils/hints.go b/utils/hints.go index cde1ae185..41e79996d 100644 --- a/utils/hints.go +++ b/utils/hints.go @@ -20,6 +20,7 @@ package utils import ( "encoding/json" "fmt" + "regexp" "sort" "strconv" "strings" @@ -205,18 +206,63 @@ func IsDisabled(hints mapstr.M, key string) bool { func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedHints []string) (mapstr.M, []string) { hints := mapstr.M{} var incorrecthints []string - found := false + var incorrecthint string + var digitCheck = regexp.MustCompile(`^[0-9]+$`) + if rawEntries, err := annotations.GetValue(prefix); err == nil { if entries, ok := rawEntries.(mapstr.M); ok { - for key, rawValue := range entries { + //Start of Annotation Check: whether the annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic + datastreamlist := GetHintAsList(entries, logName+"/"+"data_streams", "") + // We check if multiple data_streams are defined and we retrieve the hints per data_stream. Only applicable in elastic-agent + // See Metrics_apache_package_and_specific_config_per_datastream test case in hints_test.go + for _, stream := range datastreamlist { + allSupportedHints = append(allSupportedHints, stream) + incorrecthints = checkSupportedHintsSets(annotations, prefix, stream, logName, allSupportedHints, incorrecthints) + } + metricsetlist := GetHintAsList(entries, "metrics"+"/"+"metricsets", "") + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + //See Metrics_istio_module_and_specific_config_per_metricset test case in hints_test.go + for _, metric := range metricsetlist { + allSupportedHints = append(allSupportedHints, metric) + incorrecthints = checkSupportedHintsSets(annotations, prefix, metric, "metrics", allSupportedHints, incorrecthints) + } + //End of Annotation Check + + for key, rawValue := range entries { + enumeratedmodules := []string{} // If there are top level hints like co.elastic.logs/ then just add the values after the / // Only consider namespaced annotations parts := strings.Split(key, "/") if len(parts) == 2 { hintKey := fmt.Sprintf("%s.%s", parts[0], parts[1]) + + checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided + if checkdigit { + allSupportedHints = append(allSupportedHints, parts[1]) + + specificlist, _ := entries.GetValue(key) + if specificentries, ok := specificlist.(mapstr.M); ok { + for keyspec := range specificentries { + // enumeratedmodules will be populated only in cases we have module enumeration, like: + // "co.elastic.metrics/1.module": "prometheus", + // "co.elastic.metrics/2.module": "istiod", + enumeratedmodules = append(enumeratedmodules, keyspec) + } + } + } + + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go + for _, metric := range enumeratedmodules { + _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) + } + + } //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic - found = checkSupportedHints(parts[1], allSupportedHints) + _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) // Insert only if there is no entry already. container level annotations take // higher priority. @@ -239,8 +285,36 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH if strings.HasPrefix(hintKey, container) { // Split the key to get part[1] to be the hint parts := strings.Split(hintKey, "/") + + checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided + if checkdigit { + allSupportedHints = append(allSupportedHints, parts[1]) + + specificlist, _ := entries.GetValue(key) + if specificentries, ok := specificlist.(mapstr.M); ok { + for keyspec := range specificentries { + // enumeratedmodules will be populated only in cases we have module enumeration, like: + // "co.elastic.metrics/1.module": "prometheus", + // "co.elastic.metrics/2.module": "istiod", + enumeratedmodules = append(enumeratedmodules, keyspec) + } + } + } + + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go + for _, metric := range enumeratedmodules { + _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) + } + + } //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic - found = checkSupportedHints(parts[1], allSupportedHints) + _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) + + //end of check + if len(parts) == 2 { // key will be the hint type hintKey := fmt.Sprintf("%s.%s", key, parts[1]) @@ -252,8 +326,8 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH } } } - if !found { - incorrecthints = append(incorrecthints, key) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) } } } @@ -302,13 +376,39 @@ func GetHintsAsList(hints mapstr.M, key string) []mapstr.M { } // checkSupportedHints gets a specific hint annotation and compares it with the supported list of hints -func checkSupportedHints(actualannotation string, allSupportedHints []string) bool { +func checkSupportedHints(actualannotation, key string, allSupportedHints []string) (bool, string) { found := false + var incorrecthint string + for _, checksupported := range allSupportedHints { if actualannotation == checksupported { found = true break } + + } + if !found { + incorrecthint = key } - return found + return found, incorrecthint +} + +// checkSupportedHintsSets gest the data_streams or metricset lists that are defined. Searches inside specific list and returns the unsupported list of hints found +// This function will merge the incorrect hints found in metricsets of data_streams with rest incorrect hints +func checkSupportedHintsSets(annotations mapstr.M, prefix, stream, kind string, allSupportedHints, incorrecthints []string) []string { + var incorrecthint string + + if hintsindatastream, err := annotations.GetValue(prefix + "." + kind + "/" + stream); err == nil { + if hintsentries, ok := hintsindatastream.(mapstr.M); ok { + for hintkey := range hintsentries { + _, incorrecthint = checkSupportedHints(hintkey, kind+"/"+stream+"."+hintkey, allSupportedHints) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) + } + } + + } + } + + return incorrecthints } diff --git a/utils/hints_test.go b/utils/hints_test.go index 3072c90b3..0f48cc0e9 100644 --- a/utils/hints_test.go +++ b/utils/hints_test.go @@ -51,7 +51,7 @@ func TestGetProcessors(t *testing.T) { func TestGenerateHints(t *testing.T) { - var allSupportedHints = []string{"enabled", "module", "integration", "datas_treams", "host", "period", "timeout", "metrics_path", "username", "password", "stream", "processors", "multiline", "json", "disable"} + var allSupportedHints = []string{"enabled", "package", "module", "integration", "data_streams", "metricsets", "host", "period", "timeout", "metrics_path", "username", "password", "stream", "processors", "multiline", "json", "disable"} tests := []struct { name string @@ -117,7 +117,7 @@ func TestGenerateHints(t *testing.T) { "co.elastic.metrics/password": "pass", "co.elastic.metrics.foobar/period": "15s", "co.elastic.metrics.foobar1/period": "15s", - "co.elastic.hints/steam": "stdout", // On purpose this added with typo + "co.elastic.hints/streamssssssssss": "stdout", // On purpose this added with typo "not.to.include": "true", }, result: mapstr.M{ @@ -126,7 +126,7 @@ func TestGenerateHints(t *testing.T) { "pattern": "^test", }, }, - "hints": mapstr.M{"steam": "stdout"}, + "hints": mapstr.M{"streamssssssssss": "stdout"}, "metrics": mapstr.M{ "module": "prometheus", "period": "15s", @@ -135,7 +135,7 @@ func TestGenerateHints(t *testing.T) { "password": "pass", }, }, - expectedIncorrectHints: 1, // Due to co.elastic.hints/steam and not co.elastic.hints/stream + expectedIncorrectHints: 1, // Due to co.elastic.hints/streamsteamssssssssss }, // Scenarios being tested: // logs/multiline.pattern must be a nested mapstr.M under hints.logs @@ -227,6 +227,79 @@ func TestGenerateHints(t *testing.T) { }, expectedIncorrectHints: 0, }, + // Scenarios being tested: + // have co.elastic.hints/package set. + // Define multiple co.elastic.hints/data_streams and also specific configuration for each one + // Typo errors introduced for "co.elastic.hints/access.streams" and "co.elastic.hints/error.streams" + { + name: "Metrics_apache_package_and_specific_config_per_datastream", + annotations: map[string]string{ + "co.elastic.hints/package": "apache", + "co.elastic.hints/data_streams": "access,error", + "co.elastic.hints/access.period": "5m", + "co.elastic.hints/access.streamssssssssss": "stdout", // On purpose this added with typo + "co.elastic.hints/error.period": "5m", + "co.elastic.hints/error.streamssssssssss": "stderr", // On purpose this added with typo + }, + result: mapstr.M{ + "hints": mapstr.M{ + "data_streams": "access,error", + "access": mapstr.M{"period": "5m", "streamssssssssss": "stdout"}, + "error": mapstr.M{"period": "5m", "streamssssssssss": "stderr"}, + "package": "apache", + }}, + expectedIncorrectHints: 2, // Due to co.elastic.hints/access.streamssssssssss and co.elastic.hints/error.streamssssssssss typo errors + }, + // Scenarios being tested: + // have co.elastic.metrics/module set. + // Define multiple co.elastic.hints/data_streams and also specific configuration for each one + // A typo error introduced for "co.elastic.metrics/istiod.streams" + { + name: "Metrics_istio_module_and_specific_config_per_metricset", + annotations: map[string]string{ + "co.elastic.metrics/module": "istio", + "co.elastic.metrics/metricsets": "istiod,proxy", + "co.elastic.metrics/istiod.period": "5m", + "co.elastic.metrics/istiod.streamssssssssss": "stdout", // On purpose this added with typo + "co.elastic.metrics/proxy.period": "5m", + "co.elastic.metrics/proxy.stream": "stderr", + }, + result: mapstr.M{ + "metrics": mapstr.M{ + "metricsets": "istiod,proxy", + "istiod": mapstr.M{"period": "5m", "streamssssssssss": "stdout"}, + "proxy": mapstr.M{"period": "5m", "stream": "stderr"}, + "module": "istio", + }}, + expectedIncorrectHints: 1, // Due to co.elastic.metrics/istiod.streamssssssssss + }, + // Scenarios being tested: + // have co.elastic.metrics/module set for multiple enumerations. + // Define different hints for each one enumeration + // A typo error introduced for "co.elastic.metrics/1.periods" and "co.elastic.metrics/2.streams" + { + name: "Metrics_multiple_modules_and_specific_config_per_module", + annotations: map[string]string{ + "co.elastic.metrics/1.module": "prometheus", + "co.elastic.metrics/1.periodssssssssss": "15s", // On purpose this added with typo + "co.elastic.metrics/2.module": "istiod", + "co.elastic.metrics/2.period": "15s", + "co.elastic.metrics/2.streamssssssssss": "stderr", // On purpose this added with typo + }, + result: mapstr.M{ + "metrics": mapstr.M{ + "1": mapstr.M{ + "module": "prometheus", + "periodssssssssss": "15s", + }, + "2": mapstr.M{ + "module": "istiod", + "period": "15s", + "streamssssssssss": "stderr", + }, + }}, + expectedIncorrectHints: 2, // Due to co.elastic.metrics/1.periodssssssssss and co.elastic.metrics/2.streamssssssssss typo errors + }, } for _, test := range tests { @@ -237,6 +310,7 @@ func TestGenerateHints(t *testing.T) { continue } } + generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", allSupportedHints) assert.Equal(t, test.expectedIncorrectHints, len(incorrectHints)) // We validate how many incorrect hints are provided per test case. assert.Equal(t, test.result, generateHints)