Upgrade Mongodb library in Beats to v5 #31185

sayden · 2022-04-06T13:45:07Z

What does this PR do?

Replaces Mongodb library that Beats was using for the updated (and maintained) official version.

Why is it important?

Current version library was unmaintained and Beats could only support up to v3.2. This change adds support for V4 and V5 too.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2022-04-06T14:18:19Z

💚 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-05-17T14:05:02.391+0000
Duration: 177 min 58 sec

Test stats 🧪

Test	Results
Failed	0
Passed	21737
Skipped	1802
Total	23539

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

mergify · 2022-04-28T12:56:27Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature/mb/upgrade-mongodb-library upstream/feature/mb/upgrade-mongodb-library
git merge upstream/main
git push upstream feature/mb/upgrade-mongodb-library

…mb/upgrade-mongodb-library # Conflicts: # go.mod # go.sum # metricbeat/module/mongodb/collstats/collstats.go # metricbeat/module/mongodb/dbstats/dbstats.go # metricbeat/module/mongodb/replstatus/data.go # metricbeat/module/mongodb/replstatus/replstatus_integration_test.go # metricbeat/module/mongodb/status/status.go

sayden · 2022-05-08T10:47:11Z

/test

efd6 · 2022-05-08T23:37:09Z

metricbeat/module/mongodb/collstats/collstats.go

 // As a minimum it must inherit the mb.BaseMetricSet fields, but can be extended with
 // additional entries. These variables can be used to persist data or configuration between
 // multiple fetch calls.
