From fc7b3ce7dd837008ee6e765e26c29a5fc8d46020 Mon Sep 17 00:00:00 2001 From: Guillermo Sanchez Gavier Date: Fri, 6 Oct 2023 22:12:22 +0200 Subject: [PATCH] [mongodbreceiver] Fix get mongodb version (#27442) **Description:** - Improve the unit-test to catch similar errors in the future. - Fix mongodb version collection. **Link to tracking Issue:** #27441 **Testing:** Added the expectation assert for the Mock. This is the log output from running the tests in the first catching the bug ``` --- FAIL: TestScraperScrape (0.00s) --- FAIL: TestScraperScrape/Failed_to_fetch_database_names (0.00s) pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:316: FAIL: GetVersion(string) at: [pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:146 pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:292] pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:316: PASS: ListDatabaseNames(string,string,string) pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:316: FAIL: 1 out of 2 expectation(s) were met. The code you are testing needs to make 1 more call(s). at: [pathTo/opentelemetry-collector-contrib/receiver/mongodbreceiver/scraper_test.go:316] ``` **Documentation:** --- .../receiver-mongodb-fix-get-version.yaml | 27 +++++++++++++++++++ receiver/mongodbreceiver/scraper.go | 7 ++--- receiver/mongodbreceiver/scraper_test.go | 25 +++++++++++------ 3 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 .chloggen/receiver-mongodb-fix-get-version.yaml diff --git a/.chloggen/receiver-mongodb-fix-get-version.yaml b/.chloggen/receiver-mongodb-fix-get-version.yaml new file mode 100644 index 000000000000..ad6ff48375df --- /dev/null +++ b/.chloggen/receiver-mongodb-fix-get-version.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: mongodbreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Fix mongo version not being collected" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [27441] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/receiver/mongodbreceiver/scraper.go b/receiver/mongodbreceiver/scraper.go index 7557e66136a5..53de8667d0a7 100644 --- a/receiver/mongodbreceiver/scraper.go +++ b/receiver/mongodbreceiver/scraper.go @@ -21,6 +21,8 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/mongodbreceiver/internal/metadata" ) +var unknownVersion = func() *version.Version { return version.Must(version.NewVersion("0.0")) } + type mongodbScraper struct { logger *zap.Logger config *Config @@ -30,12 +32,11 @@ type mongodbScraper struct { } func newMongodbScraper(settings receiver.CreateSettings, config *Config) *mongodbScraper { - v, _ := version.NewVersion("0.0") return &mongodbScraper{ logger: settings.Logger, config: config, mb: metadata.NewMetricsBuilder(config.MetricsBuilderConfig, settings), - mongoVersion: v, + mongoVersion: unknownVersion(), } } @@ -60,7 +61,7 @@ func (s *mongodbScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { return pmetric.NewMetrics(), errors.New("no client was initialized before calling scrape") } - if s.mongoVersion == nil { + if s.mongoVersion.Equal(unknownVersion()) { version, err := s.client.GetVersion(ctx) if err == nil { s.mongoVersion = version diff --git a/receiver/mongodbreceiver/scraper_test.go b/receiver/mongodbreceiver/scraper_test.go index 50f5516240cf..11a70804bbf9 100644 --- a/receiver/mongodbreceiver/scraper_test.go +++ b/receiver/mongodbreceiver/scraper_test.go @@ -121,14 +121,14 @@ func TestScraperScrape(t *testing.T) { testCases := []struct { desc string partialErr bool - setupMockClient func(t *testing.T) client + setupMockClient func(t *testing.T) *fakeClient expectedMetricGen func(t *testing.T) pmetric.Metrics expectedErr error }{ { desc: "Nil client", partialErr: false, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { return nil }, expectedMetricGen: func(t *testing.T) pmetric.Metrics { @@ -139,7 +139,7 @@ func TestScraperScrape(t *testing.T) { { desc: "Failed to fetch database names", partialErr: true, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { fc := &fakeClient{} mongo40, err := version.NewVersion("4.0") require.NoError(t, err) @@ -155,7 +155,7 @@ func TestScraperScrape(t *testing.T) { { desc: "Failed to fetch collection names", partialErr: true, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { fc := &fakeClient{} mongo40, err := version.NewVersion("4.0") require.NoError(t, err) @@ -181,7 +181,7 @@ func TestScraperScrape(t *testing.T) { { desc: "Failed to scrape client stats", partialErr: true, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { fc := &fakeClient{} mongo40, err := version.NewVersion("4.0") require.NoError(t, err) @@ -209,7 +209,7 @@ func TestScraperScrape(t *testing.T) { { desc: "Failed to scrape with partial errors on metrics", partialErr: true, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { fc := &fakeClient{} mongo40, err := version.NewVersion("4.0") require.NoError(t, err) @@ -240,7 +240,7 @@ func TestScraperScrape(t *testing.T) { { desc: "Successful scrape", partialErr: false, - setupMockClient: func(t *testing.T) client { + setupMockClient: func(t *testing.T) *fakeClient { fc := &fakeClient{} adminStatus, err := loadAdminStatusAsMap() require.NoError(t, err) @@ -288,7 +288,12 @@ func TestScraperScrape(t *testing.T) { scraperCfg.MetricsBuilderConfig.Metrics.MongodbHealth.Enabled = true scraper := newMongodbScraper(receivertest.NewNopCreateSettings(), scraperCfg) - scraper.client = tc.setupMockClient(t) + + mc := tc.setupMockClient(t) + if mc != nil { + scraper.client = mc + } + actualMetrics, err := scraper.scrape(context.Background()) if tc.expectedErr == nil { require.NoError(t, err) @@ -307,6 +312,10 @@ func TestScraperScrape(t *testing.T) { } } + if mc != nil { + mc.AssertExpectations(t) + } + if tc.partialErr { require.True(t, scrapererror.IsPartialScrapeError(err)) } else {