PMM-11406 Better check to prevent errors from missing fields.

[JiriCtvrtka]

• The contributed code is licensed under GPL v2.0
• Contributor Licence Agreement (CLA) is signed
• util/update-modules has been ran
• Documentation updated
• Test suite update

[svetasmirnova]

Please add a test case.

[svetasmirnova]

I added a fix for the logrus.Infof error that broke tests for main. Now they are passed. I hope this is OK.

[JiriCtvrtka]

@svetasmirnova Currently, we are not checking the output from the getHostInfo method at all. I’ve made a small change in the logic to make it more stable and robust for setup where getCmdLineOpts.recordStats or serverStatus.recordStats are missing. Right now, this situation ends with an error and produces no output at all. With this change, the missing fields will simply be skipped, and the rest of the output will still be printed.

In the PMM team, our QA setup uses a sharded cluster with replica sets, but those two fields are always missing regardless of version or distribution.

I can create a new test like this:

func TestGetHostInfo(t *testing.T) {
    assert := require.New(t)

    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
    defer cancel()

    client, err := tu.TestClient(ctx, tu.MongoDBShard1PrimaryPort)
    assert.NoError(err, "cannot get a new MongoDB client")

    host, err := getHostInfo(ctx, client)
    assert.NoError(err, "getHostInfo error")

    assert.NotEmpty(host.ProcessName, "ProcessName should not be empty if serverStatus succeeds")
    assert.NotEmpty(host.Version, "Version should not be empty if serverStatus succeeds")
    assert.NotEmpty(host.ProcPath, "ProcPath should not be empty if getProcInfo succeeds")
    assert.NotEmpty(host.ProcUserName, "ProcUserName should not be empty if getProcInfo succeeds")
    assert.False(host.ProcCreateTime.IsZero(), "ProcCreateTime should not be zero if getProcInfo succeeds")
}

This would cover those cases, but since the Percona Toolkit test environment always has these fields, it won’t be able to test the missing-field scenario anyway.

Thanks for fix of print. Also in scope of PR I removed unused method in main_test file.

[svetasmirnova]

@JiriCtvrtka Yes, please add this test still. I have plans for improving the Percona Toolkit test environment btw. Not sure when find time for it.

[JiriCtvrtka]

Test added.

[svetasmirnova]

@JiriCtvrtka

test fails with:

$ go test -v
=== RUN   TestGetHostInfo
=== RUN   TestGetHostInfo/from_mongos
=== RUN   TestGetHostInfo/from_mongod
--- PASS: TestGetHostInfo (0.14s)
    --- PASS: TestGetHostInfo/from_mongos (0.08s)
    --- PASS: TestGetHostInfo/from_mongod (0.06s)
=== RUN   TestGetHostInfoResult
    main_test.go:67: 
        	Error Trace:	/home/sveta/src/percona/percona-toolkit/src/go/pt-mongodb-summary/main_test.go:67
        	Error:      	Should NOT be empty, but was 
        	Test:       	TestGetHostInfoResult
        	Messages:   	ProcPath should not be empty if getProcInfo succeeds
--- FAIL: TestGetHostInfoResult (0.06s)
=== RUN   TestClusterWideInfo
=== RUN   TestClusterWideInfo/from_mongos
=== RUN   TestClusterWideInfo/from_mongod
    main_test.go:102: getClisterWideInfo error: getClusterwideInfo.listDatabases : (NotPrimaryOrSecondary) node is not in primary or recovering state
--- FAIL: TestClusterWideInfo (0.03s)
    --- PASS: TestClusterWideInfo/from_mongos (0.03s)
    --- FAIL: TestClusterWideInfo/from_mongod (0.00s)
=== RUN   TestParseArgs
--- PASS: TestParseArgs (0.00s)
FAIL
exit status 1
FAIL	github.com/percona/percona-toolkit/src/go/pt-mongodb-summary	0.236s

Is this expected?

[JiriCtvrtka]

I think it does not work due to typo. Now it should be fixed.

[JiriCtvrtka]

@svetasmirnova
TestGetHostInfoResult – I rewrote the test to be as universal as possible and not dependent on configuration. If we have some data in the response, that’s OK. This still covers the original purpose of the change.

Regarding failing TestClusterWideInfo/from_mongod – that should not be related to my changes in this PR.

As for satisfying spelling/typos – I fixed them across the whole repository. One possible bug here because of this: https://github.com/percona/percona-toolkit/pull/1013/files#r2398484718

[JiriCtvrtka]

Not used