-
Notifications
You must be signed in to change notification settings - Fork 4k
Update docs to indicate [Get/Disable/Enable]-AzHDInsightMonitoring cmdlets are 'Classic' #15533
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
Conversation
Updated synopsis & description to differentiate from the newer Disable-AzHDInsightAzureMonitor cmdlet.
Updated synopsis & description to differentiate from the newer Enable-AzHDInsightAzureMonitor cmdlet.
Updated synopsis & description to differentiate from the newer Get-AzHDInsightAzureMonitor cmdlet.
Thank you for your contribution proazr! We will review the pull request and get back to you soon. |
@proazr thanks for your contribution! Let me route this to the correct team... Hi @idear1203 @aim-for-better would you be able to help review this? Thanks! |
Hi @isra-fel is changing the description a breaking change or not? |
Not a breaking change. It's fine. |
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.
Hi @proazr , thanks for your contribution.
Could you please also update the related description in the src/HDInsight/HDInsight/help/Az.HDInsight.md file?
The related locations are as bellow:
https://github.com/Azure/azure-powershell/blob/186c05b31242bf54653585eb2082be3fd594e2ce/src/HDInsight/HDInsight/help/Az.HDInsight.md#get-azhdinsightmonitoring
Thanks |
Updated synopsis for [Get/Disable/Enable]-AzHDInsightMonitoring cmdlets.
@aim-for-better - Thanks, I didn't realize there was this file as well. I've made the updates. |
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.
Looking good. Let me merged the PR. Thank you both :)
Description
This PR includes updates to the synopsis & descriptions for the Enable-AzHDInsightMonitoring, Disable-AzHDInsightMonitoring & Get-AzHDInsightMonitoring cmdlets to indicate that they are for managing the Classic Azure Monitor logs integration on an HDInsight cluster.
Following are a few reasons why this is necessary:
At present, there are two sets of cmdlets for managing the integration of Azure Monitor Logs w/ an HDInsight cluster, namely:
However, the help docs for both sets of cmdlets don't mention how they are different, i.e., the first set of cmdlets integrate the cluster w/ Azure Monitor Logs in the "classic" way, while the second set of cmdlets being the newer ones. This causes confusion and would require further researching to identify how they differ (happened to me).
On the other hand, the docs for the Azure CLI HDInsight commands do highlight this difference - https://docs.microsoft.com/en-us/cli/azure/hdinsight/monitor?view=azure-cli-latest
Here is an example on how the docs for Enable-AzHDInsightMonitoring & Enable-AzHDInsightAzureMonitor appear to be mostly be the same:
The portal currently indicates the Log Analytics integration for HDInsight as "Classic" under Monitoring > Monitor Integration. It would help to indicate the same in the help docs for the respective set of cmdlets.
The classic integration of Azure Monitor Logs is soon to be deprecated and the recommendation is for customers to migrate to the newer integration as documented here - https://docs.microsoft.com/en-us/azure/hdinsight/log-analytics-migration. Updating the help docs to indicate that the first set of cmdlets are "classic" will help make users aware of this distinction.
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added