Skip to content

Commit

Permalink
Adding validate flag for GenerateHints (#89)
Browse files Browse the repository at this point in the history 
Based on the
https://github.com/elastic/beats/pull/38213/files#r1523532163, we
consider adding a flag to enable the validation of GenerateHints.

The specific function is called in other places apart form kubernetes
and we dont want to add additional function calls to other beats
  • Loading branch information
@gizas
gizas authored Apr 22, 2024
1 parent fff9210 commit a399d04
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 65 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,13 @@ This project adheres to [Semantic Versioning](http://semver.org/).


[0.6.12]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.10...v0.6.12


## [0.6.13]

### Changed

- Enhance GenerateHints function with validate flag to enable the validation of hints


[0.6.13]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.12...v0.6.13
132 changes: 68 additions & 64 deletions utils/hints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ func IsDisabled(hints mapstr.M, key string) bool {
}

// GenerateHints parses annotations based on a prefix and sets up hints that can be picked up by individual Beats.
func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedHints []string) (mapstr.M, []string) {
// Arguments: annotations: include the annotatons defined in the container, container: is the container name,
// prefix: the prefix of the annotation to check, validate: boolean variable that enables the validation of hints format and allSupportedHints: list of supported annotations to validate against
func GenerateHints(annotations mapstr.M, container, prefix string, validate bool, allSupportedHints []string) (mapstr.M, []string) {
hints := mapstr.M{}
var incorrecthints []string
var incorrecthint string
Expand All @@ -212,22 +214,23 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH
if rawEntries, err := annotations.GetValue(prefix); err == nil {
if entries, ok := rawEntries.(mapstr.M); ok {

//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
if validate {
//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{}
Expand All @@ -237,32 +240,33 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH
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)
if validate {
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 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
_, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints)
//We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic
_, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints)
} //end of annotation check

// Insert only if there is no entry already. container level annotations take
// higher priority.
Expand All @@ -286,34 +290,34 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH
// 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)
if validate {
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
_, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints)
// 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)
}

//end of check
}
//We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic
_, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints)
} //end of annotation check

if len(parts) == 2 {
// key will be the hint type
Expand All @@ -326,7 +330,7 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH
}
}
}
if incorrecthint != "" {
if validate && incorrecthint != "" {
incorrecthints = append(incorrecthints, incorrecthint)
}
}
Expand Down
104 changes: 103 additions & 1 deletion utils/hints_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,113 @@ func TestGenerateHints(t *testing.T) {
}
}

generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", allSupportedHints)
generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", true, 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)
}
}

func TestGenerateHintsWithValidatedisabled(t *testing.T) {

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
annotations map[string]string
result mapstr.M
expectedIncorrectHints int // We set the number of hints that will be marked as incorrect and wont be included in the acceptable supported list
}{

// 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: 0, // Validate flag= false in GenerateHints
},
// 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: 0, // Validate flag= false in GenerateHints
},
// 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: 0, // Validate flag= false in GenerateHints
},
}

for _, test := range tests {
annMap := mapstr.M{}
for k, v := range test.annotations {
_, err := annMap.Put(k, v)
if err != nil {
continue
}
}

generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", false, 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)
}
}

func TestGetHintsAsList(t *testing.T) {
tests := []struct {
input mapstr.M
Expand Down

0 comments on commit a399d04

Please sign in to comment.