Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/httpcheck] refactor client length checking #31036

Closed
wants to merge 2 commits into from
Closed

[receiver/httpcheck] refactor client length checking #31036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aaf8e9e
[receiver/httpcheck] refactor client length checking
tranngoclam Feb 5, 2024
8b5b433
[receiver/httpcheck] fix typo in test
tranngoclam Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions receiver/httpcheckreceiver/scraper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ type httpcheckScraper struct {
// start starts the scraper by creating a new HTTP Client on the scraper
func (h *httpcheckScraper) start(_ context.Context, host component.Host) (err error) {
for _, target := range h.cfg.Targets {
client, clentErr := target.ToClient(host, h.settings)
if clentErr != nil {
err = multierr.Append(err, clentErr)
client, clientErr := target.ToClient(host, h.settings)
if clientErr != nil {
err = multierr.Append(err, clientErr)
}
h.clients = append(h.clients, client)
}
Expand All @@ -46,12 +46,13 @@ func (h *httpcheckScraper) start(_ context.Context, host component.Host) (err er

// scrape connects to the endpoint and produces metrics based on the response
func (h *httpcheckScraper) scrape(ctx context.Context) (pmetric.Metrics, error) {
if h.clients == nil || len(h.clients) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a case where this change catches something that didn't used to be caught? If clients is nil, it returns the error, if the length is 0, it returns the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's the same logic. The new change is to remove redundant checking. If a slice is nil then its length is also zero

length := len(h.clients)
if length == 0 {
return pmetric.NewMetrics(), errClientNotInit
}

var wg sync.WaitGroup
wg.Add(len(h.clients))
wg.Add(length)
var mux sync.Mutex

for idx, client := range h.clients {
Expand Down
2 changes: 1 addition & 1 deletion receiver/httpcheckreceiver/scraper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestScraperStart(t *testing.T) {
}
}

func TestScaperScrape(t *testing.T) {
func TestScraperScrape(t *testing.T) {
testCases := []struct {
desc string
expectedResponse int
Expand Down