From 02da4c8bb3160ca172db130e4ff6599338261813 Mon Sep 17 00:00:00 2001 From: Mariana Dima Date: Tue, 13 Oct 2020 16:01:19 +0200 Subject: [PATCH 1/2] Fix for azure retrieve resource by ids (#21711) * mofidy doc * start work * work * changelog * fix test (cherry picked from commit 056f0e07b233d3a438b1d21b5182cb2ea49638d2) --- CHANGELOG.next.asciidoc | 2 ++ x-pack/metricbeat/module/azure/client.go | 6 ++--- x-pack/metricbeat/module/azure/client_test.go | 2 +- .../metricbeat/module/azure/mock_service.go | 4 +-- .../module/azure/monitor_service.go | 26 ++++++++++++++----- .../module/azure/service_interface.go | 2 +- 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index a2c9ff507f5..5dda6504b78 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -417,6 +417,8 @@ field. You can revert this change by configuring tags for the module and omittin - Fix timestamp handling in remote_write. {pull}21166[21166] - Fix remote_write flaky test. {pull}21173[21173] - Visualization title fixes in aws, azure and googlecloud compute dashboards. {pull}21098[21098] +- Add a switch to the driver definition on SQL module to use pretty names {pull}17378[17378] +- Fix retrieving resources by ID for the azure module. {pull}21711[21711] {issue}21707[21707] - Use timestamp from CloudWatch API when creating events. {pull}21498[21498] - Report the correct windows events for system/filesystem {pull}21758[21758] - Fix panic in kubernetes autodiscover related to keystores {issue}21843[21843] {pull}21880[21880] diff --git a/x-pack/metricbeat/module/azure/client.go b/x-pack/metricbeat/module/azure/client.go index e488fab98b6..dd48f962b59 100644 --- a/x-pack/metricbeat/module/azure/client.go +++ b/x-pack/metricbeat/module/azure/client.go @@ -65,14 +65,14 @@ func (client *Client) InitResources(fn mapResourceMetrics) error { err = errors.Wrap(err, "failed to retrieve resources") return err } - if len(resourceList.Values()) == 0 { + if len(resourceList) == 0 { err = errors.Errorf("failed to retrieve resources: No resources returned using the configuration options resource ID %s, resource group %s, resource type %s, resource query %s", resource.Id, resource.Group, resource.Type, resource.Query) client.Log.Error(err) continue } //map resources to the client - for _, resource := range resourceList.Values() { + for _, resource := range resourceList { if !containsResource(*resource.ID, client.Resources) { client.Resources = append(client.Resources, Resource{ Id: *resource.ID, @@ -84,7 +84,7 @@ func (client *Client) InitResources(fn mapResourceMetrics) error { Subscription: client.Config.SubscriptionId}) } } - resourceMetrics, err := fn(client, resourceList.Values(), resource) + resourceMetrics, err := fn(client, resourceList, resource) if err != nil { return err } diff --git a/x-pack/metricbeat/module/azure/client_test.go b/x-pack/metricbeat/module/azure/client_test.go index 47b88f99cce..6b0df97a370 100644 --- a/x-pack/metricbeat/module/azure/client_test.go +++ b/x-pack/metricbeat/module/azure/client_test.go @@ -50,7 +50,7 @@ func TestInitResources(t *testing.T) { client := NewMockClient() client.Config = resourceQueryConfig m := &MockService{} - m.On("GetResourceDefinitions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(resources.ListResultPage{}, errors.New("invalid resource query")) + m.On("GetResourceDefinitions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]resources.GenericResource{}, errors.New("invalid resource query")) client.AzureMonitorService = m mr := MockReporterV2{} mr.On("Error", mock.Anything).Return(true) diff --git a/x-pack/metricbeat/module/azure/mock_service.go b/x-pack/metricbeat/module/azure/mock_service.go index f6f54c300e0..601f1de5b45 100644 --- a/x-pack/metricbeat/module/azure/mock_service.go +++ b/x-pack/metricbeat/module/azure/mock_service.go @@ -24,9 +24,9 @@ func (client *MockService) GetResourceDefinitionById(id string) (resources.Gener } // GetResourceDefinitions is a mock function for the azure service -func (client *MockService) GetResourceDefinitions(id []string, group []string, rType string, query string) (resources.ListResultPage, error) { +func (client *MockService) GetResourceDefinitions(id []string, group []string, rType string, query string) ([]resources.GenericResource, error) { args := client.Called(id, group, rType, query) - return args.Get(0).(resources.ListResultPage), args.Error(1) + return args.Get(0).([]resources.GenericResource), args.Error(1) } // GetMetricDefinitions is a mock function for the azure service diff --git a/x-pack/metricbeat/module/azure/monitor_service.go b/x-pack/metricbeat/module/azure/monitor_service.go index 053da3db05b..c3ed4e2fa43 100644 --- a/x-pack/metricbeat/module/azure/monitor_service.go +++ b/x-pack/metricbeat/module/azure/monitor_service.go @@ -55,16 +55,24 @@ func NewService(clientId string, clientSecret string, tenantId string, subscript } // GetResourceDefinitions will retrieve the azure resources based on the options entered -func (service MonitorService) GetResourceDefinitions(id []string, group []string, rType string, query string) (resources.ListResultPage, error) { +func (service MonitorService) GetResourceDefinitions(id []string, group []string, rType string, query string) ([]resources.GenericResource, error) { var resourceQuery string + var resourceList []resources.GenericResource if len(id) > 0 { - var filterList []string - // listing resourceID conditions does not seem to work with the API but querying by name or resource types will work + // listing multiple resourceId conditions does not seem to work with the API, extracting the name and resource type does not work as the position of the `resourceType` can move if a parent resource is involved, filtering by resource name and resource group (if extracted) is also not possible as + // different types of resources can contain the same name. for _, id := range id { - filterList = append(filterList, fmt.Sprintf("name eq '%s'", getResourceNameFromId(id))) + resource, err := service.resourceClient.List(service.context, fmt.Sprintf("resourceId eq '%s'", id), "", nil) + if err != nil { + return nil, err + } + if len(resource.Values()) > 0 { + resourceList = append(resourceList, resource.Values()...) + } } - resourceQuery = fmt.Sprintf("(%s) AND resourceType eq '%s'", strings.Join(filterList, " OR "), getResourceTypeFromId(id[0])) - } else if len(group) > 0 { + return resourceList, nil + } + if len(group) > 0 { var filterList []string for _, gr := range group { filterList = append(filterList, fmt.Sprintf("resourceGroup eq '%s'", gr)) @@ -76,7 +84,11 @@ func (service MonitorService) GetResourceDefinitions(id []string, group []string } else if query != "" { resourceQuery = query } - return service.resourceClient.List(service.context, resourceQuery, "", nil) + result, err := service.resourceClient.List(service.context, resourceQuery, "", nil) + if err == nil { + resourceList = result.Values() + } + return resourceList, err } // GetResourceDefinitionById will retrieve the azure resource based on the resource Id diff --git a/x-pack/metricbeat/module/azure/service_interface.go b/x-pack/metricbeat/module/azure/service_interface.go index e8985a7eedd..30430d03100 100644 --- a/x-pack/metricbeat/module/azure/service_interface.go +++ b/x-pack/metricbeat/module/azure/service_interface.go @@ -12,7 +12,7 @@ import ( // Service interface for the azure monitor service and mock for testing type Service interface { GetResourceDefinitionById(id string) (resources.GenericResource, error) - GetResourceDefinitions(id []string, group []string, rType string, query string) (resources.ListResultPage, error) + GetResourceDefinitions(id []string, group []string, rType string, query string) ([]resources.GenericResource, error) GetMetricDefinitions(resourceId string, namespace string) (insights.MetricDefinitionCollection, error) GetMetricNamespaces(resourceId string) (insights.MetricNamespaceCollection, error) GetMetricValues(resourceId string, namespace string, timegrain string, timespan string, metricNames []string, aggregations string, filter string) ([]insights.Metric, string, error) From be861d1ad2aff06e76387cbce9f510509beee15d Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 20 Oct 2020 09:50:43 +0200 Subject: [PATCH 2/2] fix changelog --- CHANGELOG.next.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 5dda6504b78..d18410e1117 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -417,7 +417,6 @@ field. You can revert this change by configuring tags for the module and omittin - Fix timestamp handling in remote_write. {pull}21166[21166] - Fix remote_write flaky test. {pull}21173[21173] - Visualization title fixes in aws, azure and googlecloud compute dashboards. {pull}21098[21098] -- Add a switch to the driver definition on SQL module to use pretty names {pull}17378[17378] - Fix retrieving resources by ID for the azure module. {pull}21711[21711] {issue}21707[21707] - Use timestamp from CloudWatch API when creating events. {pull}21498[21498] - Report the correct windows events for system/filesystem {pull}21758[21758]