lib/promscrape/discovery/vultr: follow-up after 17e3d019d2

- Sort the discovered labels in alphabetical order at https://docs.victoriametrics.com/sd_configs/#vultr_sd_configs
- Rename VultrConfigs to VultrSDConfigs to be consistent with the naming for other SD configs.
- Prepare query arg filters for `list instances API` at newAPIConfig() instead of passing them in a separate listParams struct.
  This simplifies the code a bit.
- Return error when bearer token isn't set at vultr_sd_configs, since this token is mandatory
  according to https://docs.victoriametrics.com/sd_configs/#vultr_sd_configs
- Remove unused fields from the parsed response from Vultr list instances API in order to simplify the code a bit.
- Remove double logging of errors inside getInstances() function, since these errors must be already logged by the caller.
- Simplify tests, so they are easier to maintain.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6041
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6068

docs/sd_configs.md

@@ -1547,28 +1547,28 @@ scrape_configs:
 
 ```
 
-Each discovered target has an [`__address__`](https://docs.victoriametrics.com/relabeling.html#how-to-modify-scrape-urls-in-targets) label set
+Each discovered target has an [`__address__`](https://docs.victoriametrics.com/relabeling/#how-to-modify-scrape-urls-in-targets) label set
 to `<FQDN>:<port>`, where FQDN is discovered instance address and `<port>` is the port from the `vultr_sd_configs` (default port is `80`).
 
-The following meta labels are available on discovered targets during [relabeling](https://docs.victoriametrics.com/vmagent.html#relabeling):
+The following meta labels are available on discovered targets during [relabeling](https://docs.victoriametrics.com/vmagent/#relabeling):
 
-* `__meta_vultr_instance_id`: A unique ID for the VPS Instance.
+* `__meta_vultr_instance_allowed_bandwidth_gb`: monthly bandwidth quota in GB.
-* `__meta_vultr_instance_label`: The user-supplied label for this instance.
+* `__meta_vultr_instance_disk_gb`: the size of the disk in GB.
-* `__meta_vultr_instance_os`: The [Operating System name](https://www.vultr.com/api/#operation/list-os).
+* `__meta_vultr_instance_features`: comma-separated list of features avabilable to instance, such as "auto_backups", "ipv6", "ddos_protection".
-* `__meta_vultr_instance_os_id`: The [Operating System id](https://www.vultr.com/api/#operation/list-os) used by this instance.
+* `__meta_vultr_instance_hostname`: hostname for this instance.
-* `__meta_vultr_instance_region`: The [Region id](https://www.vultr.com/api/#operation/list-regions) where the Instance is located.
+* `__meta_vultr_instance_id`: unique ID for the VPS Instance.
-* `__meta_vultr_instance_plan`: A unique ID for the Plan.
+* `__meta_vultr_instance_internal_ip`: internal IP used by this instance, if set. Only relevant when a VPC is attached.
-* `__meta_vultr_instance_main_ip`: The main IPv4 address.
+* `__meta_vultr_instance_label`: user-supplied label for this instance.
-* `__meta_vultr_instance_internal_ip`: The internal IP used by this instance, if set. Only relevant when a VPC is attached.
+* `__meta_vultr_instance_main_ip`: main IPv4 address.
-* `__meta_vultr_instance_main_ipv6`: The main IPv6 network address.
+* `__meta_vultr_instance_main_ipv6`: main IPv6 network address.
-* `__meta_vultr_instance_hostname`: The hostname for this instance.
+* `__meta_vultr_instance_os`: [operating System name](https://www.vultr.com/api/#operation/list-os).
-* `__meta_vultr_instance_server_status`: The server health status, which could be `none`, `locked`, `installingbooting`, `ok`.
+* `__meta_vultr_instance_os_id`: [operating System id](https://www.vultr.com/api/#operation/list-os) used by this instance.
-* `__meta_vultr_instance_vcpu_count`: Number of vCPUs.
+* `__meta_vultr_instance_plan`: unique ID for the Plan.
-* `__meta_vultr_instance_ram_mb`: The amount of RAM in MB.
+* `__meta_vultr_instance_ram_mb`: the amount of RAM in MB.
-* `__meta_vultr_instance_allowed_bandwidth_gb`: Monthly bandwidth quota in GB.
+* `__meta_vultr_instance_region`: [region id](https://www.vultr.com/api/#operation/list-regions) where the Instance is located.
-* `__meta_vultr_instance_disk_gb`: The size of the disk in GB.
+* `__meta_vultr_instance_server_status`: server health status, which could be `none`, `locked`, `installingbooting`, `ok`.
-* `__meta_vultr_instance_features`: "auto_backups", "ipv6", "ddos_protection".
+* `__meta_vultr_instance_tags`: comma-separated list of tags applied to the instance.
-* `__meta_vultr_instance_tags`: Tags to apply to the instance.
+* `__meta_vultr_instance_vcpu_count`: the number of vCPUs.
 
 The list of discovered Vultr targets is refreshed at the interval, which can be configured via `-promscrape.vultrSDCheckInterval` command-line flag, default: 30s.
 

lib/promscrape/config.go

@@ -313,7 +313,7 @@ type ScrapeConfig struct {
 	NomadSDConfigs        []nomad.SDConfig        `yaml:"nomad_sd_configs,omitempty"`
 	OpenStackSDConfigs    []openstack.SDConfig    `yaml:"openstack_sd_configs,omitempty"`
 	StaticConfigs         []StaticConfig          `yaml:"static_configs,omitempty"`
-	VultrConfigs          []vultr.SDConfig        `yaml:"vultr_configs,omitempty"`
+	VultrSDConfigs        []vultr.SDConfig        `yaml:"vultr_configs,omitempty"`
 	YandexCloudSDConfigs  []yandexcloud.SDConfig  `yaml:"yandexcloud_sd_configs,omitempty"`
 
 	// These options are supported only by lib/promscrape.
@@ -394,8 +394,11 @@ func (sc *ScrapeConfig) mustStop() {
 	for i := range sc.OpenStackSDConfigs {
 		sc.OpenStackSDConfigs[i].MustStop()
 	}
-	for i := range sc.VultrConfigs {
+	for i := range sc.VultrSDConfigs {
-		sc.VultrConfigs[i].MustStop()
+		sc.VultrSDConfigs[i].MustStop()
+	}
+	for i := range sc.YandexCloudSDConfigs {
+		sc.YandexCloudSDConfigs[i].MustStop()
 	}
 }
 
@@ -757,8 +760,8 @@ func (cfg *Config) getOpenStackSDScrapeWork(prev []*ScrapeWork) []*ScrapeWork {
 // getVultrSDScrapeWork returns `vultr_sd_configs` ScrapeWork from cfg.
 func (cfg *Config) getVultrSDScrapeWork(prev []*ScrapeWork) []*ScrapeWork {
 	visitConfigs := func(sc *ScrapeConfig, visitor func(sdc targetLabelsGetter)) {
-		for i := range sc.VultrConfigs {
+		for i := range sc.VultrSDConfigs {
-			visitor(&sc.VultrConfigs[i])
+			visitor(&sc.VultrSDConfigs[i])
 		}
 	}
 	return cfg.getScrapeWorkGeneric(visitConfigs, "vultr_sd_config", prev)

lib/promscrape/discovery/vultr/api.go

@@ -2,6 +2,7 @@ package vultr
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discoveryutils"
 )
@@ -11,22 +12,7 @@ type apiConfig struct {
 	c    *discoveryutils.Client
 	port int
 
-	listParams
+	listQueryParams string
-}
-
-// listParams is the query params of vultr ListInstance API.
-type listParams struct {
-	// paging params are not exposed to user, they will be filled
-	// dynamically during request. See `getInstances`.
-	// perPage int
-	// cursor string
-
-	// API query params for filtering.
-	label           string
-	mainIP          string
-	region          string
-	firewallGroupID string
-	hostname        string
 }
 
 // getAPIConfig get or create API config from configMap.
@@ -48,6 +34,10 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
 	// See: https://www.vultr.com/api/
 	apiServer := "https://api.vultr.com"
 
+	if sdc.HTTPClientConfig.BearerToken == nil {
+		return nil, fmt.Errorf("missing `bearer_token` option")
+	}
+
 	ac, err := sdc.HTTPClientConfig.NewConfig(baseDir)
 	if err != nil {
 		return nil, fmt.Errorf("cannot parse auth config: %w", err)
@@ -61,16 +51,31 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
 	if err != nil {
 		return nil, fmt.Errorf("cannot create client for %q: %w", apiServer, err)
 	}
+
+	// Prepare additional query params for list instance API.
+	// See https://www.vultr.com/api/#tag/instances/operation/list-instances
+	var qp url.Values
+	if sdc.Label != "" {
+		qp.Set("label", sdc.Label)
+	}
+	if sdc.MainIP != "" {
+		qp.Set("main_ip", sdc.MainIP)
+	}
+	if sdc.Region != "" {
+		qp.Set("region", sdc.Region)
+	}
+	if sdc.FirewallGroupID != "" {
+		qp.Set("firewall_group_id", sdc.FirewallGroupID)
+	}
+	if sdc.Hostname != "" {
+		qp.Set("hostname", sdc.Hostname)
+	}
+
 	cfg := &apiConfig{
 		c:    c,
 		port: port,
-		listParams: listParams{
+
-			label:           sdc.Label,
+		listQueryParams: qp.Encode(),
-			mainIP:          sdc.MainIP,
-			region:          sdc.Region,
-			firewallGroupID: sdc.FirewallGroupID,
-			hostname:        sdc.Hostname,
-		},
 	}
 	return cfg, nil
 }

lib/promscrape/discovery/vultr/api_test.go

@@ -2,15 +2,30 @@ package vultr
 
 import (
 	"testing"
+
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promauth"
 )
 
-func TestNewAPIConfig(t *testing.T) {
+func TestNewAPIConfig_Failure(t *testing.T) {
-
 	sdc := &SDConfig{}
 	baseDir := "."
 	_, err := newAPIConfig(sdc, baseDir)
+	if err == nil {
+		t.Fatalf("expecting non-nil error")
+	}
+}
+
+func TestNewAPIConfig_Success(t *testing.T) {
+	sdc := &SDConfig{
+		HTTPClientConfig: promauth.HTTPClientConfig{
+			BearerToken: &promauth.Secret{
+				S: "foobar",
+			},
+		},
+	}
+	baseDir := "."
+	_, err := newAPIConfig(sdc, baseDir)
 	if err != nil {
-		t.Errorf("newAPIConfig failed with, err: %v", err)
+		t.Fatalf("newAPIConfig failed with, err: %v", err)
-		return
 	}
 }

lib/promscrape/discovery/vultr/instance.go

@@ -4,21 +4,20 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/url"
-
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 )
 
 // ListInstanceResponse is the response structure of Vultr ListInstance API.
 type ListInstanceResponse struct {
 	Instances []Instance `json:"instances"`
-	Meta      *Meta      `json:"Meta"`
+	Meta      Meta       `json:"meta"`
 }
 
 // Instance represents Vultr Instance (VPS).
+//
 // See: https://github.com/vultr/govultr/blob/5125e02e715ae6eb3ce854f0e7116c7ce545a710/instance.go#L81
 type Instance struct {
 	ID               string   `json:"id"`
-	Os               string   `json:"os"`
+	OS               string   `json:"os"`
 	RAM              int      `json:"ram"`
 	Disk             int      `json:"disk"`
 	MainIP           string   `json:"main_ip"`
@@ -30,39 +29,22 @@ type Instance struct {
 	Hostname         string   `json:"hostname"`
 	Label            string   `json:"label"`
 	InternalIP       string   `json:"internal_ip"`
-	OsID             int      `json:"os_id"`
+	OSID             int      `json:"os_id"`
 	Features         []string `json:"features"`
 	Plan             string   `json:"plan"`
 	Tags             []string `json:"tags"`
-
-	// The following fields are defined in the response but are not used during service discovery.
-	//DefaultPassword string `json:"default_password,omitempty"`
-	//DateCreated     string `json:"date_created"`
-	//Status          string `json:"status"`
-	//PowerStatus     string `json:"power_status"`
-	//NetmaskV4       string `json:"netmask_v4"`
-	//GatewayV4       string `json:"gateway_v4"`
-	//V6Network       string `json:"v6_network"`
-	//V6NetworkSize   int    `json:"v6_network_size"`
-	//// Deprecated: Tag should no longer be used. Instead, use Tags.
-	//Tag             string `json:"tag"`
-	//KVM             string `json:"kvm"`
-	//AppID           int    `json:"app_id"`
-	//ImageID         string `json:"image_id"`
-	//FirewallGroupID string `json:"firewall_group_id"`
-	//UserScheme      string `json:"user_scheme"`
 }
 
 // Meta represents the available pagination information
+//
+// See https://www.vultr.com/api/#section/Introduction/Meta-and-Pagination
 type Meta struct {
-	Total int `json:"total"`
+	Links Links `json:"links"`
-	Links *Links
 }
 
 // Links represent the next/previous cursor in your pagination calls
 type Links struct {
 	Next string `json:"next"`
-	Prev string `json:"prev"`
 }
 
 // getInstances retrieve instance from Vultr HTTP API.
@@ -70,39 +52,30 @@ func getInstances(cfg *apiConfig) ([]Instance, error) {
 	var instances []Instance
 
 	// prepare GET params
-	params := url.Values{}
+	queryParams := cfg.listQueryParams
-	params.Set("per_page", "100")
-	params.Set("label", cfg.label)
-	params.Set("main_ip", cfg.mainIP)
-	params.Set("region", cfg.region)
-	params.Set("firewall_group_id", cfg.firewallGroupID)
-	params.Set("hostname", cfg.hostname)
 
 	// send request to vultr API
 	for {
 		// See: https://www.vultr.com/api/#tag/instances/operation/list-instances
-		path := fmt.Sprintf("/v2/instances?%s", params.Encode())
+		path := "/v2/instances?" + queryParams + "&per_page=100"
-		resp, err := cfg.c.GetAPIResponse(path)
+		data, err := cfg.c.GetAPIResponse(path)
 		if err != nil {
-			logger.Errorf("get response from vultr failed, path:%s, err: %v", path, err)
+			return nil, fmt.Errorf("cannot get Vultr response from %q: %w", path, err)
-			return nil, err
 		}
 
-		var listInstanceResp ListInstanceResponse
+		var resp ListInstanceResponse
-		if err = json.Unmarshal(resp, &listInstanceResp); err != nil {
+		if err := json.Unmarshal(data, &resp); err != nil {
-			logger.Errorf("unmarshal response from vultr failed, err: %v", err)
+			return nil, fmt.Errorf("cannot unmarshal ListInstanceResponse obtained from %q: %w; response=%q", path, err, data)
-			return nil, err
 		}
 
-		instances = append(instances, listInstanceResp.Instances...)
+		instances = append(instances, resp.Instances...)
 
-		if listInstanceResp.Meta != nil && listInstanceResp.Meta.Links != nil && listInstanceResp.Meta.Links.Next != "" {
+		if resp.Meta.Links.Next == "" {
-			// if `next page` is available, set the cursor param and request again.
-			params.Set("cursor", listInstanceResp.Meta.Links.Next)
-		} else {
-			// otherwise exit the loop
 			break
 		}
+
+		// if `next page` is available, set the cursor param and request again.
+		queryParams = cfg.listQueryParams + "&cursor=" + url.QueryEscape(resp.Meta.Links.Next)
 	}
 
 	return instances, nil

lib/promscrape/discovery/vultr/instance_test.go

@@ -1,7 +1,6 @@
 package vultr
 
 import (
-	"errors"
 	"reflect"
 	"testing"
 
@@ -9,175 +8,146 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discoveryutils"
 )
 
-// TestGetInstances runs general test cases for GetInstances
+func TestGetInstances_Success(t *testing.T) {
-func TestGetInstances(t *testing.T) {
+	s := newMockVultrServer(func() []byte {
-	testCases := []struct {
+		const resp = `{
-		name           string
+			"instances": [{
-		apiResponse    string
+			"id": "fake-id-07f7-4b68-88ac-fake-id",
-		apiError       bool
+			"os": "Ubuntu 22.04 x64",
-		expectError    bool
+			"ram": 1024,
-		expectResponse []Instance
+			"disk": 25,
-	}{
+			"main_ip": "64.176.84.27",
-		{
+			"vcpu_count": 1,
-			name:           "success response",
+			"region": "sgp",
-			apiResponse:    mockListInstanceSuccessResp,
+			"plan": "vc2-1c-1gb",
-			apiError:       false,
+			"date_created": "2024-04-05T05:41:28+00:00",
-			expectError:    false,
+			"status": "active",
-			expectResponse: expectSuccessInstances,
+			"allowed_bandwidth": 1,
-		},
+			"netmask_v4": "255.255.254.0",
-		{
+			"gateway_v4": "64.176.63.2",
-			name:           "failed response",
+			"power_status": "running",
-			apiResponse:    mockListInstanceFailedResp,
+			"server_status": "installingbooting",
-			apiError:       true,
+			"v6_network": "2002:18f0:4100:263a::",
-			expectError:    true,
+			"v6_main_ip": "2002:18f0:4100:263a:5300:07ff:fdd7:691c",
-			expectResponse: nil,
+			"v6_network_size": 64,
-		},
+			"label": "vultr-sd",
-	}
+			"internal_ip": "",
+			"kvm": "https:\/\/my.vultr.com\/subs\/vps\/novnc\/api.php?data=secret_data_string",
+			"hostname": "vultr-sd",
+			"tag": "",
+			"tags": [],
+			"os_id": 1743,
+			"app_id": 0,
+			"image_id": "",
+			"firewall_group_id": "",
+			"features": ["ipv6"],
+			"user_scheme": "root"
+		}]
+	}`
 
-	for _, tt := range testCases {
+		return []byte(resp)
-		t.Run(tt.name, func(t *testing.T) {
-			// Prepare a mock Vultr server.
-			mockServer := newMockVultrServer(func() ([]byte, error) {
-				var e error
-				if tt.apiError {
-					e = errors.New("mock error")
-				}
-				return []byte(tt.apiResponse), e
 	})
 
 	// Prepare a discovery HTTP client who calls mock server.
-			client, _ := discoveryutils.NewClient(mockServer.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
+	client, err := discoveryutils.NewClient(s.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
+	if err != nil {
+		t.Fatalf("unexpected error wen creating http client: %s", err)
+	}
 	cfg := &apiConfig{
 		c: client,
 	}
 
 	// execute `getInstances`
 	instances, err := getInstances(cfg)
-
+	if err != nil {
-			// evaluate test result
+		t.Fatalf("unexpected error in getInstances(): %s", err)
-			if tt.expectError != (err != nil) {
-				t.Errorf("getInstances expect (error != nil): %t, got error: %v", tt.expectError, err)
 	}
 
-			if !reflect.DeepEqual(tt.expectResponse, instances) {
+	expectedInstances := []Instance{
-				t.Errorf("getInstances expect result: %v, got: %v", tt.expectResponse, instances)
+		{
+			ID:               "fake-id-07f7-4b68-88ac-fake-id",
+			OS:               "Ubuntu 22.04 x64",
+			RAM:              1024,
+			Disk:             25,
+			MainIP:           "64.176.84.27",
+			VCPUCount:        1,
+			Region:           "sgp",
+			Plan:             "vc2-1c-1gb",
+			AllowedBandwidth: 1,
+			ServerStatus:     "installingbooting",
+			V6MainIP:         "2002:18f0:4100:263a:5300:07ff:fdd7:691c",
+			Label:            "vultr-sd",
+			InternalIP:       "",
+			Hostname:         "vultr-sd",
+			Tags:             []string{},
+			OSID:             1743,
+			Features:         []string{"ipv6"},
+		},
 	}
+	if !reflect.DeepEqual(instances, expectedInstances) {
+		t.Fatalf("unexpected result\ngot\n%#v\nwant\n%#v", instances, expectedInstances)
+	}
+}
+
+func TestGetInstances_Failure(t *testing.T) {
+	s := newMockVultrServer(func() []byte {
+		return []byte("some error")
 	})
+
+	// Prepare a discovery HTTP client who calls mock server.
+	client, err := discoveryutils.NewClient(s.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
+	if err != nil {
+		t.Fatalf("unexpected error wen creating http client: %s", err)
+	}
+	cfg := &apiConfig{
+		c: client,
+	}
+
+	// execute `getInstances`
+	if _, err := getInstances(cfg); err == nil {
+		t.Fatalf("expecting non-nil error from getInstances()")
 	}
 }
 
-// TestGetInstancesPaging run test cases for response with multiple pages.
+func TestGetInstances_Paging(t *testing.T) {
-func TestGetInstancesPaging(t *testing.T) {
 	// Prepare a mock Vultr server.
 	// requestCount control the mock response for different page request.
 	requestCount := 0
-
+	s := newMockVultrServer(func() []byte {
-	mockServer := newMockVultrServer(func() ([]byte, error) {
 		// for the 1st request, response with `next` cursor
 		if requestCount == 0 {
 			requestCount++
-			return []byte(mockListInstanceSuccessPage0Resp), nil
+			return []byte(mockListInstanceSuccessPage0Resp)
 		}
-		// for the 2nd+ request, response with `prev` cursor and empty `next`.
+		// for the 2nd+ request, response with empty `next`.
-		return []byte(mockListInstanceSuccessPage1Resp), nil
+		return []byte(mockListInstanceSuccessPage1Resp)
 	})
 
 	// Prepare a discovery HTTP client who calls mock server.
-	client, _ := discoveryutils.NewClient(mockServer.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
+	client, err := discoveryutils.NewClient(s.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
+	if err != nil {
+		t.Fatalf("unexpected error wen creating http client: %s", err)
+	}
 	cfg := &apiConfig{
 		c: client,
 	}
 
 	// execute `getInstances`
 	instances, err := getInstances(cfg)
-
-	// evaluate test result
 	if err != nil {
-		t.Errorf("getInstances expect error: %v, got error: %v", nil, err)
+		t.Fatalf("unexpected error in getInstances(): %s", err)
 	}
-
 	if !reflect.DeepEqual(expectSuccessPagingInstances, instances) {
-		t.Errorf("getInstances expect result: %v, got: %v", expectSuccessPagingInstances, instances)
+		t.Fatalf("unexpected getInstances() result\ngot\n%#v\nwant\n%#v", instances, expectSuccessPagingInstances)
 	}
 }
 
 // ------------ Test dataset ------------
-var (
-	// mockListInstanceSuccessResp is crawled from a real-world response of ListInstance API
-	// with sensitive info removed/modified.
-	mockListInstanceSuccessResp = `{
-	"instances": [{
-		"id": "fake-id-07f7-4b68-88ac-fake-id",
-		"os": "Ubuntu 22.04 x64",
-		"ram": 1024,
-		"disk": 25,
-		"main_ip": "64.176.84.27",
-		"vcpu_count": 1,
-		"region": "sgp",
-		"plan": "vc2-1c-1gb",
-		"date_created": "2024-04-05T05:41:28+00:00",
-		"status": "active",
-		"allowed_bandwidth": 1,
-		"netmask_v4": "255.255.254.0",
-		"gateway_v4": "64.176.63.2",
-		"power_status": "running",
-		"server_status": "installingbooting",
-		"v6_network": "2002:18f0:4100:263a::",
-		"v6_main_ip": "2002:18f0:4100:263a:5300:07ff:fdd7:691c",
-		"v6_network_size": 64,
-		"label": "vultr-sd",
-		"internal_ip": "",
-		"kvm": "https:\/\/my.vultr.com\/subs\/vps\/novnc\/api.php?data=secret_data_string",
-		"hostname": "vultr-sd",
-		"tag": "",
-		"tags": [],
-		"os_id": 1743,
-		"app_id": 0,
-		"image_id": "",
-		"firewall_group_id": "",
-		"features": ["ipv6"],
-		"user_scheme": "root"
-	}],
-	"meta": {
-		"total": 1,
-		"links": {
-			"next": "",
-			"prev": ""
-		}
-	}
-}`
-	expectSuccessInstances = []Instance{
-		{
-			ID:               "fake-id-07f7-4b68-88ac-fake-id",
-			Os:               "Ubuntu 22.04 x64",
-			RAM:              1024,
-			Disk:             25,
-			MainIP:           "64.176.84.27",
-			VCPUCount:        1,
-			Region:           "sgp",
-			Plan:             "vc2-1c-1gb",
-			AllowedBandwidth: 1,
-			ServerStatus:     "installingbooting",
-			V6MainIP:         "2002:18f0:4100:263a:5300:07ff:fdd7:691c",
-			Label:            "vultr-sd",
-			InternalIP:       "",
-			Hostname:         "vultr-sd",
-			Tags:             []string{},
-			OsID:             1743,
-			Features:         []string{"ipv6"},
-		},
-	}
-)
-
-var (
-	mockListInstanceFailedResp = `{"error":"Invalid API token.","status":401}`
-)
-
 var (
 	// mockListInstanceSuccessPage0Resp contains `next` cursor
 	mockListInstanceSuccessPage0Resp = `{
 	"instances": [{
-		"id": "fake-id-07f7-4b68-88ac-fake-id",
+		"id": "1-fake-id-07f7-4b68-88ac-fake-id",
 		"os": "Ubuntu 22.04 x64",
 		"ram": 1024,
 		"disk": 25,
@@ -209,17 +179,15 @@ var (
 		"user_scheme": "root"
 	}],
 	"meta": {
-		"total": 2,
 		"links": {
-			"next": "fake-cursor-string",
+			"next": "fake-cursor-string"
-			"prev": ""
 		}
 	}
 }`
-	// mockListInstanceSuccessPage1Resp contains `prev` cursor
+	// mockListInstanceSuccessPage1Resp contains empty 'next' cursor
 	mockListInstanceSuccessPage1Resp = `{
 	"instances": [{
-		"id": "fake-id-07f7-4b68-88ac-fake-id",
+		"id": "2-fake-id-07f7-4b68-88ac-fake-id",
 		"os": "Ubuntu 22.04 x64",
 		"ram": 1024,
 		"disk": 25,
@@ -251,17 +219,15 @@ var (
 		"user_scheme": "root"
 	}],
 	"meta": {
-		"total": 2,
 		"links": {
-			"next": "",
+			"next": ""
-			"prev": "fake-cursor-string"
 		}
 	}
 }`
 	expectSuccessPagingInstances = []Instance{
 		{
-			ID:               "fake-id-07f7-4b68-88ac-fake-id",
+			ID:               "1-fake-id-07f7-4b68-88ac-fake-id",
-			Os:               "Ubuntu 22.04 x64",
+			OS:               "Ubuntu 22.04 x64",
 			RAM:              1024,
 			Disk:             25,
 			MainIP:           "64.176.84.27",
@@ -275,12 +241,12 @@ var (
 			InternalIP:       "",
 			Hostname:         "vultr-sd",
 			Tags:             []string{},
-			OsID:             1743,
+			OSID:             1743,
 			Features:         []string{"ipv6"},
 		},
 		{
-			ID:               "fake-id-07f7-4b68-88ac-fake-id",
+			ID:               "2-fake-id-07f7-4b68-88ac-fake-id",
-			Os:               "Ubuntu 22.04 x64",
+			OS:               "Ubuntu 22.04 x64",
 			RAM:              1024,
 			Disk:             25,
 			MainIP:           "64.176.84.27",
@@ -294,7 +260,7 @@ var (
 			InternalIP:       "",
 			Hostname:         "vultr-sd",
 			Tags:             []string{},
-			OsID:             1743,
+			OSID:             1743,
 			Features:         []string{"ipv6"},
 		},
 	}

lib/promscrape/discovery/vultr/mock_server_test.go

@@ -1,40 +1,23 @@
 package vultr
 
 import (
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 )
 
-func newMockVultrServer(jsonResponse func() ([]byte, error)) *vultrServer {
+type mockVultrServer struct {
-	rw := &vultrServer{}
-	rw.Server = httptest.NewServer(http.HandlerFunc(rw.handler))
-	rw.jsonResponse = jsonResponse
-	return rw
-}
-
-type vultrServer struct {
 	*httptest.Server
-	jsonResponse func() ([]byte, error)
+	responseFunc func() []byte
 }
 
-func (rw *vultrServer) err(w http.ResponseWriter, err error) {
+func newMockVultrServer(responseFunc func() []byte) *mockVultrServer {
-	w.WriteHeader(http.StatusBadRequest)
+	var s mockVultrServer
-	w.Write([]byte(err.Error()))
+	s.responseFunc = responseFunc
+	s.Server = httptest.NewServer(http.HandlerFunc(s.handler))
+	return &s
 }
 
-func (rw *vultrServer) handler(w http.ResponseWriter, r *http.Request) {
+func (s *mockVultrServer) handler(w http.ResponseWriter, _ *http.Request) {
-	if r.Method != http.MethodGet {
+	data := s.responseFunc()
-		rw.err(w, fmt.Errorf("bad method %q", r.Method))
+	w.Write(data)
-		return
-	}
-
-	resp, err := rw.jsonResponse()
-	if err != nil {
-		rw.err(w, err)
-		return
-	}
-
-	w.Write(resp)
-	w.WriteHeader(http.StatusOK)
 }

lib/promscrape/discovery/vultr/vultr.go

@@ -13,16 +13,13 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/proxy"
 )
 
-const (
+// SDCheckInterval defines interval for Vultr targets refresh.
-	separator = ","
-)
-
-// SDCheckInterval defines interval for docker targets refresh.
 var SDCheckInterval = flag.Duration("promscrape.vultrSDCheckInterval", 30*time.Second, "Interval for checking for changes in Vultr. "+
 	"This works only if vultr_sd_configs is configured in '-promscrape.config' file. "+
 	"See https://docs.victoriametrics.com/sd_configs.html#vultr_sd_configs for details")
 
 // SDConfig represents service discovery config for Vultr.
+//
 // See: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#vultr_sd_config
 // Additional query params are supported, while Prometheus only supports `Port` and HTTP auth.
 type SDConfig struct {
@@ -62,7 +59,7 @@ func (sdc *SDConfig) GetLabels(baseDir string) ([]*promutils.Labels, error) {
 
 // MustStop stops further usage for sdc.
 func (sdc *SDConfig) MustStop() {
-	configMap.Delete(sdc)
+	_ = configMap.Delete(sdc)
 }
 
 // getInstanceLabels returns labels for vultr instances obtained from the given cfg
@@ -72,34 +69,36 @@ func getInstanceLabels(instances []Instance, port int) []*promutils.Labels {
 	for _, instance := range instances {
 		m := promutils.NewLabels(18)
 		m.Add("__address__", discoveryutils.JoinHostPort(instance.MainIP, port))
-		m.Add("__meta_vultr_instance_id", instance.ID)
-		m.Add("__meta_vultr_instance_label", instance.Label)
-		m.Add("__meta_vultr_instance_os", instance.Os)
-		m.Add("__meta_vultr_instance_os_id", strconv.Itoa(instance.OsID))
-		m.Add("__meta_vultr_instance_region", instance.Region)
-		m.Add("__meta_vultr_instance_plan", instance.Plan)
-		m.Add("__meta_vultr_instance_main_ip", instance.MainIP)
-		m.Add("__meta_vultr_instance_internal_ip", instance.InternalIP)
-		m.Add("__meta_vultr_instance_main_ipv6", instance.V6MainIP)
-		m.Add("__meta_vultr_instance_hostname", instance.Hostname)
-		m.Add("__meta_vultr_instance_server_status", instance.ServerStatus)
-		m.Add("__meta_vultr_instance_vcpu_count", strconv.Itoa(instance.VCPUCount))
-		m.Add("__meta_vultr_instance_ram_mb", strconv.Itoa(instance.RAM))
 		m.Add("__meta_vultr_instance_allowed_bandwidth_gb", strconv.Itoa(instance.AllowedBandwidth))
 		m.Add("__meta_vultr_instance_disk_gb", strconv.Itoa(instance.Disk))
+		m.Add("__meta_vultr_instance_hostname", instance.Hostname)
+		m.Add("__meta_vultr_instance_id", instance.ID)
+		m.Add("__meta_vultr_instance_internal_ip", instance.InternalIP)
+		m.Add("__meta_vultr_instance_label", instance.Label)
+		m.Add("__meta_vultr_instance_main_ip", instance.MainIP)
+		m.Add("__meta_vultr_instance_main_ipv6", instance.V6MainIP)
+		m.Add("__meta_vultr_instance_os", instance.OS)
+		m.Add("__meta_vultr_instance_os_id", strconv.Itoa(instance.OSID))
+		m.Add("__meta_vultr_instance_plan", instance.Plan)
+		m.Add("__meta_vultr_instance_region", instance.Region)
+		m.Add("__meta_vultr_instance_ram_mb", strconv.Itoa(instance.RAM))
+		m.Add("__meta_vultr_instance_server_status", instance.ServerStatus)
+		m.Add("__meta_vultr_instance_vcpu_count", strconv.Itoa(instance.VCPUCount))
 
-		// We surround the separated list with the separator as well. This way regular expressions
-		// in relabeling rules don't have to consider feature positions.
 		if len(instance.Features) > 0 {
-			features := separator + strings.Join(instance.Features, separator) + separator
+			m.Add("__meta_vultr_instance_features", joinStrings(instance.Features))
-			m.Add("__meta_vultr_instance_features", features)
 		}
 
 		if len(instance.Tags) > 0 {
-			tags := separator + strings.Join(instance.Tags, separator) + separator
+			m.Add("__meta_vultr_instance_tags", joinStrings(instance.Tags))
-			m.Add("__meta_vultr_instance_tags", tags)
 		}
+
 		ms = append(ms, m)
 	}
+
 	return ms
 }
+
+func joinStrings(a []string) string {
+	return "," + strings.Join(a, ",") + ","
+}

lib/promscrape/discovery/vultr/vultr_test.go

@@ -11,7 +11,7 @@ func TestGetInstanceLabels(t *testing.T) {
 	input := []Instance{
 		{
 			ID:               "fake-id-07f7-4b68-88ac-fake-id",
-			Os:               "Ubuntu 22.04 x64",
+			OS:               "Ubuntu 22.04 x64",
 			RAM:              1024,
 			Disk:             25,
 			MainIP:           "64.176.84.27",
@@ -25,12 +25,12 @@ func TestGetInstanceLabels(t *testing.T) {
 			InternalIP:       "",
 			Hostname:         "vultr-sd",
 			Tags:             []string{"mock tags"},
-			OsID:             1743,
+			OSID:             1743,
 			Features:         []string{"ipv6"},
 		},
 		{
 			ID:               "fake-id-07f7-4b68-88ac-fake-id",
-			Os:               "Ubuntu 22.04 x64",
+			OS:               "Ubuntu 22.04 x64",
 			RAM:              1024,
 			Disk:             25,
 			MainIP:           "64.176.84.27",
@@ -44,7 +44,7 @@ func TestGetInstanceLabels(t *testing.T) {
 			InternalIP:       "",
 			Hostname:         "vultr-sd",
 			Tags:             []string{"mock tags"},
-			OsID:             1743,
+			OSID:             1743,
 			Features:         []string{"ipv6"},
 		},
 	}

lib/promscrape/discovery/yandexcloud/yandexcloud.go

@@ -38,6 +38,11 @@ func (sdc *SDConfig) GetLabels(baseDir string) ([]*promutils.Labels, error) {
 	}
 }
 
+// MustStop stops further usage for sdc.
+func (sdc *SDConfig) MustStop() {
+	_ = configMap.Delete(sdc)
+}
+
 func (cfg *apiConfig) getInstances(folderID string) ([]instance, error) {
 	instancesURL := cfg.serviceEndpoints["compute"] + "/compute/v1/instances"
 	instancesURL += "?folderId=" + url.QueryEscape(folderID)