Add oplog metricset to mongodb module by a3dho3yn · Pull Request #7604 · elastic/beats

a3dho3yn · 2018-07-15T08:16:21Z

Oplog size and window are two important metrics which show replication health.
With this metric set, we can have this information about replication:

{"mongodb": {
  "oplog": {
    "size": {
      "allocated": 2605587456,
      "used": 2616684138
    },
    "first": {
       "ts": 6515806468564845000
    },
    "last": {
       "ts": 6578335797915681000
    },
    "window": 62529329350836220
  }
}}

elasticmachine · 2018-07-15T08:16:23Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2018-07-15T08:16:27Z

metricbeat/module/mongodb/oplog/oplog_integration_test.go

+	used := event["size"].(common.MapStr)["used"].(int64)
+	assert.True(t, used > 0)
+
+	first_ts := event["first"].(common.MapStr)["ts"].(int64)


don't use underscores in Go names; var first_ts should be firstTs

houndci-bot · 2018-07-15T08:16:27Z

metricbeat/module/mongodb/oplog/oplog.go

+
+	// get first and last items in the oplog
+	oplog_iter := collection.Find(nil).Sort("$natural").Iter()
+	oplog_reverse_iter := collection.Find(nil).Sort("-$natural").Iter()


don't use underscores in Go names; var oplog_reverse_iter should be oplogReverseIter

houndci-bot · 2018-07-15T08:16:27Z

metricbeat/module/mongodb/oplog/oplog.go

+	used := int64(oplogStatus["size"].(float64))
+
+	// get first and last items in the oplog
+	oplog_iter := collection.Find(nil).Sort("$natural").Iter()


don't use underscores in Go names; var oplog_iter should be oplogIter

houndci-bot · 2018-07-15T08:16:27Z

metricbeat/module/mongodb/oplog/oplog.go

+	return false
+}
+
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {


exported function New should have comment or be unexported

houndci-bot · 2018-07-15T08:16:28Z

metricbeat/module/mongodb/oplog/oplog.go

+		mb.DefaultMetricSet())
+}
+
+type MetricSet struct {


exported type MetricSet should have comment or be unexported

houndci-bot · 2018-07-15T08:16:28Z

metricbeat/module/mongodb/oplog/oplog.go

+	"gopkg.in/mgo.v2/bson"
+)
+
+const oplog_col = "oplog.rs"


don't use underscores in Go names; const oplog_col should be oplogCol

kvch · 2018-07-16T07:27:30Z

jenkins test this

ruflin · 2018-07-16T07:35:13Z

Could you add a changelog entry?

jsoriano

Thanks for working on this! :) It looks quite good, I have added some comments, the only serious thing is the failing test.

jsoriano · 2018-07-16T15:29:08Z

metricbeat/module/mongodb/oplog/oplog.go

+var debugf = logp.MakeDebug("mongodb.oplog")
+
+func init() {
+	logp.Info("initializing oplog")


We don't use to log initializations.

jsoriano · 2018-07-16T15:32:03Z

metricbeat/docs/fields.asciidoc

+
+--
+
+*`mongodb.oplog.last.ts`*::


I wouldn't abbreviate timestamps field names, what about first.time or first.timestamp?

jsoriano · 2018-07-16T15:36:20Z

metricbeat/module/mongodb/oplog/oplog.go

+	}
+
+	firstTs := int64(first.(bson.M)["ts"].(bson.MongoTimestamp))
+	lastTs := int64(last.(bson.M)["ts"].(bson.MongoTimestamp))


Add checks for type conversions here

jsoriano · 2018-07-16T15:44:47Z

metricbeat/module/mongodb/oplog/oplog_integration_test.go

+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.


Add this selector so this is executed only when running integration tests:

// +build integration

I think this is the reason why CI builds are failing.

