Enhance RabbitMQ test to use real data and test with 3.6 #10667

sayden · 2019-02-11T11:45:24Z

"Fixes" this issue #10014 by:

Using data returned from an instance
The instance is version 3.6

Currently, RabbitMQ module uses local data stored here to mock responses from the server during tests. This also generates file data.json which is not very nice.

AFAIK, the "missing data" reported in the issue are some fields that aren't parsed by design.

Current PR uses exchange Metricset to actively send messages and consume them from a RabbitMQ instance, to later produce a real data.json file.

My main concern about this PR are:

Now we need a third party library (https://github.com/streadway/amqp) althought as it's only used on tests, it's not shipped with the release binary.
"Missing fields" reported in the issue and the discuss forum aren't being parsed yet. So, that problem isn't solved anyways.

Comments welcome :)

EDIT:
Finally I added some version specific tests using the new framework and updated to_error V2_ interface_. I couldn't update Metricset node to use the new testing framework because it needs 2 HTTP calls to fetch metrics (and the testing framework it only with single HTTP calls at the moment)

ruflin

If we have new dependencies we need to add them with govendor to the repo as otherwise CI does not find them.

It seems the main objective of this PR is to generate a nicer data.json. For this to happen I wonder if we should better go the approach suggested here? #10648

ruflin · 2019-02-12T12:56:16Z

metricbeat/module/rabbitmq/exchange/exchange_test.go

