Add system tests to logstash package #4443

crespocarlos · 2022-10-11T16:36:47Z

What does this PR do?

This PR adds system tests to the Logstash package

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
~~I have added an entry to my package's changelog.yml file.~~
I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

Run cd packages/logstash && elastic-package test system

Related issues

Closes [Stack Monitoring] Add system tests #4008

(cherry picked from commit 4df6b991a6db0cb851598c34063363d90f1c0749)

elasticmachine · 2022-10-11T16:46:53Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-10-14T11:04:06.024+0000
Duration: 18 min 6 sec

Test stats 🧪

Test	Results
Failed	0
Passed	25
Skipped	0
Total	25

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2022-10-11T19:02:28Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`2/2`)	💚
Files	100.0% (`6/6`)	💚 2.783
Classes	100.0% (`6/6`)	💚 2.783
Methods	100.0% (`46/46`)	💚 10.188
Lines	87.558% (`190/217`)	👎 -3.909
Conditionals	100.0% (`0/0`)	💚

crespocarlos · 2022-10-11T19:16:35Z

/test

elasticmachine · 2022-10-12T11:58:12Z

🚀 Benchmarks report

Package `logstash` 👍(0) 💚(1) 💔(1)

Expand to view

Data stream	Previous EPS	New EPS	Diff (%)	Result
`slowlog`	4098.36	3344.48	-753.88 (-18.39%)	💔

To see the full report comment with /test benchmark fullreport

crespocarlos · 2022-10-14T08:35:58Z

packages/logstash/_dev/deploy/docker/docker-compose.yml

 services:
  logstash:
-    image: "docker.elastic.co/logstash/logstash:${ELASTIC_VERSION}"
+    user: root


The container needs to be accessed by root user to have permission to write in the ${SERVICE_LOGS_DIR} folder

crespocarlos · 2022-10-14T08:37:08Z

packages/logstash/_dev/deploy/docker/docker-compose.yml

 services:
  logstash:
-    image: "docker.elastic.co/logstash/logstash:${ELASTIC_VERSION}"
+    user: root
+    image: "docker.elastic.co/logstash/logstash:${ELASTIC_VERSION:-8.5.0-SNAPSHOT}"


.env file doesn't work in the CI, 8.5.0-SNAPSHOT is the default value, in case the var is null. We need to figure out how to make this a dynamic value.

Looks like we can create a variants.yml file that defines all the versions we want to run the test suite against. The variable defined in the variants file are then available in the docker compose - see #4013 (comment)

ah. nice! I'll try this

klacabane · 2022-10-14T09:41:49Z

packages/logstash/data_stream/node_stats/fields/package-fields.yml

@@ -55,7 +55,7 @@
          type: group
          fields:
            - name: stat
-              type: group
+              type: object


Is there any difference between object and group types ?

The only difference here is that the static tests won't fail in case stat receives a null value. It will act as a json object too https://www.elastic.co/guide/en/elasticsearch/reference/current/object.html. In practice, I didn't see any difference

klacabane · 2022-10-14T09:54:01Z

packages/logstash/data_stream/node_stats/fields/fields.yml

                    - name: load_average
                      type: group
                      fields:
                        - name: 15m
-                          type: long
+                          type: half_float


The agent mappings will become the source of truth and we should eventually align the metricbeat mappings (in ES and in metricbeat repo) on this version once we're confident. We could potentially improve the existing mappings tooling to be smarter so it can interpret the diffs and patch the repos accordingly

Yeah. This change was actually from a PR @matschaffer worked on, which I happen to have reviewed and remembered about. We need to be careful now when updating metricbeat mappings. Do we have any tooling to keep them aligned?

There's a script here but it only detects the deltas and we'd need something to act on them. With .monitoring-* being source of truth it's difficult to automatically apply deltas to agent or metricbeat mappings because the latter have one definition per metricset while the former is just one json object, so we don't know the target metricset to patch unless we have a manual step in between. With the agent mappings being source of truth we could do the following:

update agent mapping

detect delta with .monitoring-* mapping and add/update/delete the entry. We can simply patch the json blob here

detect delta with the metricset's mappings in the metricbeat module. We have 1:1 relationship with data stream/metricset so we know which fields to update in the metricbeat module

