[Metricbeat] Update Ceph module to support new API #16254

mtojek · 2020-02-11T18:56:14Z

Opening as draft to feel all fields and starting building using CI.

This PR updates Ceph module to use restful API introduced in latest releases.

How to test this PR locally

Use docker-compose up ceph to boot up Ceph Nautilus module. The port 5000 and 8003 should be exposed.
Enable ceph metricbeat module.
Configure module similarly to the mgr_cluster_disk_integration_test.go:

func getConfig(host, password string) map[string]interface{} {
	return map[string]interface{}{
		"module":                "ceph",
		"metricsets":            []string{"mgr_cluster_disk"},
		"hosts":                 []string{host},
		"username":              user,
		"password":              password,
		"ssl.verification_mode": "none",
	}
}

user - "demo"
password - go to https://localhost:5000/restful-list-keys.json
metricsets - include all: mgr_cluster_disk, mgr_cluster_health, mgr_osd_perf, mgr_osd_pool_stats, mgr_osd_tree, mgr_pool_disk

Start metricbeat with -e -d processors and observe events flow.

mtojek · 2020-02-11T20:21:58Z

jenkins, test this please

mtojek · 2020-02-12T19:04:53Z

Adding @jsoriano as there is an issue with nested objects.

I'm afraid that this might be a blocker for light module, hence considering either rewriting this or using some magic with processors.

jsoriano · 2020-02-13T09:46:05Z

metricbeat/module/ceph/docker-compose.yml

@@ -1,11 +1,23 @@
-version: '2.3'
+version: '3.7'


Is there any reason you need 3.7? We try to have the same version in all docker compose files because some times we use multiple of them in the same scenario, and we will do it more in the future for E2E testing.

3.X versions can be problematic for us because we make use of depends_on and extends, that are not supported in these versions focused on docker swarm. See these comments.

Fixed (forgot to clean this after experimenting).

jsoriano · 2020-02-13T10:05:39Z

metricbeat/module/ceph/mgr_cluster_disk/_meta/data.json