@@ -61,16 +65,27 @@ func TestFetchEventContents(t *testing.T) {
 }

 func TestData(t *testing.T) {
-	server := mtest.Server(t, mtest.DefaultServerConfig)
-	defer server.Close()
+	t.Skip("Skipping `data.json` generation test")


There is a check in the WriteEventsReporterV2 for the -data flag. If we want to skip it already here, we should do it based on the -data flag instead of always skipping it.

sayden · 2019-03-13T12:19:08Z

I'll take advantage of the new testing interface now on this PR

ruflin · 2019-04-15T07:15:03Z

metricbeat/module/rabbitmq/node/_meta/data.json

-        "name": "host.example.com"
+    "@timestamp": "2019-03-01T08:05:34.853Z",
+    "error": {
+        "message": "HTTP error 404 in : 404 Not Found"


Error document as example? :-)

ruflin · 2019-04-15T07:15:26Z

metricbeat/module/rabbitmq/node/_meta/testdata/docs.json-expected.json

+[
+    {
+        "error": {
+            "message": "HTTP error 404 in : 404 Not Found"


Not sure if this is expected?

ruflin · 2019-04-15T07:15:50Z

metricbeat/module/rabbitmq/node/node_integration_test.go

-	}
-}
-
-func TestFetch(t *testing.T) {


Should we keep this around even though it's skipped. This is as a reminder that we should fix it.

sayden · 2019-05-06T18:35:05Z

jenkins, test this please

sayden · 2019-05-06T21:15:30Z

Updated the title to show the current state.

jsoriano

Thanks for working on this.
Please update the compatibility notes mentioning that it should work with 3.6 too.
It'd be also good if we could test the node metricset with multiple versions, but not sure of the best way of doing it.

sayden · 2019-05-29T10:37:13Z

I passed this PR to ready again to wait until #12228 is in. Then I'll merge conflict and merge.

sayden · 2019-06-07T12:31:13Z

Ok, conflicts solved :) Ready for review / merging

EDIT: Ups, I forgot to update the compatibility docs. Doing it now.

kaiyan-sheng · 2019-06-10T15:04:42Z

metricbeat/docs/modules/rabbitmq.asciidoc

@@ -14,7 +14,7 @@ If `management.path_prefix` is set in RabbitMQ configuration, `management_path_p
 [float]
 === Compatibility

-The rabbitmq module is tested with RabbitMQ 3.7.4, and it should be compatible
+The rabbitmq module is fully tested with RabbitMQ 3.7.4 and partially tested with 3.6.0, 3.6.5 and 3.7.14 and it should be compatible


Do we need to be more specific on "partially tested" here?

kaiyan-sheng

I'm just curious if we should be more specific on "partially tested". Other than that it looks good to me!

ruflin · 2019-06-11T06:19:12Z

+1 on what @kaiyan-sheng says related to partial testing. Either we are confident it works and is supported or we should not mention it. Otherwise it's consuming for the customer on what to expect and also from a support perspective, if we support it or not.

sayden · 2019-07-05T08:45:07Z

Text have been changed to reflect what's exactly tested with which version

sayden · 2019-07-05T08:45:22Z

jenkins, test this please

sayden added in progress Pull request is currently in progress. Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Feb 11, 2019

sayden self-assigned this Feb 11, 2019

sayden requested a review from ruflin February 11, 2019 11:45

sayden requested a review from a team as a code owner February 11, 2019 11:45

sayden requested a review from jsoriano February 11, 2019 11:45

ruflin reviewed Feb 12, 2019

View reviewed changes

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 43a8c2e to 30e39b0 Compare April 12, 2019 11:34

sayden requested a review from a team as a code owner April 12, 2019 11:34

ruflin reviewed Apr 15, 2019

View reviewed changes

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 2c22bae to 246cf5b Compare April 15, 2019 11:35

alvarolobato added the [zube]: In Progress label Apr 23, 2019

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 246cf5b to 38c94b6 Compare April 23, 2019 18:50

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 38c94b6 to 5bc7e22 Compare May 3, 2019 15:21

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 5bc7e22 to 718b00a Compare May 6, 2019 21:11

sayden added review [zube]: In Review and removed in progress Pull request is currently in progress. [zube]: In Progress labels May 6, 2019

jsoriano reviewed May 7, 2019

View reviewed changes

jsoriano mentioned this pull request May 20, 2019

[Libbeat][Metricbeat]Add IgnoreAllErrors to schema.Conv object #12089

Merged

sayden added [zube]: Ready and removed [zube]: In Review labels May 29, 2019

sayden added blocked and removed review labels May 29, 2019

sayden removed the blocked label Jun 5, 2019

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 6327d63 to 27a0b1f Compare June 7, 2019 10:52

sayden added the in progress Pull request is currently in progress. label Jun 7, 2019

sayden added review and removed in progress Pull request is currently in progress. labels Jun 7, 2019

kaiyan-sheng reviewed Jun 10, 2019

View reviewed changes

kaiyan-sheng approved these changes Jun 10, 2019

View reviewed changes

sayden added 4 commits June 11, 2019 15:51

Atomic commit

d33fd03

make fmt

cd128c7

Update compatibility docs

6354b32

Docs updated

79bc558

sayden force-pushed the bugfix/mb/rabbitmq-fix-test branch from 8e61fb3 to 79bc558 Compare June 11, 2019 13:52

andresrc added [zube]: In Review and removed [zube]: Ready labels Jun 11, 2019

sayden merged commit 1138587 into elastic:master Jul 5, 2019

sayden changed the title ~~Enhance RabbitMQ test to use real data and test with 3.6~~ Enhance RabbitMQ tests with more versions other than 3.6 Jul 5, 2019

zube bot changed the title ~~Enhance RabbitMQ tests with more versions other than 3.6~~ Enhance RabbitMQ test to use real data and test with 3.6 Jul 5, 2019

zube bot added [zube]: Done and removed [zube]: In Review labels Jul 5, 2019

andresrc removed the [zube]: Done label Jul 8, 2019

jsoriano mentioned this pull request Jul 22, 2019

Missing data in events collected by RabbitMQ metricbeat module #10014

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance RabbitMQ test to use real data and test with 3.6 #10667

Enhance RabbitMQ test to use real data and test with 3.6 #10667

sayden commented Feb 11, 2019 •

edited

Loading

ruflin left a comment

ruflin Feb 12, 2019

sayden commented Mar 13, 2019

ruflin Apr 15, 2019

ruflin Apr 15, 2019

ruflin Apr 15, 2019

sayden commented May 6, 2019

sayden commented May 6, 2019

jsoriano left a comment

sayden commented May 29, 2019

sayden commented Jun 7, 2019 •

edited

Loading

kaiyan-sheng Jun 10, 2019

kaiyan-sheng left a comment

ruflin commented Jun 11, 2019

sayden commented Jul 5, 2019

sayden commented Jul 5, 2019

Enhance RabbitMQ test to use real data and test with 3.6 #10667

Enhance RabbitMQ test to use real data and test with 3.6 #10667

Conversation

sayden commented Feb 11, 2019 • edited Loading

ruflin left a comment

Choose a reason for hiding this comment

ruflin Feb 12, 2019

Choose a reason for hiding this comment

sayden commented Mar 13, 2019

ruflin Apr 15, 2019

Choose a reason for hiding this comment

ruflin Apr 15, 2019

Choose a reason for hiding this comment

ruflin Apr 15, 2019

Choose a reason for hiding this comment

sayden commented May 6, 2019

sayden commented May 6, 2019

jsoriano left a comment

Choose a reason for hiding this comment

sayden commented May 29, 2019

sayden commented Jun 7, 2019 • edited Loading

kaiyan-sheng Jun 10, 2019

Choose a reason for hiding this comment

kaiyan-sheng left a comment

Choose a reason for hiding this comment

ruflin commented Jun 11, 2019

sayden commented Jul 5, 2019

sayden commented Jul 5, 2019

sayden commented Feb 11, 2019 •

edited

Loading

sayden commented Jun 7, 2019 •

edited

Loading