-type MetricSet struct {
-	*mongodb.MetricSet
+type Metricset struct {


Is there a reason for this name change? There appears to be disagreement in the code base about how this should be spelled; the docs spell it as a single word, but the mb package spells it as a camel-cased pair.

Well, I'm afraid that it's out of the scope of the PR to reach a consensus 🙂 Personally I don't like the camelCased version and I think statistically you'll see it more often without camelCase like in the docs of most of the metricsets of most of the modules. https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-metricset-mongodb-collstats.html

efd6 · 2022-05-08T23:47:07Z

metricbeat/module/mongodb/collstats/collstats.go

+	defer func() {
+		_ = client.Disconnect(context.Background())
+	}()


This may be an unpopular opinion, but I think assigning to the blank identifier to satisfy the linter is an antipattern; it does not handle the error but also evades the rationale behind enforcing require-explanation: true and makes it harder in the future to clean up linter supression because it is harder to find and reason about.

+1, I'd rather see an explanation of why errors can be ignored where possible using the //nolint:errcheck $reason comment instead.

On top of my head, most of Close and Disconnect operations across metricbeat modules ignores the error. As the person who has tried to maintain this module on the last years, I just wanted to do the dirty work of upgrading the library and leave improvements to the @elastic/obs-service-integrations team now

Keep in mind that it takes me more time to write this message on GH than to write a line that logs the error.

sayden · 2022-05-09T17:13:01Z

Thanks for the messages, it will be nice if @efd6 can take a quick look at the Packetbeat file that was modified in this PR, that's why security was pinged.

Thanks!

efd6

Packetbeat changes are fine.

mergify · 2022-05-10T15:01:25Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature/mb/upgrade-mongodb-library upstream/feature/mb/upgrade-mongodb-library
git merge upstream/main
git push upstream feature/mb/upgrade-mongodb-library

…mb/upgrade-mongodb-library # Conflicts: # metricbeat/module/mongodb/dbstats/dbstats.go # metricbeat/module/mongodb/metricset.go # metricbeat/module/mongodb/mongodb.go # packetbeat/protos/mongodb/mongodb_parser.go

ManojS-shetty

Tested for the compatibility for MongoDB version 5.0 and >5.0 and beat managed establish the connection and pull the metrics.

As discussed please close the related issue for mongo DB. Rest LGTM.

ManojS-shetty · 2022-05-10T04:27:52Z

metricbeat/module/mongodb/dbstats/data.go

@@ -24,40 +24,40 @@ import (

 var schema = s.Schema{
 	"db":          c.Str("db"),
-	"collections": c.Int("collections"),
-	"objects":     c.Int("objects"),
+	"collections": c.Ifc("collections"),


@sayden Is this also part of this driver update change? Can you please let us know in brief for the change from Int to Ifc() .

mergify · 2022-05-12T11:00:07Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature/mb/upgrade-mongodb-library upstream/feature/mb/upgrade-mongodb-library
git merge upstream/main
git push upstream feature/mb/upgrade-mongodb-library

mergify · 2022-05-16T14:51:51Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature/mb/upgrade-mongodb-library upstream/feature/mb/upgrade-mongodb-library
git merge upstream/main
git push upstream feature/mb/upgrade-mongodb-library

…mb/upgrade-mongodb-library # Conflicts: # go.sum # metricbeat/module/mongodb/metricset.go

cmacknz · 2022-05-26T17:51:28Z

metricbeat/module/mongodb/replstatus/replstatus_integration_test.go

 	"github.com/elastic/elastic-agent-libs/mapstr"
 )

 func TestFetch(t *testing.T) {
-	t.Skip("Flaky test: https://github.com/elastic/beats/issues/29208")


This test is flaky again: #31768

Collstats metricset plus module

b2f9f7b

sayden assigned sayden and ManojS-shetty Apr 6, 2022

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 6, 2022

sayden added the Team:Service-Integrations Label for the Service Integrations team label Apr 13, 2022

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 13, 2022

sayden added 15 commits May 7, 2022 14:55

fix collstats metricset

be29d0a

Abstract connection calls at the module level

244e399

fix dbstats metricset

23bdadb

fix metrics metricset

363137e

fix status metricset

10bea25

fix status metricset

a83a544

mage fmt update

1a1ad28

Merge with master and fix conflicts

b1109bd

Fix linting

798911a

fix linting in packetbeat

7d7011d

tidy up modules

fe6ff3d

Update NOTICE.txt

6f9820a

Fix test

ac2f7da

Fix remaining tests

503b68f

elasticmachine mentioned this pull request May 8, 2022

Flaky Test [Build&Test / filebeat-goIntegTest / TestParsersMultilineTimeout – github.com/elastic/beats/v7/filebeat/input/filestream] #31445

Closed

sayden added bug enhancement labels May 8, 2022

sayden marked this pull request as ready for review May 8, 2022 17:06

sayden requested review from a team as code owners May 8, 2022 17:06

sayden requested a review from a team as a code owner May 8, 2022 17:06

sayden requested review from cmacknz and leehinman and removed request for a team May 8, 2022 17:06

efd6 reviewed May 8, 2022

View reviewed changes

sayden unassigned ManojS-shetty May 9, 2022

sayden requested review from ManojS-shetty and lalit-satapathy May 9, 2022 08:03

efd6 reviewed May 9, 2022

View reviewed changes

sayden added 4 commits May 11, 2022 17:08

Add a message to potential disconnection errors

56f2cbf

Replace the logger library on the protos of Packetbeat for mongo

3ce25c1

Add changelog entry

8059f71

ManojS-shetty approved these changes May 12, 2022

View reviewed changes

Merge branch 'main' into feature/mb/upgrade-mongodb-library

339dfdc

sayden mentioned this pull request May 13, 2022

Metricbeats cannot connect to mongodb 5.x #29511

Open

sayden added 3 commits May 17, 2022 11:18

Merge commit '5129d6ce91e37cc0df6c8c51ace7ccabb7db97db' into feature/…

a72224a

…mb/upgrade-mongodb-library # Conflicts: # go.sum # metricbeat/module/mongodb/metricset.go

Fix conflicts

62f4353

Fix notice

ff0a1c0

sayden merged commit 49ae09a into elastic:main May 17, 2022

cmacknz mentioned this pull request May 26, 2022

Flaky Test: github.com/elastic/beats/v7/metricbeat/module/mongodb/replstatus #31768

Open

cmacknz reviewed May 26, 2022

View reviewed changes

andresrc mentioned this pull request Jun 22, 2022

Remove invalid entry #32046

Merged

chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023

Upgrade Mongodb library in Beats to accept v5 metrics (#31185)

33fffe7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Mongodb library in Beats to v5 #31185

Upgrade Mongodb library in Beats to v5 #31185

sayden commented Apr 6, 2022 •

edited

Loading

elasticmachine commented Apr 6, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

mergify bot commented Apr 28, 2022

sayden commented May 8, 2022

efd6 May 8, 2022

sayden May 9, 2022 •

edited

Loading

efd6 May 8, 2022

cmacknz May 9, 2022

sayden May 9, 2022

sayden commented May 9, 2022

efd6 left a comment

mergify bot commented May 10, 2022

ManojS-shetty left a comment

ManojS-shetty May 10, 2022

mergify bot commented May 12, 2022

mergify bot commented May 16, 2022

cmacknz May 26, 2022

Upgrade Mongodb library in Beats to v5 #31185

Upgrade Mongodb library in Beats to v5 #31185

Conversation

sayden commented Apr 6, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Apr 6, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

mergify bot commented Apr 28, 2022

sayden commented May 8, 2022

efd6 May 8, 2022

Choose a reason for hiding this comment

sayden May 9, 2022 • edited Loading

Choose a reason for hiding this comment

efd6 May 8, 2022

Choose a reason for hiding this comment

cmacknz May 9, 2022

Choose a reason for hiding this comment

sayden May 9, 2022

Choose a reason for hiding this comment

sayden commented May 9, 2022

efd6 left a comment

Choose a reason for hiding this comment

mergify bot commented May 10, 2022

ManojS-shetty left a comment

Choose a reason for hiding this comment

ManojS-shetty May 10, 2022

Choose a reason for hiding this comment

mergify bot commented May 12, 2022

mergify bot commented May 16, 2022

cmacknz May 26, 2022

Choose a reason for hiding this comment

sayden commented Apr 6, 2022 •

edited

Loading

elasticmachine commented Apr 6, 2022 •

edited by jenkins-beats-ci bot

Loading

sayden May 9, 2022 •

edited

Loading