PMM-11341 Auto discovery limit. #2568

JiriCtvrtka · 2023-10-23T12:03:38Z

Link to the Feature Build:
Percona-Lab/pmm-submodules#3424

PostgreSQL exporter PR: percona/postgres_exporter#162

codecov · 2023-10-23T12:06:17Z

Codecov Report

Attention: 36 lines in your changes are missing coverage. Please review.

Comparison is base (21d018b) 42.75% compared to head (221ab29) 42.77%.

Files	Patch %	Lines
managed/services/agents/service_info_broker.go	0.00%	20 Missing ⚠️
managed/services/management/postgresql.go	0.00%	5 Missing ⚠️
agent/serviceinfobroker/service_info_broker.go	78.94%	3 Missing and 1 partial ⚠️
managed/models/agent_helpers.go	62.50%	3 Missing ⚠️
managed/services/management/rds.go	75.00%	2 Missing ⚠️
.../commands/inventory/add_agent_postgres_exporter.go	0.00%	1 Missing ⚠️
admin/commands/management/add_postgresql.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2568      +/-   ##
==========================================
+ Coverage   42.75%   42.77%   +0.01%     
==========================================
  Files         391      391              
  Lines       49168    49241      +73     
==========================================
+ Hits        21020    21061      +41     
- Misses      26170    26199      +29     
- Partials     1978     1981       +3

