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

Move azure module to GA #16754

Merged
merged 10 commits into from
Mar 4, 2020
Merged

Move azure module to GA #16754

merged 10 commits into from
Mar 4, 2020

Conversation

narph
Copy link
Contributor

@narph narph commented Mar 3, 2020

Should fix some of the points in #16024

  • moved azure.resource.name to cloud.instance.name
  • added cloud.instance.id and cloud.machine.type (mapping the resource size for compute_vm and compute_vm_scaleset)
  • added more integration tests and generated the data.json files
  • modified dashboards based on the new fields

@narph narph self-assigned this Mar 3, 2020
@narph narph added Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team labels Mar 3, 2020
@narph narph requested a review from exekias March 3, 2020 18:12
@narph narph marked this pull request as ready for review March 3, 2020 18:12
@narph narph requested a review from a team as a code owner March 3, 2020 18:12
@narph narph added [zube]: In Review needs_backport PR is waiting to be backported to other branches. labels Mar 3, 2020
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

just some minor comments

Comment on lines -20 to +23
ClientID string `config:"client_id" validate:"required"`
ClientId string `config:"client_id" validate:"required"`
ClientSecret string `config:"client_secret" validate:"required"`
TenantID string `config:"tenant_id" validate:"required"`
SubscriptionID string `config:"subscription_id" validate:"required"`
TenantId string `config:"tenant_id" validate:"required"`
SubscriptionId string `config:"subscription_id" validate:"required"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these changes? I don't think there is anything wrong with ID

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was a mix of Id and ID in the code so I went with most used for consistency

"tenant_id": tenantId,
"subscription_id": subscriptionId,
func TestData(t *testing.T) {
config, err := test.GetConfig("container_instance")
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

@narph
Copy link
Contributor Author

narph commented Mar 4, 2020

cc @sorantis, just a heads up on the changes, dashboards have been updated as well

@narph narph merged commit 1894441 into elastic:master Mar 4, 2020
@narph narph deleted the azure-module-ga branch March 4, 2020 11:28
narph added a commit to narph/beats that referenced this pull request Mar 4, 2020
* work on GA

* mage fmt

* fix

* work on tests

* fix

* work on tests

* generate from test

(cherry picked from commit 1894441)
narph added a commit that referenced this pull request Mar 4, 2020
* work on GA

* mage fmt

* fix

* work on tests

* fix

* work on tests

* generate from test

(cherry picked from commit 1894441)
@narph narph added the test-plan Add this PR to be manual test plan label Mar 27, 2020
@narph narph added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 27, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants