Skip to content

improve(go.d/smartctl): add configurable concurrent device scanning#20569

Merged
ilyam8 merged 3 commits intonetdata:masterfrom
ilyam8:go.d-smartctl-conc-scan
Jun 24, 2025
Merged

improve(go.d/smartctl): add configurable concurrent device scanning#20569
ilyam8 merged 3 commits intonetdata:masterfrom
ilyam8:go.d-smartctl-conc-scan

Conversation

@ilyam8
Copy link
Member

@ilyam8 ilyam8 commented Jun 24, 2025

Summary
Test Plan
Additional Information
For users: How does this change affect me?

@ilyam8 ilyam8 requested a review from Copilot June 24, 2025 19:40
@github-actions github-actions bot added area/collectors Everything related to data collection collectors/go.d area/metadata Integrations metadata area/go labels Jun 24, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds configurable concurrent scanning for devices in the smartctl collector to improve performance when monitoring many devices. Key changes include:

  • Introducing the new concurrent_scans configuration in YAML, JSON, metadata, and schema.
  • Updating documentation and examples to describe concurrent scanning behavior.
  • Implementing concurrent scanning logic in the collector and adding corresponding test cases.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/go/plugin/go.d/collector/smartctl/testdata/config.yaml Added concurrent_scans field to YAML configuration.
src/go/plugin/go.d/collector/smartctl/testdata/config.json Added concurrent_scans field to JSON configuration.
src/go/plugin/go.d/collector/smartctl/metadata.yaml Updated metadata and examples to document concurrent scanning.
src/go/plugin/go.d/collector/smartctl/config_schema.json Updated configuration schema to include concurrent_scans.
src/go/plugin/go.d/collector/smartctl/collector_test.go Added a new test case for concurrent scanning devices.
src/go/plugin/go.d/collector/smartctl/collector.go Set default ConcurrentScans value in the Collector configuration.
src/go/plugin/go.d/collector/smartctl/collect.go Implemented concurrent scanning logic and merged results concurrently using a pool.
Comments suppressed due to low confidence (2)

src/go/plugin/go.d/collector/smartctl/collect.go:54

  • The concurrent scanning is only enabled when more than one device is present. If a single device may still benefit from concurrent treatment in future scenarios, consider documenting this behavior or adjusting the condition.
	if c.ConcurrentScans > 0 && len(c.scannedDevices) > 1 {

src/go/plugin/go.d/collector/smartctl/collect.go:84

  • Merging metrics with maps.Copy overwrites keys from previous devices if duplicated. Ensure that device-specific metric keys are unique or adjust the merging strategy to prevent potential data loss.
			maps.Copy(mx, tempMx)

stelfrag
stelfrag previously approved these changes Jun 24, 2025
@ilyam8 ilyam8 marked this pull request as draft June 24, 2025 19:59
@ilyam8 ilyam8 marked this pull request as ready for review June 24, 2025 20:12
@ilyam8 ilyam8 merged commit ff448d1 into netdata:master Jun 24, 2025
104 of 105 checks passed
@ilyam8 ilyam8 deleted the go.d-smartctl-conc-scan branch June 24, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/collectors Everything related to data collection area/go area/metadata Integrations metadata collectors/go.d

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants