Skip to content

Conversation

@BupycHuk
Copy link
Member

PMM-5795

Link to the Feature Build: SUBMODULES-0

If this PR adds or removes or alters one or more API endpoints, please review and add or update the relevant API documents as well:

  • API Docs updated

If this PR is related to some other PRs in this or other repositories, please provide links to those PRs:

  • Links to related pull requests (optional).

- Introduced a new DebugCommand to facilitate debugging of exporter data collection for specific agents.
- Added methods for validating resolutions, collecting metrics, and fetching logs.
- Enhanced the CLI with a new debug command option in cli.go.
- Implemented functionality to handle agent status and service type constants.
- Created unit tests for the DebugCommand to ensure reliability and correctness.
- Updated existing code to utilize the new debug functionalities where applicable.

This commit enhances the debugging capabilities of the PMM agent, allowing for better diagnostics and troubleshooting.
- Deleted the agent binary and configuration files for mysqld, node, and postgres exporters.
- Cleared temporary files related to VM agent persistent queues.
- This cleanup helps streamline the agent's file structure and removes unused resources.
- Deleted the PMM agent binary file, streamlining the file structure and removing unused resources.
- This change follows the recent cleanup of agent configuration files and temporary data.
- Updated the `fetchAgentLogs` method to support fetching logs for multiple agents in a single request, improving efficiency.
- Modified the `ZipLogs` method to handle multiple `agent_id` query parameters, allowing for flexible log retrieval.
- Adjusted the `AgentsLogs` method to simplify log retrieval by directly using agent IDs without composite keys.
- Added unit tests to ensure the new functionality works as expected, validating the handling of multiple agent logs.

This commit improves the logging capabilities of the PMM agent, facilitating better diagnostics and reducing the number of requests needed for log retrieval.
@codecov
Copy link

codecov bot commented Oct 12, 2025

Codecov Report

❌ Patch coverage is 14.20205% with 586 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.09%. Comparing base (13c4fd3) to head (7863855).
⚠️ Report is 3 commits behind head on v3.

Files with missing lines Patch % Lines
admin/commands/debug.go 12.46% 558 Missing and 4 partials ⚠️
admin/commands/base.go 0.00% 18 Missing ⚠️
agent/agentlocal/agent_local.go 88.88% 1 Missing and 1 partial ⚠️
agent/agents/supervisor/supervisor.go 0.00% 2 Missing ⚠️
admin/cmd/bootstrap.go 0.00% 1 Missing ⚠️
admin/commands/inventory/list_agents.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v3    #4652      +/-   ##
==========================================
- Coverage   44.56%   44.09%   -0.48%     
==========================================
  Files         363      364       +1     
  Lines       45706    46375     +669     
==========================================
+ Hits        20369    20448      +79     
- Misses      23677    24263     +586     
- Partials     1660     1664       +4     
Flag Coverage Δ
admin 16.58% <12.08%> (-0.76%) ⬇️
agent 53.24% <80.00%> (-0.07%) ⬇️
managed 44.52% <100.00%> (ø)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Merged the declaration of `DebugServiceTypesKeys` into a single line for improved readability.
- Updated the file permission syntax in `fetchAgentLogs` from `0600` to `0o600` to align with Go's octal notation.

These changes enhance code clarity and maintain consistency with Go best practices.
- Introduced a new `category` field in the `TestCreateAgentInfo` function to enhance the test cases for agent creation.
- Updated the `createAgentInfo` function call to include the `category` parameter, ensuring that the tests validate the agent's category correctly.

This change improves the test coverage for agent information, allowing for better validation of agent categories in the system.
Copy link

@percona-robot percona-robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

golangci

🚫 [golangci] reported by reviewdog 🐶
unnecessary conversion (unconvert)

int64(agent.ListenPort), serviceID, AgentCategoryExporter))


🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

req, err := http.NewRequestWithContext(ctx, "GET", vmagentURL, nil)


🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

req, err := http.NewRequestWithContext(ctx, "GET", exporterURL, nil)


🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

req, err := http.NewRequestWithContext(ctx, "GET", pmmAgentURL, nil)

// findAgentsByServiceType finds agents based on service type
func (cmd *DebugCommand) findAgentsByServiceType(ctx context.Context) ([]*agentInfo, error) {
// Get local node information
status, err := agentlocal.GetStatus(agentlocal.DoNotRequestNetworkInfo)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Function GetStatus should pass the context parameter (contextcheck)

}

// extractServiceAgents extracts exporter and QAN agents for a service from agents response
func (cmd *DebugCommand) extractServiceAgents(payload *agentsService.ListAgentsOKBody, serviceID string) []*agentInfo {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
calculated cyclomatic complexity for function extractServiceAgents is 40, max is 30 (cyclop)

if err != nil {
return nil, nil, errors.Wrap(err, "failed to fetch vmagent targets")
}
defer resp.Body.Close()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Error return value of resp.Body.Close is not checked (errcheck)

if err != nil {
return 0, errors.Wrap(err, "failed to fetch metrics")
}
defer resp.Body.Close()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Error return value of resp.Body.Close is not checked (errcheck)

if err != nil {
return 0, errors.Wrap(err, "failed to create output file")
}
defer file.Close()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Error return value of file.Close is not checked (errcheck)

assert.Equal(t, "/api/v1/targets", r.URL.Path)
w.Header().Set("Content-Type", "application/json")
_, err := w.Write([]byte(tt.serverJSON))
require.NoError(t, err)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
go-require: do not use require in http handlers (testifylint)

// Create test server
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, err := w.Write([]byte(tt.serverContent))
require.NoError(t, err)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
go-require: do not use require in http handlers (testifylint)

// 2. pmm-agent.log (always included)
// 3. NOT other agents' logs
// Count of files should be exactly 2
assert.Equal(t, 2, len(zipExs.File), "Should contain exactly 2 files: agent log + pmm-agent.log")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
len: use assert.Len (testifylint)

// 2. Second agent's log file
// 3. pmm-agent.log (always included)
// Count of files should be exactly 3
assert.Equal(t, 3, len(zipExs.File), "Should contain exactly 3 files: 2 agent logs + pmm-agent.log")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
len: use assert.Len (testifylint)

}
nodeAgents = append(nodeAgents, createAgentInfo(
agent.AgentID, "NODE_EXPORTER", GetAgentStatus(agent.Status),
int64(agent.ListenPort), "", AgentCategoryExporter))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
unnecessary conversion (unconvert)

AgentID: agentID,
AgentType: agentType,
Status: status,
ListenPort: int64(listenPort),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
unnecessary conversion (unconvert)

}
serviceAgents = append(serviceAgents, createAgentInfo(
agent.AgentID, "MYSQLD_EXPORTER", GetAgentStatus(agent.Status),
int64(agent.ListenPort), serviceID, AgentCategoryExporter))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
unnecessary conversion (unconvert)

vmagentURL := fmt.Sprintf("http://%s/api/v1/targets",
net.JoinHostPort(agentlocal.Localhost, strconv.FormatInt(vmagentPort, 10)))

req, err := http.NewRequestWithContext(ctx, "GET", vmagentURL, nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

}

// Create request with context
req, err := http.NewRequestWithContext(ctx, "GET", exporterURL, nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

}

// Create request
req, err := http.NewRequestWithContext(ctx, "GET", pmmAgentURL, nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
"GET" can be replaced by http.MethodGet (usestdlibvars)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants