PMM-14109 Improve Advisor UX

.github/workflows/managed.yml

@@ -61,7 +61,6 @@ jobs:
             -v ./:/root/go/src/github.com/percona/pmm \
             -v ~/go-mod-cache:/home/pmm/.cache/go/mod \
             -v ~/go-build-cache:/home/pmm/.cache/go-build \
-            -v ./managed/data/advisors/:/usr/local/percona/advisors/ \
             -v ./managed/data/checks/:/usr/local/percona/checks/ \
             -v ./managed/data/alerting-templates/:/usr/local/percona/alerting-templates/ \
             -v ./.devcontainer/Makefile:/root/go/src/github.com/percona/pmm/Makefile:ro \

.github/workflows/percona-intelligence.yml

@@ -4,7 +4,6 @@ on:
   pull_request:
     paths:
       - 'managed/data/alerting-templates/**'
-      - 'managed/data/advisors/**'
       - 'managed/data/checks/**'
 
 permissions:

.gitignore

@@ -8,7 +8,6 @@ settings.json
 # Temporary (OS, build, config, etc)
 .DS_Store
 /tmp/
-
 .env
 .netrc
 .modules
@@ -18,7 +17,6 @@ packer.log
 ci.yml
 Makefile.local
 
-
 # PMM specific
 bin
 /dev/clickhouse-backups/

AGENTS.md

@@ -172,6 +172,7 @@ Relationships:
 - Import grouping: stdlib, then external (`github.com/percona`, third-party), then internal (this repo)
 - Use `any` instead of `interface{}`
 - Use modern slice helpers (`slices.Contains`), range loops
+- Use `new(expr)` instead of `pointer.To(expr)` when a pointer to a value is needed
 - Don't use named return values
 - Don't inline comments (`code // comment`); put comments on separate lines
 - Don't add obvious/redundant comments; only comment non-obvious intent

Makefile.include

@@ -59,16 +59,25 @@ check-all: check-license check    ## Run linter and license checks
 check-new:
 	bin/golangci-lint run -c=.golangci.yml --new-from-rev=origin/main --new
 
-FILES = $(shell find . -type f -name '*.go')
+# Files passed to the Go formatters: the FILES override if given, the whole tree otherwise.
+FILES ?=
+GO_FILES = $(if $(strip $(FILES)),$(FILES),$(shell find . -type f -name '*.go'))
 
-format:               ## Format source code
+# Go files changed on the current branch (committed, staged or unstaged), plus untracked ones.
+CHANGED_GO_FILES = $(sort $(shell git diff --name-only --diff-filter=d $$(git merge-base main HEAD) -- '*.go'; git ls-files --others --exclude-standard -- '*.go'))
+
+format:               ## Format source code; pass FILES="a.go b.go" to format only those files
 	make -C api format
-	go tool gofumpt -l -w $(FILES)
-	go tool goimports -local github.com/percona/pmm -l -w $(FILES)
-	go tool gci write --section Standard --section Default --section "Prefix(github.com/percona/pmm)" $(FILES)
+	go tool gofumpt -l -w $(GO_FILES)
+	go tool goimports -local github.com/percona/pmm -l -w $(GO_FILES)
+	go tool gci write --section Standard --section Default --section "Prefix(github.com/percona/pmm)" $(GO_FILES)
+
+format-changed:       ## Format the Go files changed on this branch (much faster than `make format`)
+	@files="$(CHANGED_GO_FILES)"; \
+	if [ -z "$$files" ]; then echo "No changed Go files."; else $(MAKE) format FILES="$$files"; fi
 
 format-fast:         ## Format only (without running goimports and gci)
-	@go tool gofumpt -l -w $(FILES)
+	@go tool gofumpt -l -w $(GO_FILES)
 
 serve:                ## Serve API documentation with nginx
 	nginx -p . -c api/nginx/nginx.conf

agent/docker-compose.yml

@@ -8,8 +8,6 @@ services:
       - "127.0.0.1:443:8443"
     environment:
       - PMM_DEBUG=1
-      # for local development
-      - PMM_DEV_ADVISOR_CHECKS_FILE=/srv/checks/custom-checks.yml
     volumes:
       - ./testdata/checks:/srv/checks
 

api-tests/docker-compose.yml

@@ -7,8 +7,6 @@ services:
       - 127.0.0.1:443:8443
     environment:
       - PMM_DEBUG=1
-      # for local development
-      # - PMM_DEV_ADVISOR_CHECKS_FILE=/srv/checks/custom-checks.yml
     volumes:
       - ./testdata/checks:/srv/checks
 

api-tests/server/advisors_test.go

@@ -50,30 +50,6 @@ func TestStartChecks(t *testing.T) {
 	})
 }
 
-func TestGetAdvisorCheckResults(t *testing.T) {
-	t.Run("with disabled Advisors", func(t *testing.T) {
-		toggleAdvisorChecks(t, false)
-		t.Cleanup(func() { RestoreSettingsDefaults(t) })
-
-		results, err := advisorClient.Default.AdvisorService.GetFailedChecks(nil)
-		pmmapitests.AssertAPIErrorf(t, err, 400, codes.FailedPrecondition, `advisor checks are disabled.`)
-		assert.Nil(t, results)
-	})
-
-	t.Run("with enabled Advisors", func(t *testing.T) {
-		toggleAdvisorChecks(t, true)
-		t.Cleanup(func() { RestoreSettingsDefaults(t) })
-
-		resp, err := advisorClient.Default.AdvisorService.StartAdvisorChecks(nil)
-		require.NoError(t, err)
-		assert.NotNil(t, resp)
-
-		results, err := advisorClient.Default.AdvisorService.GetFailedChecks(nil)
-		require.NoError(t, err)
-		assert.NotNil(t, results)
-	})
-}
-
 func TestListAdvisorChecks(t *testing.T) {
 	toggleAdvisorChecks(t, true)
 	t.Cleanup(func() { RestoreSettingsDefaults(t) })
@@ -98,11 +74,8 @@ func TestListAdvisors(t *testing.T) {
 	assert.NotNil(t, resp)
 	assert.NotEmpty(t, resp.Payload.Advisors)
 	for _, a := range resp.Payload.Advisors {
-		assert.NotEmpty(t, a.Name, "%+v", a)
-		assert.NotEmpty(t, a.Summary, "%+v", a)
-		assert.NotEmpty(t, a.Description, "%+v", a)
 		assert.NotEmpty(t, a.Category, "%+v", a)
-		assert.NotEmpty(t, a.Comment, "%+v", a)
+		assert.NotEmpty(t, a.Subcategory, "%+v", a)
 		assert.NotEmpty(t, a.Checks, "%+v", a)
 
 		for _, c := range a.Checks {