Oh yes! My bad :(

jsoriano · 2018-07-16T15:54:09Z

metricbeat/module/mongodb/oplog/oplog.go

+// New creates a new instance of the MetricSet
+// Part of new is also setting up the configuration by processing additional
+// configuration entries if needed.
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {


Oh, add also here the experimental warning, something like:

cfgwarn.Experimental("The mongodb oplog metricset is experimental.")

a3dho3yn · 2018-07-16T18:24:32Z

@jsoriano Thanks for your comments. I've fixed these issues and pushed them to my branch.

jsoriano · 2018-07-17T16:08:44Z

@a3dho3yn unfortunately I cannot see the fixes, could you check that you pushed to the branch used for this PR?

a3dho3yn · 2018-08-13T15:41:37Z

@jsoriano We just finished our work but I have a problem with the integration tests.
When I run make test-module in my machine, everything goes well:

=== RUN   TestFetch
time="2018-08-13T17:31:23+04:30" level=info msg="[0/35] [mongodb]: Starting "
time="2018-08-13T17:31:23+04:30" level=warning msg="Error while reading .dockerignore (/home/ho3yn/go/src/github.com/elastic/beats/metricbeat/module/mongodb/_meta/.dockerignore) : open /home/ho3yn/go/src/github.com/elastic/beats/metricbeat/module/mongodb/_meta/.dockerignore: no such file or directory"
time="2018-08-13T17:31:23+04:30" level=info msg="Building metricbeat_mongodb..."
time="2018-08-13T17:31:23+04:30" level=info msg="Recreating mongodb"
time="2018-08-13T17:31:23+04:30" level=info msg="[1/35] [mongodb]: Started "
--- PASS: TestFetch (3.76s)
        replstatus_integration_test.go:49: mongodb/replstatus event: {"headroom":{"max":null,"min":null},"lag":{"max":null,"min":null},"members":{"arbiter":{"count":0,"hosts":null},"down":{"count":0,"hosts":null},"primary":{"host":"9c951bf3e380:27017","optime":1534165281},"recovering":{"count":0,"hosts":null},"rollback":{"count":0,"hosts":null},"secondary":{"count":0,"hosts":null,"optimes":null},"startup2":{"count":0,"hosts":null},"unhealthy":{"count":0,"hosts":null},"unknown":{"count":0,"hosts":null}},"oplog":{"first":{"timestamp":1534161289},"last":{"timestamp":1534165281},"size":{"allocated":1038090240,"used":38484},"window":3992},"optimes":{"applied":1534165281,"durable":1534165281,"last_committed":1534165281},"server_date":"2018-08-13T17:31:27.055+04:30","set_name":"beats"}
=== RUN   TestData
--- SKIP: TestData (1.13s)
        data_generator.go:44: skip data generation tests
PASS
ok      github.com/elastic/beats/metricbeat/module/mongodb/replstatus   (cached)

But tests are failing in the CI due to no reachable servers :(

jsoriano · 2018-08-14T15:22:45Z

I have been trying and the tests fail if they are run just after starting the container and it passes if the container was already started beforehand (or in a previous execution), so this is something that can be probably solved by improving the healthcheck, that currently only checks if the port is open.

On the other hand, I have also seen that tests only fail in the replstatus metricset, the only one setting the session mode to strong with mongoSession.SetMode(mgo.Strong, true). I wonder if it this is really needed. If this line is removed, tests pass too.

a3dho3yn · 2018-08-14T17:54:33Z

Optimes should be calculated from primray's view. That's why we used the strong mode. When there is one node and we initiate the replica set, this member should become primary and there should be no problem with forcing commands to run against primary. May be initiating takes time and we should wait for it. I will check this. Thanks for your insights:)

…

On Tue, Aug 14, 2018 at 7:53 PM Jaime Soriano Pastor < ***@***.***> wrote: I have been trying and the tests fails if the test is run just after start the container and it passes if the container was already started beforehand, so this is something that can be probably solved by improving the healthcheck, that currently only checks if the port is open. On the other hand, I have also seen that tests only fail in the replstatus metricset, the only one setting the session mode to strong with mongoSession.SetMode(mgo.Strong, true). I wonder if it this is really needed. If this line is removed, tests pass too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7604 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFk_xLFKaNQkjdHhLmwSLdOXr9RHBhakks5uQutagaJpZM4VQKYb> .

jsoriano · 2018-08-17T10:27:40Z

jenkins, test this please

jsoriano

It is looking good, I see it being merged soon 🙂
Only some small comments left.

jsoriano · 2018-08-17T10:42:40Z

metricbeat/module/mongodb/replstatus/_meta/data.json

@@ -0,0 +1,30 @@
+{


Could this file be updated?

jsoriano · 2018-08-17T10:46:40Z

metricbeat/module/mongodb/replstatus/replstatus.go

+func init() {
+	mb.Registry.MustAddMetricSet("mongodb", "replstatus", New,
+		mb.WithHostParser(mongodb.ParseURL),
+		mb.DefaultMetricSet())


If this requires replicaset to work maybe it'd be better to make it non-default.

jsoriano · 2018-08-17T10:48:57Z

metricbeat/module/mongodb/replstatus/replstatus_integration_test.go

+		myState, ok := status["myState"].(int)
+		t.Logf("Mongodb state is %d", myState)
+		if ok && myState == 1 {
+			time.Sleep(5 * time.Second) // hack, wait more for replica set to become stable


Is there any way to detect this stability? 🙂

We can leave it by now with the sleep in any case and revisit it later.

jsoriano · 2018-08-17T10:49:52Z

metricbeat/module/mongodb/replstatus/replstatus_integration_test.go

+		if ok && myState == 1 {
+			time.Sleep(5 * time.Second) // hack, wait more for replica set to become stable
+			break
+		}


Could we add a sleep after every retry?

Yes we can, but I see no reason to do this.
As you mentioned before, we should wait for some condition instead of sleeping. First I thought we should wait for a primary node, and I expected this condition to be sufficient for running the test. But then -in action- I figured out it needs something more than a node in the primary state. As I didn't find any condition to wait for, I used this sleep expresion.

If you're worried about blowing up CPU with this loop, I should note that state changes very fast (like 5 3 2 2 1) and it doesn't seem to be an issue.

Ok, let's leave it like this by now.

jsoriano · 2018-08-20T07:39:59Z

jenkins, test this

Add metrics about replication health. Co-authored-by: Hossein Taleghani <a3dho3yn@users.noreply.github.com> Co-authored-by: Bahar Taghavi <bahareh.t@fanap.plus> (cherry picked from commit ca8f56b)

a3dho3yn and others added 5 commits July 12, 2018 22:32

make oplog metricset

15cad2f

add fields and docs

0164707

add schema

ec8b2d2

add oplog metriset implementation

8f3697a

Update date model and add tests

1bd5b5a

houndci-bot reviewed Jul 15, 2018

View reviewed changes

Hossein Taleghani added 5 commits July 15, 2018 13:40

Fix naming

fd6d8d0

Add comments to public members

193d0a4

Add license headers

8681c45

Fix formats

e0f63c9

Update docs

b533b5d

ruflin added module review Metricbeat Metricbeat labels Jul 16, 2018

ruflin requested a review from jsoriano July 16, 2018 07:34

Hossein Taleghani and others added 2 commits July 16, 2018 12:16

Add changelog entry

8ee5ffd

Merge branch 'master' into dev/add-oplog-metricset

b6d80e4

jsoriano previously requested changes Jul 16, 2018

View reviewed changes

jsoriano reviewed Jul 16, 2018

View reviewed changes

a3dho3yn added 6 commits July 16, 2018 22:11

Use timestamp instead of ts

9725625

Add experimental warning

3718683

Use safe conversion

c9e9d72

Set integration flag on test

02fd30a

make update

d54098e

fix test

60d25f9

a3dho3yn and others added 12 commits August 8, 2018 19:58

fix oplog integration tests

d5cc468

Add "hasSecondary" flag and merge min & max lag

2daf4f6

add optimes to primary and secondary

51f6c5c

change optime.Ts to optime.getTimeStamp

61ac103

update fields.yml

99e57d8

Update change log entry

464fbad

make update and fmt

0863868

mongodb.yml.disabled was left behind

d9fbd73

Fix connect mode

c57cf59

Replace map[string]interface{} with common.MapStr

874c674

Check primary host and repl set name in integration test

565209a

make fmt

271a53f

a3dho3yn added 2 commits August 17, 2018 14:05

wait for PRIMARY state

3b26b84

make fmt

db91745

jsoriano requested changes Aug 17, 2018

View reviewed changes

a3dho3yn added 2 commits August 18, 2018 01:16

Remove DefaultMetricSet option

9cc20a0

Update data.json

aca4962

jsoriano approved these changes Aug 20, 2018

View reviewed changes

jsoriano merged commit ca8f56b into elastic:master Aug 20, 2018

jsoriano mentioned this pull request Aug 20, 2018

Cherry-pick #7604 to 6.x: Add oplog metricset to mongodb module #8019

Merged

jsoriano added the v6.5.0 label Aug 20, 2018


		--

		`mongodb.oplog.last.ts`::

Conversation

a3dho3yn commented Jul 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jul 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvch commented Jul 16, 2018

Uh oh!

ruflin commented Jul 16, 2018

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a3dho3yn commented Jul 16, 2018

Uh oh!

jsoriano commented Jul 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a3dho3yn commented Aug 13, 2018

Uh oh!

jsoriano commented Aug 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a3dho3yn commented Aug 14, 2018 via email

Uh oh!

jsoriano commented Aug 17, 2018

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano commented Aug 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

a3dho3yn commented Jul 15, 2018 •

edited

Loading

jsoriano commented Jul 17, 2018 •

edited

Loading

jsoriano commented Aug 14, 2018 •

edited

Loading