PMM-12375 add database version to services (#2488)

* PMM-12375 create serviceInfoBroker component * PMM-12375 serviceInfoBroker won't ping the db * PMM-12375 fix the test * PMM-12375 add tests for serviceInfoBroker * PMM-12375 fix the agent tests * PMM-12375 fix agent tests, server-side * PMM-12375 fix services tests * PMM-12375 do not update if version is empty * PMM-12375 remove the unused parameter * PMM-12375 remove the version from Stats * PMM-12375 add message type processing to channel * PMM-12375 update codeowners * PMM-12375 respect skipConnectionCheck, simpler schema * PMM-12375 fix linter warnings * PMM-12375 fix license headers * PMM-12375 respect skipConnectionCheck param * PMM-12375 minor fixes * PMM-12375 update the docs * PMM-12375 do not run serviceInfo for older clients * PMM-12375 add serviceInfoBroker to rds * PMM-12375 add serviceInfoBroker to azure * PMM-12375 add version to ListServices result * PMM-12375 a minor fix in teardown fn * PMM-12375 cleanup the inevntory_metrics_test * PMM-12375 follow up on review * PMM-12375 revert changes, move to other PRs * PMM-12375 revert changed related to linter * PMM-12375 make version nullable * PMM-12375 do not call FindService when skipping connection check * PMM-12375 fix pointer reference * PMM-12375 re-generate interfaces * PMM-12375 fix a dereference * PMM-12375 make ServiceInfoBroker work for older agents * PMM-12375 fix the linter error * PMM-12375 follow up on review * PMM-12375 a few corrections to managed's readme.md * PMM-12375 fix the mock file version * PMM-12375 bump up the compatibility version * PMM-12375 regen the mockfiles * PMM-12375 remove debug msg * Fix linter --------- Co-authored-by: Alex Tymchuk <[email protected]> Co-authored-by: Alex Demidoff <[email protected]> Co-authored-by: Nurlan Moldomurov <[email protected]>
percona · Oct 24, 2023 · 5730e16 · 5730e16
1 parent be137c9
commit 5730e16
Show file tree

Hide file tree

Showing 67 changed files with 3,663 additions and 1,546 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -4,9 +4,9 @@ We'd be glad to welcome you to Percona community which tries to keep the open so
 
 ## Table of contents
 1. [Project repos structure](#Project-repos-structure)
-2. [API documentation](#API-documentation)
+2. [API documentation](#API-Reference-Documentation)
 3. [Prerequisites](#Prerequisites)
-4. [Submitting a Bug](#Submitting-a-Bug)
+4. [Submitting a bug](#Submitting-a-Bug)
 5. [Setup your local development environment](#Setup-your-local-development-environment)
 6. [Tests](#Tests)
 7. [Feature Build](#Feature-Build)
@@ -202,19 +202,22 @@ There are number of approaches for the code review and ownership: Code Ownership
 
 For more efficient review process we use a mixed approach:
 * repos that have CODEOWNERS
-  * add **auto-review-team** additionally to CODEOWNERS assigned
+  * github will assign reviewers automatically
 * repos that don't have CODEOWNERS
-  * add **auto-review-team**
+  * add reviewers as follows:
+      * add `pmm-review-fe` for UI/UX reviews
+      * add `pmm-review-exporters` for exporter reviews [see PMM Client](#PMM-Client)
+      * add `pmm-review-be` for backend reviews
 * if you know exactly who should review your code
-  * add ppl to the review
+  * add them to the review
 
 
-| Team                 | Description                                                             | Members |
-| -------------------- | ----------------------------------------------------------------------- | ------- |
-| pmm-review-fe        | ppl for UI/UX reviews for [FrontEnd repos](#FrontEnd)                   | [FE team](https://github.com/orgs/percona/teams/pmm-review-fe/members)        |
-| pmm-review-exporters | reviewers for all exporters [see PMM Client](#PMM-Client)               | [Exporters team](https://github.com/orgs/percona/teams/pmm-review-exporters/members) |
-| pmm-review-be        | Back-End engineers                                                      | [BE team](https://github.com/orgs/percona/teams/pmm-review-be/members)        |
-| PMM Admins           | ppl that could use admins rights to force merge or change repo settings | [PMM Admin team](https://github.com/orgs/percona/teams/pmm-admins/members)           |
+| Team                 | Description                                                    | Members |
+| -------------------- | -------------------------------------------------------------- | ------- |
+| pmm-review-fe        | UI reviewers of PRs to [FrontEnd repos](#FrontEnd)             | [FE team](https://github.com/orgs/percona/teams/pmm-review-fe/members)        |
+| pmm-review-exporters | exporter reviewers of PRs to [PMM Client](#PMM-Client)         | [Exporters team](https://github.com/orgs/percona/teams/pmm-review-exporters/members) |
+| pmm-review-be        | reviewers of backend (Go) PRs                                  | [BE team](https://github.com/orgs/percona/teams/pmm-review-be/members)        |
+| PMM Admins           | reviewers that could use admins rights to force merge or change repo settings | [PMM Admin team](https://github.com/orgs/percona/teams/pmm-admins/members)           |
 
 
 ## After your Pull Request is merged

diff --git a/admin/commands/management/add.go b/admin/commands/management/add.go
@@ -21,7 +21,7 @@ import (
 
 // AddCommand is used by Kong for CLI flags and commands.
 type AddCommand struct {
-	External           AddExternalCommand           `cmd:"" help:"Add External source of data (like a custom exporter running on a port) to the monitoring"`
+	External           AddExternalCommand           `cmd:"" help:"Add External source of data (like a custom exporter running on a port) to monitoring"`
 	ExternalServerless AddExternalServerlessCommand `cmd:"" help:"Add External Service on Remote node to monitoring"`
 	HAProxy            AddHAProxyCommand            `cmd:"" name:"haproxy" help:"Add HAProxy to monitoring"`
 	MongoDB            AddMongoDBCommand            `cmd:"" name:"mongodb" help:"Add MongoDB to monitoring"`

diff --git a/admin/commands/management/add_mysql.go b/admin/commands/management/add_mysql.go
@@ -146,7 +146,7 @@ func (cmd *AddMySQLCommand) RunCmd() (commands.Result, error) {
 
 	if cmd.CreateUser {
 		return nil, errors.New("Unrecognized option. To create a user, see " +
-			"'https://www.percona.com/doc/percona-monitoring-and-management/2.x/concepts/services-mysql.html#pmm-conf-mysql-user-account-creating'")
+			"'https://docs.percona.com/percona-monitoring-and-management/setting-up/client/mysql.html#create-a-database-account-for-pmm'")
 	}
 
 	var (

diff --git a/admin/commands/management/add_mysql_test.go b/admin/commands/management/add_mysql_test.go
@@ -155,7 +155,7 @@ func TestRun(t *testing.T) {
 		_, err := cmd.RunCmd()
 
 		if assert.Error(t, err) {
-			expected := "Unrecognized option. To create a user, see 'https://www.percona.com/doc/percona-monitoring-and-management/2.x/concepts/services-mysql.html#pmm-conf-mysql-user-account-creating'"
+			expected := "Unrecognized option. To create a user, see 'https://docs.percona.com/percona-monitoring-and-management/setting-up/client/mysql.html#create-a-database-account-for-pmm'"
 			assert.Equal(t, expected, err.Error())
 		}
 	})

diff --git a/agent/client/channel/channel.go b/agent/client/channel/channel.go
@@ -290,6 +290,11 @@ func (c *Channel) runReceiver() {
 				ID:      msg.Id,
 				Payload: p.AgentLogs,
 			}
+		case *agentpb.ServerMessage_ServiceInfo:
+			c.requests <- &ServerRequest{
+				ID:      msg.Id,
+				Payload: p.ServiceInfo,
+			}
 
 		// responses
 		case *agentpb.ServerMessage_Pong:
@@ -311,7 +316,7 @@ func (c *Channel) runReceiver() {
 			}
 			c.Send(&AgentResponse{
 				ID:     msg.Id,
-				Status: grpcstatus.New(codes.Unimplemented, "can't handle message type send, it is not implemented"),
+				Status: grpcstatus.New(codes.Unimplemented, "can't handle message type sent, it is not implemented"),
 			})
 		}
 	}

diff --git a/agent/client/client.go b/agent/client/client.go
@@ -68,6 +68,7 @@ type Client struct {
 	supervisor        supervisor
 	connectionChecker connectionChecker
 	softwareVersioner softwareVersioner
+	serviceInfoBroker serviceInfoBroker
 
 	l       *logrus.Entry
 	backoff *backoff.Backoff
@@ -89,12 +90,13 @@ type Client struct {
 // New creates new client.
 //
 // Caller should call Run.
-func New(cfg configGetter, supervisor supervisor, r *runner.Runner, connectionChecker connectionChecker, sv softwareVersioner, cus *connectionuptime.Service, logStore *tailog.Store) *Client { //nolint:lll
+func New(cfg configGetter, supervisor supervisor, r *runner.Runner, connectionChecker connectionChecker, sv softwareVersioner, sib serviceInfoBroker, cus *connectionuptime.Service, logStore *tailog.Store) *Client { //nolint:lll
 	return &Client{
 		cfg:               cfg,
 		supervisor:        supervisor,
 		connectionChecker: connectionChecker,
 		softwareVersioner: sv,
+		serviceInfoBroker: sib,
 		l:                 logrus.WithField("component", "client"),
 		backoff:           backoff.New(backoffMinDelay, backoffMaxDelay),
 		dialTimeout:       dialTimeout,
@@ -388,6 +390,9 @@ loop:
 			case *agentpb.CheckConnectionRequest:
 				responsePayload = c.connectionChecker.Check(ctx, p, req.ID)
 
+			case *agentpb.ServiceInfoRequest:
+				responsePayload = c.serviceInfoBroker.GetInfoFromService(ctx, p, req.ID)
+
 			case *agentpb.StartJobRequest:
 				var resp agentpb.StartJobResponse
 				if err := c.handleStartJobRequest(p); err != nil {

diff --git a/agent/client/client_test.go b/agent/client/client_test.go
@@ -83,7 +83,7 @@ func TestClient(t *testing.T) {
 		ctx, cancel := context.WithCancel(context.Background())
 
 		cfgStorage := config.NewStorage(&config.Config{})
-		client := New(cfgStorage, nil, nil, nil, nil, nil, nil)
+		client := New(cfgStorage, nil, nil, nil, nil, nil, nil, nil)
 		cancel()
 		err := client.Run(ctx)
 		assert.EqualError(t, err, "missing PMM Server address: context canceled")
@@ -98,7 +98,7 @@ func TestClient(t *testing.T) {
 				Address: "127.0.0.1:1",
 			},
 		})
-		client := New(cfgStorage, nil, nil, nil, nil, nil, nil)
+		client := New(cfgStorage, nil, nil, nil, nil, nil, nil, nil)
 		cancel()
 		err := client.Run(ctx)
 		assert.EqualError(t, err, "missing Agent ID: context canceled")
@@ -115,7 +115,7 @@ func TestClient(t *testing.T) {
 				Address: "127.0.0.1:1",
 			},
 		})
-		client := New(cfgStorage, nil, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
+		client := New(cfgStorage, nil, nil, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
 		err := client.Run(ctx)
 		assert.EqualError(t, err, "failed to dial: context deadline exceeded")
 	})
@@ -164,7 +164,7 @@ func TestClient(t *testing.T) {
 			s.On("ClearChangesChannel").Return()
 
 			r := runner.New(cfgStorage.Get().RunnerCapacity)
-			client := New(cfgStorage, &s, r, nil, nil, connectionuptime.NewService(time.Hour), nil)
+			client := New(cfgStorage, &s, r, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
 			err := client.Run(context.Background())
 			assert.NoError(t, err)
 			assert.Equal(t, serverMD, client.GetServerConnectMetadata())
@@ -192,7 +192,7 @@ func TestClient(t *testing.T) {
 				},
 			})
 
-			client := New(cfgStorage, nil, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
+			client := New(cfgStorage, nil, nil, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
 			client.dialTimeout = 100 * time.Millisecond
 			err := client.Run(ctx)
 			assert.EqualError(t, err, "failed to get server metadata: rpc error: code = Canceled desc = context canceled", "%+v", err)
@@ -282,7 +282,7 @@ func TestUnexpectedActionType(t *testing.T) {
 	s.On("ClearChangesChannel").Return()
 
 	r := runner.New(cfgStorage.Get().RunnerCapacity)
-	client := New(cfgStorage, s, r, nil, nil, connectionuptime.NewService(time.Hour), nil)
+	client := New(cfgStorage, s, r, nil, nil, nil, connectionuptime.NewService(time.Hour), nil)
 	err := client.Run(context.Background())
 	assert.NoError(t, err)
 	assert.Equal(t, serverMD, client.GetServerConnectMetadata())

diff --git a/agent/client/deps.go b/agent/client/deps.go
@@ -24,6 +24,7 @@ import (
 )
 
 //go:generate ../../bin/mockery --name=connectionChecker --case=snake --inpackage --testonly
+//go:generate ../../bin/mockery --name=serviceInfoBroker --case=snake --inpackage --testonly
 //go:generate ../../bin/mockery --name=supervisor --case=snake --inpackage --testonly
 
 // connectionChecker is a subset of methods of connectionchecker.ConnectionChecker used by this package.
@@ -32,6 +33,11 @@ type connectionChecker interface {
 	Check(ctx context.Context, req *agentpb.CheckConnectionRequest, id uint32) *agentpb.CheckConnectionResponse
 }
 
+// serviceInfoBroker is a subset of methods of serviceinfobroker.ServiceInfoBroker used by this package.
+type serviceInfoBroker interface {
+	GetInfoFromService(ctx context.Context, req *agentpb.ServiceInfoRequest, id uint32) *agentpb.ServiceInfoResponse
+}
+
 // softwareVersioner is a subset of methods of version.Versioner used by this package.
 type softwareVersioner interface {
 	MySQLdVersion() (string, error)

diff --git a/agent/client/mock_service_info_broker_test.go b/agent/client/mock_service_info_broker_test.go
diff --git a/agent/commands/run.go b/agent/commands/run.go
@@ -33,6 +33,7 @@ import (
 	"github.com/percona/pmm/agent/connectionchecker"
 	"github.com/percona/pmm/agent/connectionuptime"
 	"github.com/percona/pmm/agent/runner"
+	"github.com/percona/pmm/agent/serviceinfobroker"
 	"github.com/percona/pmm/agent/tailog"
 	"github.com/percona/pmm/agent/versioner"
 	"github.com/percona/pmm/api/inventorypb"
@@ -69,8 +70,9 @@ func Run() {
 
 		supervisor := supervisor.NewSupervisor(ctx, v, configStorage)
 		connectionChecker := connectionchecker.New(configStorage)
+		serviceInfoBroker := serviceinfobroker.New(configStorage)
 		r := runner.New(cfg.RunnerCapacity)
-		client := client.New(configStorage, supervisor, r, connectionChecker, v, prepareConnectionService(ctx, cfg), logStore)
+		client := client.New(configStorage, supervisor, r, connectionChecker, v, serviceInfoBroker, prepareConnectionService(ctx, cfg), logStore)
 		localServer := agentlocal.NewServer(configStorage, supervisor, client, configFilepath, logStore)
 
 		logrus.Infof("Window check connection time is %.2f hour(s)", cfg.WindowConnectedTime.Hours())

diff --git a/agent/connectionchecker/connection_checker.go b/agent/connectionchecker/connection_checker.go
@@ -21,7 +21,6 @@ import (
 	"database/sql"
 	"fmt"
 	"io"
-	"math"
 	"net/http"
 	"path/filepath"
 	"strconv"
@@ -140,22 +139,6 @@ func (cc *ConnectionChecker) checkMySQLConnection(ctx context.Context, dsn strin
 		} else {
 			res.Error = err.Error()
 		}
-		return &res
-	}
-
-	var count uint64
-	if err = db.QueryRowContext(ctx, "SELECT /* agent='connectionchecker' */ COUNT(*) FROM information_schema.tables").Scan(&count); err != nil {
-		res.Error = err.Error()
-		return &res
-	}
-
-	tableCount := int32(count)
-	if count > math.MaxInt32 {
-		tableCount = math.MaxInt32
-	}
-
-	res.Stats = &agentpb.CheckConnectionResponse_Stats{
-		TableCount: tableCount,
 	}
 
 	return &res

diff --git a/agent/connectionchecker/connection_checker_test.go b/agent/connectionchecker/connection_checker_test.go
@@ -237,7 +237,7 @@ func TestConnectionChecker(t *testing.T) {
 		})
 	}
 
-	t.Run("TableCount", func(t *testing.T) {
+	t.Run("Stats should be empty", func(t *testing.T) {
 		cfgStorage := config.NewStorage(&config.Config{
 			Paths: config.Paths{TempDir: t.TempDir()},
 		})
@@ -247,7 +247,8 @@ func TestConnectionChecker(t *testing.T) {
 			Type: inventorypb.ServiceType_MYSQL_SERVICE,
 		}, 0)
 		require.NotNil(t, resp)
-		assert.InDelta(t, 250, resp.Stats.TableCount, 150)
+		// CheckConnectionResponse_Stats are deprecated, but we can't remove them yet without breaking older clients.
+		assert.Equal(t, (*agentpb.CheckConnectionResponse_Stats)(nil), resp.Stats) //nolint:staticcheck
 	})
 
 	t.Run("MongoDBWithSSL", func(t *testing.T) {