I'm thinking we should open a follow up issue to sweep mappings for other mis-alignments. We've had a number of recent tweaks to fix bugs that were ultimately rooted in things not being in sync between metricbeat and the ES-shipped templates.

nice, @miltonhultgren beat me to it elastic/kibana#143439 :)

packages/logstash/data_stream/node/_dev/test/system/test-default-config.yml

Add system tests for logstash package

03670c2

(cherry picked from commit 4df6b991a6db0cb851598c34063363d90f1c0749)

Update README

72dfc90

crespocarlos changed the title ~~Add system tests for logstash package~~ Add system tests to logstash package Oct 11, 2022

Test .env file

6f69d52

crespocarlos added 2 commits October 12, 2022 14:49

Set default env var value

9b9429d

Fix alias mapping

3fdd75c

crespocarlos force-pushed the 4008-logstash-package-system-tests branch 5 times, most recently from 7d91acc to f050889 Compare October 12, 2022 18:44

Fix mapping and use Hostname var on test config

8bb0994

crespocarlos force-pushed the 4008-logstash-package-system-tests branch from f050889 to 8bb0994 Compare October 12, 2022 19:12

crespocarlos added 2 commits October 12, 2022 21:32

Add missing filed to node_stats mapping

3b4ffe8

Fix docker-compose file

ec8d0b5

crespocarlos marked this pull request as ready for review October 14, 2022 08:35

crespocarlos requested a review from a team as a code owner October 14, 2022 08:35

crespocarlos commented Oct 14, 2022

View reviewed changes

klacabane reviewed Oct 14, 2022

View reviewed changes

packages/logstash/data_stream/node/_dev/test/system/test-default-config.yml Outdated Show resolved Hide resolved

Fix test config and replaces .env with variantes.yml

30fc344

klacabane approved these changes Oct 14, 2022

View reviewed changes

matschaffer mentioned this pull request Oct 17, 2022

[Stack Monitoring] Review diff between documented and shipped fields elastic/kibana#143439

Open

crespocarlos added Integration:logstash Logstash Team:Infra Monitoring UI - DEPRECATED Label for the Infrastructure Monitoring UI team. - DEPRECATED - Use Team:obs-ux-infra_services labels Oct 27, 2022

crespocarlos merged commit 094a533 into elastic:main Nov 1, 2022

klacabane mentioned this pull request Nov 15, 2022

[logstash] fix mappings #4594

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add system tests to logstash package #4443

Add system tests to logstash package #4443

crespocarlos commented Oct 11, 2022 •

edited

Loading

elasticmachine commented Oct 11, 2022 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Oct 11, 2022 •

edited

Loading

crespocarlos commented Oct 11, 2022

elasticmachine commented Oct 12, 2022 •

edited

Loading

crespocarlos Oct 14, 2022

crespocarlos Oct 14, 2022

klacabane Oct 14, 2022 •

edited

Loading

crespocarlos Oct 14, 2022

klacabane Oct 14, 2022

crespocarlos Oct 14, 2022

klacabane Oct 14, 2022

crespocarlos Oct 14, 2022

klacabane Oct 14, 2022

matschaffer Oct 17, 2022

matschaffer Oct 17, 2022

Add system tests to logstash package #4443

Add system tests to logstash package #4443

Conversation

crespocarlos commented Oct 11, 2022 • edited Loading

What does this PR do?

Checklist

How to test this PR locally

Related issues

elasticmachine commented Oct 11, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Oct 11, 2022 • edited Loading

🌐 Coverage report

crespocarlos commented Oct 11, 2022

elasticmachine commented Oct 12, 2022 • edited Loading

🚀 Benchmarks report

Package logstash 👍(0) 💚(1) 💔(1)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klacabane Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos commented Oct 11, 2022 •

edited

Loading

elasticmachine commented Oct 11, 2022 •

edited

Loading

elasticmachine commented Oct 11, 2022 •

edited

Loading

elasticmachine commented Oct 12, 2022 •

edited

Loading

Package `logstash` 👍(0) 💚(1) 💔(1)

klacabane Oct 14, 2022 •

edited

Loading