Flag	Coverage Δ
admin	`10.49% <50.00%> (+0.04%)`	⬆️
agent	`53.60% <78.94%> (-0.01%)`	⬇️
vmproxy	`68.57% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

managed/models/agent_helpers.go

it-percona-cla · 2023-10-25T13:22:08Z

All committers have signed the CLA.

agent/serviceinfobroker/service_info_broker.go

managed/services/agents/service_info_broker.go

managed/services/agents/state.go

BupycHuk

Overall looks good to me.
Just few comments to improve it

managed/services/agents/connection_checker.go

api/agentpb/agent.proto

BupycHuk · 2023-10-26T20:56:01Z

managed/models/agent_helpers.go

+// PostgreSQLExtendedOptionsParams contains extended parameters for PostgreSQL exporter.
+type PostgreSQLExtendedOptionsParams interface {
+	GetAutoDiscoveryLimit() int32
+}


let's include it to PostgreSQLOptionsParams

Included. Originally I separated it since we dont need such field for QAN agents.

ah, let's revert it then, keep it separated.

btw why do we pass service parameters to agent in inventory API?

@BupycHuk Reverted. It is passed to have limit exposed in inventory UI. It is aligned with MySQL implementation.

got it, let's create task for V3 to standardize it

Task: https://jira.percona.com/browse/PMM-12634

managed/models/agent_model.go

managed/services/agents/connection_checker.go

managed/services/management/rds.go

managed/services/management/postgresql.go

BupycHuk · 2023-11-13T11:23:54Z

admin/commands/inventory/add_agent_mysqld_exporter.go

@@ -94,7 +94,7 @@ type AddAgentMysqldExporterCommand struct {
 	TLSCAFile                 string            `name:"tls-ca" help:"Path to certificate authority certificate file"`
 	TLSCertFile               string            `name:"tls-cert" help:"Path to client certificate file"`
 	TLSKeyFile                string            `name:"tls-key" help:"Path to client key file"`
-	TablestatsGroupTableLimit int32             `placeholder:"number" help:"Tablestats group collectors will be disabled if there are more than that number of tables (default: 0 - always enabled; negative value - always disabled)"`
+	TablestatsGroupTableLimit int32             `placeholder:"number" help:"Tablestats group collectors will be disabled if there are more than that number of tables (default: server-defined)"`


wow, what about negative value? was it disabling or not?

BupycHuk · 2023-11-13T11:29:26Z

managed/services/management/postgresql.go

@@ -27,6 +27,10 @@ import (
 	"github.com/percona/pmm/managed/services"
 )

+const (
+	defaultAutoDiscoveryDatabaseLimit = 1000


let's have it low like 10 or 15

@BupycHuk Should Matěj change this also on FE? Default 1000 is also there (like in case of MySQL).

JiriCtvrtka mentioned this pull request Oct 23, 2023

PMM-11341 Auto discovery limit. percona/postgres_exporter#156

Closed

github-actions bot reviewed Oct 24, 2023

View reviewed changes

managed/models/agent_helpers.go Outdated Show resolved Hide resolved

github-actions bot reviewed Oct 25, 2023

View reviewed changes

agent/serviceinfobroker/service_info_broker.go Outdated Show resolved Hide resolved

PMM-11341 Auto discovery control.

55d5655

JiriCtvrtka force-pushed the PMM-11341-autodiscovery-control branch from c010c78 to 55d5655 Compare October 26, 2023 09:44

PMM-11341 Fix.

2069925

github-actions bot reviewed Oct 26, 2023

View reviewed changes

managed/services/agents/service_info_broker.go Outdated Show resolved Hide resolved

JiriCtvrtka added 3 commits October 26, 2023 12:11

PMM-11341 Same comments on all places.

7fcbd3b

Merge branch 'main' into PMM-11341-autodiscovery-control

1f4fc9f

PMM-11341 Remove live service info broker scrape.

5009442

github-actions bot reviewed Oct 26, 2023

View reviewed changes

managed/services/agents/state.go Outdated Show resolved Hide resolved

PMM-11341 Lint.

07452fd

JiriCtvrtka requested review from ademidoff and artemgavrilov October 26, 2023 11:28

JiriCtvrtka marked this pull request as ready for review October 26, 2023 11:29

JiriCtvrtka requested review from BupycHuk and a team as code owners October 26, 2023 11:29

JiriCtvrtka requested review from idoqo and removed request for a team October 26, 2023 11:29

JiriCtvrtka mentioned this pull request Oct 26, 2023

PMM-11341 Auto discovery limit. Percona-Lab/pmm-submodules#3424

Closed

1 task

PMM-11341 Connection checker for PG.

a00925b

BupycHuk reviewed Oct 26, 2023

View reviewed changes

JiriCtvrtka added 3 commits October 30, 2023 07:17

PMM-11341 Fix comment.

a170704

PMM-11341 Discover limit field even for QAN agents.

cec000c

PMM-11341 Remove version check from connection checker.

0066b0b

BupycHuk reviewed Oct 30, 2023

View reviewed changes

managed/services/agents/connection_checker.go Outdated Show resolved Hide resolved

PMM-11341 Remove whole PG connection check block on managed side.

884c5e5

JiriCtvrtka requested a review from BupycHuk October 30, 2023 07:19

JiriCtvrtka and others added 7 commits November 9, 2023 17:42

PMM-11341 Lint.

2a9830a

PMM-11341 Lower case.

76c2288

Merge branch 'main' into PMM-11341-autodiscovery-control

35bf48c

PMM-11341 FIx MySQL, PG pmm-admin description.

b0a907c

PMM-11341 DB list query moved above.

4935cab

PMM-11341 Lint.

6002c41

PMM-11341 MySQL, PG comments fix for limit.

eda8268

percona deleted a comment from percona-robot Nov 9, 2023

PMM-11341 Lint.

8eb1175

percona-robot reviewed Nov 9, 2023

View reviewed changes

managed/services/management/rds.go Outdated Show resolved Hide resolved

PMM-11341 Remove nolint.

dbfcc1b

percona-robot reviewed Nov 9, 2023

View reviewed changes

managed/services/management/postgresql.go Outdated Show resolved Hide resolved

JiriCtvrtka added 2 commits November 9, 2023 19:18

PMM-11341 Ifs refactor.

0de232a

PMM-12341 Refactor.

6d18ef9

JiriCtvrtka requested review from ademidoff and BupycHuk November 9, 2023 18:53

ademidoff approved these changes Nov 13, 2023

View reviewed changes

BupycHuk reviewed Nov 13, 2023

View reviewed changes

BupycHuk approved these changes Nov 13, 2023

View reviewed changes

JiriCtvrtka and others added 6 commits November 13, 2023 12:45

PMM-11341 Change default limit to 10.

92bfc60

Merge branch 'main' into PMM-11341-autodiscovery-control

6a2d55b

PMM-11341 Small description change.

dcc83e1

PMM-11341 Fix RDS test after change of default value.

6235b38

PMM-11341 Fix description for MySQL.

efc4350

Merge branch 'main' into PMM-11341-autodiscovery-control

c045ca7

JiriCtvrtka force-pushed the PMM-11341-autodiscovery-control branch from 10affb2 to c045ca7 Compare November 16, 2023 09:41

JiriCtvrtka added 2 commits November 16, 2023 10:59

PMM-11341 Fix mariaDB 10.4 expected queryID.

630cf5f

PMM-11341 Fix API tests for add PG.

221ab29

JiriCtvrtka merged commit 6c37873 into main Nov 16, 2023
32 checks passed

JiriCtvrtka deleted the PMM-11341-autodiscovery-control branch November 16, 2023 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PMM-11341 Auto discovery limit. #2568

PMM-11341 Auto discovery limit. #2568

JiriCtvrtka commented Oct 23, 2023 •

edited

Loading

codecov bot commented Oct 23, 2023 •

edited

Loading

it-percona-cla commented Oct 25, 2023 •

edited

Loading

BupycHuk left a comment

BupycHuk Oct 26, 2023

JiriCtvrtka Oct 30, 2023

BupycHuk Oct 30, 2023

BupycHuk Oct 30, 2023

JiriCtvrtka Oct 30, 2023

BupycHuk Oct 30, 2023

JiriCtvrtka Oct 30, 2023

BupycHuk Nov 13, 2023

BupycHuk Nov 13, 2023

JiriCtvrtka Nov 13, 2023

BupycHuk Nov 13, 2023

PMM-11341 Auto discovery limit. #2568

PMM-11341 Auto discovery limit. #2568

Conversation

JiriCtvrtka commented Oct 23, 2023 • edited Loading

codecov bot commented Oct 23, 2023 • edited Loading

Codecov Report

it-percona-cla commented Oct 25, 2023 • edited Loading

BupycHuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiriCtvrtka commented Oct 23, 2023 •

edited

Loading

codecov bot commented Oct 23, 2023 •

edited

Loading

it-percona-cla commented Oct 25, 2023 •

edited

Loading