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.

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

Fix for azure retrieve resource by ids #21711

Merged
merged 37 commits into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
938e66c
mofidy doc
narph Jul 23, 2020
4daef08
Merge branch 'master' of github.com:elastic/beats
narph Aug 3, 2020
b613178
Merge branch 'master' of github.com:elastic/beats
narph Aug 4, 2020
05364cf
Merge branch 'master' of github.com:elastic/beats
narph Aug 4, 2020
f147c4d
Merge branch 'master' of github.com:elastic/beats
narph Aug 4, 2020
4574718
Merge branch 'master' of github.com:elastic/beats
narph Aug 17, 2020
1e43077
Merge branch 'master' of github.com:elastic/beats
narph Aug 19, 2020
807cf06
Merge branch 'master' of github.com:elastic/beats
narph Aug 24, 2020
2096668
Merge branch 'master' of github.com:elastic/beats
narph Aug 27, 2020
da8ac1f
Merge branch 'master' of github.com:elastic/beats
narph Aug 27, 2020
c2d8930
Merge branch 'master' of github.com:elastic/beats
narph Aug 27, 2020
7bd9e73
Merge branch 'master' of github.com:elastic/beats
narph Aug 31, 2020
6e89a84
Merge branch 'master' of github.com:elastic/beats
narph Aug 31, 2020
bdf21e9
Merge branch 'master' of github.com:elastic/beats
narph Sep 2, 2020
7833687
Merge branch 'master' of github.com:elastic/beats
narph Sep 3, 2020
bbf6178
Merge branch 'master' of github.com:elastic/beats
narph Sep 4, 2020
4ba8817
Merge branch 'master' of github.com:elastic/beats
narph Sep 7, 2020
0cba5dc
Merge branch 'master' of github.com:elastic/beats
narph Sep 8, 2020
b2625ca
Merge branch 'master' of github.com:elastic/beats
narph Sep 8, 2020
5100e6a
Merge branch 'master' of github.com:elastic/beats
narph Sep 9, 2020
a302d31
Merge branch 'master' of github.com:elastic/beats
narph Sep 14, 2020
631d667
Merge branch 'master' of github.com:elastic/beats
narph Sep 15, 2020
35072a5
Merge branch 'master' of github.com:elastic/beats
narph Sep 17, 2020
4b2f87a
Merge branch 'master' of github.com:elastic/beats
narph Sep 22, 2020
f26b533
Merge branch 'master' of github.com:elastic/beats
narph Sep 28, 2020
c61620d
Merge branch 'master' of github.com:elastic/beats
narph Sep 28, 2020
43f90c4
Merge branch 'master' of github.com:elastic/beats
narph Sep 30, 2020
2d28f07
Merge branch 'master' of github.com:elastic/beats
narph Oct 1, 2020
0a42bbe
Merge branch 'master' of github.com:elastic/beats
narph Oct 2, 2020
0bedffe
Merge branch 'master' of github.com:elastic/beats
narph Oct 5, 2020
2eb7142
Merge branch 'master' of github.com:elastic/beats
narph Oct 5, 2020
871abe6
start work
narph Oct 6, 2020
358fa00
work
narph Oct 12, 2020
0d9ce59
changelog
narph Oct 12, 2020
0919186
fix test
narph Oct 12, 2020
f9e4522
Merge branch 'master' of github.com:elastic/beats
narph Oct 12, 2020
f0aa34b
Merge branch 'master' into az-res
narph Oct 12, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- 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]

*Packetbeat*
Expand Down
6 changes: 3 additions & 3 deletions x-pack/metricbeat/module/azure/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/metricbeat/module/azure/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions x-pack/metricbeat/module/azure/mock_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 19 additions & 7 deletions x-pack/metricbeat/module/azure/monitor_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Nit. Split this line 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM, would it be possible to add a test case that would reproduce the original problem?

We will need integration tests in this case, this behavior is based on API supportability, I am hoping to have those in the near future.

// 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))
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x-pack/metricbeat/module/azure/service_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down