+                "num_osds": 1,
+                "num_per_pool_osds": 1
+            },
+            "pools": [


Adding @jsoriano as there is an issue with nested objects.

I'm afraid that this might be a blocker for light module, hence considering either rewriting this or using some magic with processors.

Yes, this kind of arrays of objects are unsupported, this question appears with some frequency, see this thread for example that also shows this problem with the http module.

Something similar was also discussed for the Azure input, there we receive messages formatted as JSON objects containing lists of records, we wanted an event per record, and there was no way to do it using processors. It was finally supported by handling this in the underlying Kafka input (see #13965). We could consider doing something similar for the http module.

The reason processors cannot be used for that, is that they are not able to generate multiple events from a single event, what is what we would need here.

For this specific data I think that ideally we should generate:

An event for the disk, that contains the global stats and maybe a summarization of the pools (as number of pools).

An event per pool, that also contains a field with the name of the disk for correlation.

A note about nested objects. We could make the effort of supporting them, and then we could store events like this one without unexpected results, but it would be an important effort with (in my opinion) little benefits as the information stored this way would be more difficult to query, and we could hardly take advantage of existing Kibana applications.

After adjusting the PR there is no need for nested arrays.

jsoriano · 2020-02-13T10:21:13Z

metricbeat/module/ceph/mgr_cluster_disk/manifest.yml

+      Content-Type: application/json
+    query:
+      wait: "1"
+    body: '{"prefix": "df", "format": "json"}'


We are collecting this information now from two metricsets, one for disks (cluster_disk), and another one for pools (pool_disk), they both make the same query (to /api/v0.1/df), but collect different data from the response.

We could keep this model and have two metricsets for the same data, this would allow a more intuitive transition from the old metricsets to the new ones. It would also allow to create by now the replacement for cluster_disk, that gets the stats for the disks and drops the pools array. And we can continue thinking on how to improve the http json metricset to support this kind of arrays.

Looking at the response objects once again, I came to the conclusion that there is too much wiring, removing and transforming stuff around to have a nice looking metric structure. I'm ok with rewriting this to standard metric sets.

mtojek · 2020-02-13T22:02:11Z

Alright, I've rewritten metricsets to a standard approach. The PR is ready to review.

jsoriano

I have added some comments, the only thing that could be important to address before merging is to decide if we could and should reuse old fields when possible. I think we should do that to ease migration and allow to reuse dashboards. It may be also important to do some modifications to the reference configuration files we are providing.

The rest are suggestions or things that could be addressed in the future.

metricbeat/module/ceph/mgr_osd_disk/mgr_osd_disk.go

metricbeat/module/ceph/_meta/config.yml

metricbeat/module/ceph/mgr_cluster_disk/mgr_cluster_disk.go

metricbeat/module/ceph/docker-compose.yml

jsoriano · 2020-02-16T14:14:03Z

metricbeat/module/ceph/mgr_cluster_disk/_meta/fields.yml

+      type: long
+      description: >
+        Used bytes of the cluster
+      format: bytes


Could we reuse the same field names as we are using with old modules? This way the transition will be easier and we will be able to reuse dashboards and so on.

Right now all field names are as much identical as possible. The difference is in metric set names:

- name: cluster_disk type: group description: > cluster_disk release: ga fields: - name: available.bytes type: long description: > Available bytes of the cluster format: bytes - name: total.bytes type: long description: > Total bytes of the cluster format: bytes - name: used.bytes type: long description: > Used bytes of the cluster format: bytes

and

- name: mgr_cluster_disk type: group description: > mgr_cluster_disk release: beta fields: - name: available.bytes type: long description: > Available bytes of the cluster format: bytes - name: total.bytes type: long description: > Total bytes of the cluster format: bytes - name: used.bytes type: long description: > Used bytes of the cluster format: bytes

metricbeat/module/ceph/mgr_cluster_disk/mgr_cluster_disk.go

metricbeat/module/ceph/mgr_cluster_disk/mgr_cluster_disk_test.go

jsoriano · 2020-02-16T14:26:27Z

metricbeat/module/ceph/mgr_cluster_health/_meta/data.json

+        "name": "host.example.com"
+    },
+    "ceph": {
+        "mgr_cluster_health": {


As mentioned in other comment, it'd be nice if we could use the same fields as old modules for common data. Here for example it could be cluster_health.

Suggested change

"mgr_cluster_health": {

"cluster_health": {

Unfortunately this is also metricset name, hence it can't be replaced (causes "metricset name already registered" error). Other fields are equal in this case.

The name of the metricset doesn't need to be neccesarily in the event, and in this case I think it would be beneficial to have equivalent fields in the same places.

To do that we can report events with the metrics at the module level:

reporter.Event(mb.Event{ ModuleFields: common.MapStr{ "cluster_health": event, }, })

Or even at the root level:

reporter.Event(mb.Event{ RootFields: common.MapStr{ "ceph": common.MapStr{ "cluster_health": event, }, }, })

If, for some reason, it is needed to differentiate per metricset in a query or dashboard, the event.dataset field can be used.

metricbeat/module/ceph/mgr_cluster_health/mgr_cluster_health.go

metricbeat/module/ceph/mgr_cluster_health/mgr_cluster_health_integration_test.go

mtojek · 2020-02-18T10:51:07Z

Also, rebased against master (update to python 3).

mtojek · 2020-02-18T13:16:49Z

jenkins, test this again please

jsoriano · 2020-02-18T17:32:27Z

libbeat/tests/compose/project.go

+// UpWithEnvironmentVariable adds environment variable.
+func UpWithEnvironmentVariable(keyValue string) UpOption {
+	return func(options *UpOptions) { options.Environment = append(options.Environment, keyValue) }
+}


This shouldn't be needed at the end, right?

Right. Removed entire feature.

jsoriano · 2020-02-18T17:34:15Z

metricbeat/module/ceph/mgrtest/compose.go

+	return compose.EnsureUp(t, "ceph",
+		compose.UpWithTimeout(upTimeout),
+		compose.UpWithEnvironmentVariable(fmt.Sprintf("CEPH_CODENAME=%s", defaultCodename)),
+		compose.UpWithEnvironmentVariable(fmt.Sprintf("CEPH_VERSION=%s", defaultVersion)),


Overriding the environment shouldn't be needed as we have now the defaults in the docker-compose.yml file.

Same here. Removed.

jsoriano · 2020-02-18T18:01:14Z

metricbeat/module/ceph/mgr_cluster_health/_meta/data.json

+        "name": "host.example.com"
+    },
+    "ceph": {
+        "mgr_cluster_health": {


The name of the metricset doesn't need to be neccesarily in the event, and in this case I think it would be beneficial to have equivalent fields in the same places.

To do that we can report events with the metrics at the module level:

reporter.Event(mb.Event{ ModuleFields: common.MapStr{ "cluster_health": event, }, })

Or even at the root level:

reporter.Event(mb.Event{ RootFields: common.MapStr{ "ceph": common.MapStr{ "cluster_health": event, }, }, })

If, for some reason, it is needed to differentiate per metricset in a query or dashboard, the event.dataset field can be used.

mtojek · 2020-02-18T19:15:56Z

To do that we can report events with the metrics at the module level:

Didn't know it's allowed to access them there. Cool!

Just want to highlight one thing - to properly validate all fields, some fields.yml files have been also updated (e.g. mgr_pool_disk -> pool_disk), which means that the aggregated fields.all.yml contain two same records. The same story should be for docs I believe, not sure about the (rendering) outcome...

jsoriano · 2020-02-18T19:20:16Z

Just want to highlight one thing - to properly validate all fields, some fields.yml files have been also updated (e.g. mgr_pool_disk -> pool_disk), which means that the aggregated fields.all.yml contain two same records. The same story should be for docs I believe, not sure about the (rendering) outcome...

Not sure if this is going to work. When we have had several metricsets using the same fields we have documented these fields in a single place, in only one of the metricsets or in the module. This avoids duplication and fields validation should still work.

mtojek · 2020-02-18T19:32:35Z

Just want to highlight one thing - to properly validate all fields, some fields.yml files have been also updated (e.g. mgr_pool_disk -> pool_disk), which means that the aggregated fields.all.yml contain two same records. The same story should be for docs I believe, not sure about the (rendering) outcome...

Not sure if this is going to work. When we have had several metricsets using the same fields we have documented these fields in a single place, in only one of the metricsets or in the module. This avoids duplication and fields validation should still work.

Fixed. Removed definitions from new metricsets.

mtojek · 2020-02-19T07:47:02Z

jenkins, test this please

jsoriano

A couple of questions I am ok with negative responses 🙂 So in any case, it LGTM.

metricbeat/module/ceph/mgr_osd_perf/_meta/data.json

metricbeat/module/ceph/mgr_osd_pool_stats/_meta/data.json

metricbeat/module/ceph/mgr_pool_disk/_meta/data.json

* add new ceph metricsets * adjust system tests (cherry picked from commit 273c6a8)

… new API (#16404) * [Metricbeat] Update Ceph module to support new API (#16254) * add new ceph metricsets * adjust system tests (cherry picked from commit 273c6a8) * Fix: version

* add new ceph metricsets * adjust system tests

mtojek added the Metricbeat Metricbeat label Feb 11, 2020

mtojek requested a review from a team February 11, 2020 18:56

mtojek self-assigned this Feb 11, 2020

mtojek added the Team:Services (Deprecated) Label for the former Integrations-Services team label Feb 11, 2020

mtojek marked this pull request as ready for review February 12, 2020 19:03

mtojek requested a review from jsoriano February 12, 2020 19:03

jsoriano reviewed Feb 13, 2020

View reviewed changes

andresrc added [zube]: Inbox [zube]: In Progress and removed [zube]: Inbox labels Feb 13, 2020

mtojek requested review from a team and jsoriano February 13, 2020 22:02

mtojek added [zube]: In Review and removed [zube]: In Progress labels Feb 14, 2020

zube bot added [zube]: In Progress [zube]: In Review and removed [zube]: In Review [zube]: In Progress labels Feb 14, 2020

jsoriano reviewed Feb 16, 2020

View reviewed changes

mtojek requested review from jsoriano and a team February 18, 2020 10:51

jsoriano reviewed Feb 18, 2020

View reviewed changes

Another review round

0672054

mtojek requested review from jsoriano and a team February 18, 2020 19:18

Define fields only in original metricset

b1550b7

jsoriano approved these changes Feb 19, 2020

View reviewed changes

metricbeat/module/ceph/mgr_osd_perf/_meta/data.json Show resolved Hide resolved

metricbeat/module/ceph/mgr_osd_pool_stats/_meta/data.json Show resolved Hide resolved

metricbeat/module/ceph/mgr_pool_disk/_meta/data.json Show resolved Hide resolved

mtojek merged commit 273c6a8 into elastic:master Feb 19, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Feb 19, 2020

mtojek added a commit to mtojek/beats that referenced this pull request Feb 19, 2020

[Metricbeat] Update Ceph module to support new API (elastic#16254)

abfcc97

* add new ceph metricsets * adjust system tests (cherry picked from commit 273c6a8)

mtojek mentioned this pull request Feb 19, 2020

Cherry-pick #16254 to 7.x: [Metricbeat] Update Ceph module to support new API #16404

Merged

mtojek added the v7.7.0 label Feb 19, 2020

mtojek mentioned this pull request Feb 19, 2020

Update changelog with changes in PR 16254 #16405

Merged

mtojek mentioned this pull request Feb 19, 2020

Cherry-pick #16405 to 7.x: Update changelog with changes in PR 16254 #16416

Merged

kvch pushed a commit to kvch/beats that referenced this pull request Feb 20, 2020

[Metricbeat] Update Ceph module to support new API (elastic#16254)

60b7665

* add new ceph metricsets * adjust system tests

andresrc removed the [zube]: Done label Feb 24, 2020

toha70 mentioned this pull request Feb 26, 2020

Update Ceph module to support new API #7723

Closed

mtojek added the test-plan Add this PR to be manual test plan label Mar 16, 2020

andresrc added the test-plan-added This PR has been added to the test plan label Mar 21, 2020

sorantis mentioned this pull request Apr 3, 2020

Metricbeat Ceph stats collection breaks for Ceph Mimic release #7429

Closed

jsoriano mentioned this pull request Jan 21, 2021

chore: increase timeout for Ceph elastic/e2e-testing#618

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Metricbeat] Update Ceph module to support new API #16254

[Metricbeat] Update Ceph module to support new API #16254

mtojek commented Feb 11, 2020 •

edited

Loading

mtojek commented Feb 11, 2020

mtojek commented Feb 12, 2020 •

edited

Loading

jsoriano Feb 13, 2020

mtojek Feb 18, 2020

jsoriano Feb 13, 2020

mtojek Feb 18, 2020

jsoriano Feb 13, 2020

mtojek Feb 13, 2020

mtojek commented Feb 13, 2020

jsoriano left a comment •

edited

Loading

jsoriano Feb 16, 2020

mtojek Feb 18, 2020

jsoriano Feb 16, 2020

mtojek Feb 18, 2020 •

edited

Loading

jsoriano Feb 18, 2020

mtojek commented Feb 18, 2020

mtojek commented Feb 18, 2020

jsoriano Feb 18, 2020

mtojek Feb 18, 2020

jsoriano Feb 18, 2020

mtojek Feb 18, 2020

jsoriano Feb 18, 2020

mtojek commented Feb 18, 2020 •

edited

Loading

jsoriano commented Feb 18, 2020 •

edited

Loading

mtojek commented Feb 18, 2020

mtojek commented Feb 19, 2020

jsoriano left a comment

[Metricbeat] Update Ceph module to support new API #16254

[Metricbeat] Update Ceph module to support new API #16254

Conversation

mtojek commented Feb 11, 2020 • edited Loading

How to test this PR locally

mtojek commented Feb 11, 2020

mtojek commented Feb 12, 2020 • 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

mtojek commented Feb 13, 2020

jsoriano left a comment • 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

mtojek Feb 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtojek commented Feb 18, 2020

mtojek commented Feb 18, 2020

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

mtojek commented Feb 18, 2020 • edited Loading

jsoriano commented Feb 18, 2020 • edited Loading

mtojek commented Feb 18, 2020

mtojek commented Feb 19, 2020

jsoriano left a comment

Choose a reason for hiding this comment

mtojek commented Feb 11, 2020 •

edited

Loading

mtojek commented Feb 12, 2020 •

edited

Loading

jsoriano left a comment •

edited

Loading

mtojek Feb 18, 2020 •

edited

Loading

mtojek commented Feb 18, 2020 •

edited

Loading

jsoriano commented Feb 18, 2020 •

edited

Loading