-
Couldn't load subscription status.
- Fork 128
Support policy tests in input packages with input type otelcol #2873
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
Conversation
This reverts commit b649672.
| @@ -0,0 +1 @@ | |||
| 9.2.0-SNAPSHOT | |||
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.
Forced to run this package with 9.2.0-SNAPSHOT.
This should be removed once the default version is 9.2.0 or higher.
| receivers: | ||
| httpcheck/b0f518d6-4e2d-4c5d-bda7-f9808df537b7: | ||
| collection_interval: 1m | ||
| targets: | ||
| - endpoints: | ||
| - https://epr.elastic.co | ||
| method: GET |
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.
@jsoriano in these policies, could it be more than one receiver/exporter/extensions/processors?
Focusing on receiver as example, if there is another receiver defined, would it start also with httpcheck ?
receivers: httpcheck/b0f518d6-4e2d-4c5d-bda7-f9808df537b7: collection_interval: 1m targets: - endpoints: - https://epr.elastic.co method: GET httpcheck/b0f518d6-4e2d-4c5d-bda7-11111111111: collection_interval: 2m targets: - endpoints: - https://example.com method: GET If that could happen, the resulting yaml would contain just one receiver httpcheck/componentid
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.
In practice I don't think this has a real use case for receivers, but in theory this will be allowed, yes. And it will be definitely possible with processors.
So you are right that we may need some replacement that is not always the same to avoid collisions, but is still predictable.
| /test |
| processors: | ||
| batch/11c35ad0-4351-49d4-9c78-fa679ce9d950: | ||
| send_batch_size: 10 | ||
| timeout: 1s | ||
| batch/e6e379c5-6446-4090-af10-a9e5f8fc4640: | ||
| send_batch_size: 10000 | ||
| timeout: 10s |
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.
With the current implementation, this could fail in some test executions:
--- FAIL: TestComparePolicies (0.00s) --- FAIL: TestComparePolicies/otel_ids (0.00s) policy_test.go:472: Error Trace: /home/mariorodriguez/Coding/work/elastic-package-otel/internal/testrunner/runners/policy/policy_test.go:472 Error: Should be empty, but was --- want +++ got @@ -17,4 +17,4 @@ batch/componentid: - send_batch_size: 10 - timeout: 1s + send_batch_size: 10000 + timeout: 10s receivers: Test: TestComparePolicies/otel_ids FAIL FAIL github.com/elastic/elastic-package/internal/testrunner/runners/policy 0.061s FAIL Working to replace the OTEL ids (directly on the []byte variable) before running the cleanup process.
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.
Hopefully, this is solved by replacing the OTEL IDs before cleaning up the policy with the current filters. This was added in 50de2aa
| /test |
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.
Looks good overall, added some suggestions. In any case we will need to make changes here as the E2E implementation consolidates, so we will have opportunities to revisit this.
| - httpcheck/4bae34b3-8f66-49c1-b04f-d58af1b5f743 | ||
| processors: | ||
| - batch/11c35ad0-4351-49d4-9c78-fa679ce9d950 | ||
| - batch/e6e379c5-6446-4090-af10-a9e5f8fc4640 |
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 is likely going to change as more plumbing is needed, see elastic/kibana#233090.
But we can go on with this change and regenerate files later.
| policy = otelComponentIDsRegexp.ReplaceAllFunc(policy, func(match []byte) []byte { | ||
| count := 0 | ||
| lines := bytes.Split(match, []byte("\n")) | ||
| for i, line := range lines { | ||
| line = bytes.TrimRight(line, "\r\n") | ||
| stringLine := string(line) | ||
| if strings.Contains(stringLine, ":") { | ||
| if uniqueOtelComponentIDReplace.regexp.MatchString(stringLine) { | ||
| replacement := fmt.Sprintf(uniqueOtelComponentIDReplace.replace, strconv.Itoa(count)) | ||
| count++ | ||
| lines[i] = []byte(uniqueOtelComponentIDReplace.regexp.ReplaceAllString(stringLine, replacement)) | ||
| | ||
| // store the otel ID replaced without the space indentation and the colon to be replaced later | ||
| // (e.g. http_check/4391d954-1ffe-4014-a256-5eda78a71828 replaced by http_check/componentid-0) | ||
| otelID := strings.SplitN(strings.TrimSpace(stringLine), ":", 2)[0] | ||
| replacementsDone[otelID] = strings.SplitN(strings.TrimSpace(string(lines[i])), ":", 2)[0] | ||
| } | ||
| } | ||
| } | ||
| return bytes.Join(lines, []byte("\n")) | ||
| }) |
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.
Is it neccesary to take into account the root level? Couldn't we directly replace the matching ids?
| policy = otelComponentIDsRegexp.ReplaceAllFunc(policy, func(match []byte) []byte { | |
| count := 0 | |
| lines := bytes.Split(match, []byte("\n")) | |
| for i, line := range lines { | |
| line = bytes.TrimRight(line, "\r\n") | |
| stringLine := string(line) | |
| if strings.Contains(stringLine, ":") { | |
| if uniqueOtelComponentIDReplace.regexp.MatchString(stringLine) { | |
| replacement := fmt.Sprintf(uniqueOtelComponentIDReplace.replace, strconv.Itoa(count)) | |
| count++ | |
| lines[i] = []byte(uniqueOtelComponentIDReplace.regexp.ReplaceAllString(stringLine, replacement)) | |
| // store the otel ID replaced without the space indentation and the colon to be replaced later | |
| // (e.g. http_check/4391d954-1ffe-4014-a256-5eda78a71828 replaced by http_check/componentid-0) | |
| otelID := strings.SplitN(strings.TrimSpace(stringLine), ":", 2)[0] | |
| replacementsDone[otelID] = strings.SplitN(strings.TrimSpace(string(lines[i])), ":", 2)[0] | |
| } | |
| } | |
| } | |
| return bytes.Join(lines, []byte("\n")) | |
| }) | |
| policy = uniqueOtelComponentIDReplace.regexp.ReplaceAllStringFunc(policy, func(match string) string { | |
| replacement := fmt.Sprintf(uniqueOtelComponentIDReplace.replace, strconv.Itoa(count)) | |
| replaced := uniqueOtelComponentIDReplace.regexp.ReplaceAllString(match, replacement) | |
| otelID, _, _ := strings.Cut(strings.TrimSpace(match), ":") | |
| replacementsDone[otelID], _, _ = strings.Cut(strings.TrimSpace(string(replaced)), ":") | |
| }) |
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.
Maybe it could be done like that, but it could not be ensured that all the replacements performed would be related to OTEL.
The current implementation, at least, looks for OTEL IDs just in the block definitions detailed by the regexp otelComponentIDsRegexp:
var otelComponentIDsRegexp = regexp.MustCompile(`(?m)^(?:extensions|receivers|processors|connectors|exporters):(?:\s\{\}\n|\n(?:\s{2,}.+\n)+)`)I'd prefer to keep it for now like this to avoid replacing unexpected strings in other settings/fields while validating these policies.
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
| test integrations |
| Created or updated PR in integrations repository to test this version. Check elastic/integrations#15254 |
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.
👍
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
💚 Build Succeeded
History
cc @mrodm |
Supersedes #2799
Relates #2835
Relates elastic/kibana#227673
Add support to run policy tests in input packages defining a policy template with the input type
otelcol. Continuing the work done in #2799In order to support policy tests in those packages, it is needed to add filters to remove the ids that may appear in policies with OTel configuration. Some examples:
This PR adds a new test package
httpcheckthat includes this new input type (copied from elastic/integrations#14315).In order to test this new input type, this PR also enables the feature flag
enableOtelIntegrationin Kibana configuration file. That feature flag was added in elastic/kibana#227673.Examples:
How to test this PR locally
elastic-package stack up -v -d --version 9.2.0-SNAPSHOT elastic-package -C test/packages/parallel/httpcheck test policy -v elastic-package stack down -v