-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add support for different Azure Cloud environments in the metricbeat azure module #21044
Conversation
Pinging @elastic/integrations-platforms (Team:Platforms) |
💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left one minor, let me know you think :)!
func NewService(clientId string, clientSecret string, tenantId string, subscriptionId string) (*MonitorService, error) { | ||
clientConfig := auth.NewClientCredentialsConfig(clientId, clientSecret, tenantId) | ||
func NewService(config Config) (*MonitorService, error) { | ||
clientConfig := auth.NewClientCredentialsConfig(config.ClientId, config.ClientSecret, config.TenantId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this function be called with just config
as input to avoid this unfolding and the extra assignments bellow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean the auth.NewClientCredentialsConfig
, this is part of the azure sdk so we cannot pass the config we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I see 👍🏼
metricsDefinitionClient := insights.NewMetricDefinitionsClient(subscriptionId) | ||
resourceClient := resources.NewClient(subscriptionId) | ||
metricNamespaceClient := insights.NewMetricNamespacesClient(subscriptionId) | ||
metricsClient := insights.NewMetricsClientWithBaseURI(config.ResourceManagerEndpoint, config.SubscriptionId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, could this one take only one argument config
instead of multiple values of config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same , this is part of the sdk, and to instantiate the metrics client is asking for the separate values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
func NewService(clientId string, clientSecret string, tenantId string, subscriptionId string) (*MonitorService, error) { | ||
clientConfig := auth.NewClientCredentialsConfig(clientId, clientSecret, tenantId) | ||
func NewService(config Config) (*MonitorService, error) { | ||
clientConfig := auth.NewClientCredentialsConfig(config.ClientId, config.ClientSecret, config.TenantId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I see 👍🏼
metricsDefinitionClient := insights.NewMetricDefinitionsClient(subscriptionId) | ||
resourceClient := resources.NewClient(subscriptionId) | ||
metricNamespaceClient := insights.NewMetricNamespacesClient(subscriptionId) | ||
metricsClient := insights.NewMetricsClientWithBaseURI(config.ResourceManagerEndpoint, config.SubscriptionId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
…azure module (elastic#21044) * mofidy doc * work * changelog * fix url * work * fmt update * err (cherry picked from commit 554d564)
…azure module (elastic#21044) * mofidy doc * work * changelog * fix url * work * fmt update * err (cherry picked from commit 554d564)
* upstream/master: (93 commits) Update commands used in the quick start (elastic#22248) Add interval documentation to `monitor` metricset (elastic#22152) [CI] enable x-pack/packetbeat in the CI (elastic#22252) Fix awscloudwatch input documentation (elastic#22247) Add support for different Azure Cloud environments in the metricbeat azure module (elastic#21044) [CI] support windows-2008-r2 (elastic#19791) protect against accessing undefined variables in sysmon module (elastic#22236) [CI] archive only if failed steps (elastic#22220) Add pe fields to Sysmon module (elastic#22217) [CI][flaky] Support 7.x branches and PRs (elastic#22197) Perfmon - Fix regular expressions to comply to multiple parentheses in instance name and object (elastic#22146) ci: improve linting speed (elastic#22103) Move cloudfoundry tags with metadata to common metadata fields (elastic#22150) [Docs] Update custom beat docs (elastic#22194) [Ingest Manager] Agent fix snapshot download for upgrade (elastic#22175) Update shared-autodiscover.asciidoc (elastic#21827) [DOCS] Warn about compression and Azure Event Hub for Kafka (elastic#21578) [CI][flaky] reporting for PRs in GitHub (elastic#21853) [Packetbeat] Create x-pack magefile (elastic#21979) [Elastic Agent] Fix deb/rpm installation (elastic#22153) ...
* upstream/master: (93 commits) Update commands used in the quick start (elastic#22248) Add interval documentation to `monitor` metricset (elastic#22152) [CI] enable x-pack/packetbeat in the CI (elastic#22252) Fix awscloudwatch input documentation (elastic#22247) Add support for different Azure Cloud environments in the metricbeat azure module (elastic#21044) [CI] support windows-2008-r2 (elastic#19791) protect against accessing undefined variables in sysmon module (elastic#22236) [CI] archive only if failed steps (elastic#22220) Add pe fields to Sysmon module (elastic#22217) [CI][flaky] Support 7.x branches and PRs (elastic#22197) Perfmon - Fix regular expressions to comply to multiple parentheses in instance name and object (elastic#22146) ci: improve linting speed (elastic#22103) Move cloudfoundry tags with metadata to common metadata fields (elastic#22150) [Docs] Update custom beat docs (elastic#22194) [Ingest Manager] Agent fix snapshot download for upgrade (elastic#22175) Update shared-autodiscover.asciidoc (elastic#21827) [DOCS] Warn about compression and Azure Event Hub for Kafka (elastic#21578) [CI][flaky] reporting for PRs in GitHub (elastic#21853) [Packetbeat] Create x-pack magefile (elastic#21979) [Elastic Agent] Fix deb/rpm installation (elastic#22153) ...
What does this PR do?
Adds support for different Azure Cloud environments in the metricbeat azure module
Adds 2 config options (similar to the filebeat azure module):
resource_manager_endpoint
::string
Optional, by default the azure public environment is being used, to override, users can provide a specific resource manager endpoint in order to use a different azure environment.
Ex:
https://management.chinacloudapi.cn for azure ChinaCloud
https://management.microsoftazure.de for azure GermanCloud
https://management.azure.com for azure PublicCloud
https://management.usgovcloudapi.net for azure USGovernmentCloud
active_directory_endpoint
::string
Optional, by default the associated active directory endpoint to the resource manager endpoint is being used, to override, users can provide a specific active directory endpoint in order to use a different azure environment.
Ex:
https://login.microsoftonline.com for azure ChinaCloud
https://login.microsoftonline.us for azure GermanCloud
https://login.chinacloudapi.cn for azure PublicCloud
https://login.microsoftonline.de for azure USGovernmentCloud
Why is it important?
Adds support for different Azure Cloud environments in the metricbeat azure module
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues