Values overwritten when more than 1 page of data available

[piechu]

Hi,

I have been playing with new relic terraform plugin which is using newrelic-client-go. I noticed that when I was searching for an alert notification channel - depending on environment - sometimes it was found, sometimes not. When I tried to compare the difference between environments - the only one was number of already defined channels. I thought that problem might be with plugin getting just one page of data - but since it is using newrelic-client-go - it should receive all data regardless how many is defined in NewRelic.

Playing with the code - I've noticed that data returned from newrelic-client-go ListChannels method is corrupted - multiple entries were duplicated. I think it is caused by using reference for response (&response) when calling alerts.client.Get - as same object is being reuse - it does not reallocate memory, which cause entries in array overwritten. Call ends up with proper number of elements in alertChannels slice, but with duplicated entries.

I've checked that assigning fresh value for response works as expected. As I'm not a Go expert maybe there is a better solution.

func (alerts *Alerts) ListChannels() ([]*Channel, error) {
	response := alertChannelsResponse{}
	alertChannels := []*Channel{}
	nextURL := "/alerts_channels.json"

	for nextURL != "" {
		resp, err := alerts.client.Get(nextURL, nil, &response)

		if err != nil {
			return nil, err
		}

		alertChannels = append(alertChannels, response.Channels...)

		paging := alerts.pager.Parse(resp)
		nextURL = paging.Next

		response = alertChannelsResponse{}
	}

	return alertChannels, nil
}

Thanks.

[sanderblue]

Hi @piechu, interesting find. Thanks for reporting and digging into this. We'll take a look and report back.

[sanderblue]

@piechu I tried reproducing this issue multiple times with over 300 channels and was unable to reproduce it. Do you have any other information that might help me reproduce the problem? How many channels, etc?

[piechu]

@sanderblue I have around 430 notification channels and every call to New Relic return page with 200 entries. Do you check for number of entries or actual values in table? As number of entries should be correct.

[sanderblue]

I checked for duplicates entries (duplicate IDs).

[sanderblue]

So after doing some testing in a better controlled environment, I was able to reproduce the duplication issue. Thanks again for the investigation on your end. We'll let you know once we get a fix out for this.

[piechu]

@sanderblue great to hear that. I have a unit test covering this scenario - attaching patch here.
test.txt