[App Service] Fix #30030, #10043, #29147, #28987: monitoring, logging, and infrastructure improvements#33073
[App Service] Fix #30030, #10043, #29147, #28987: monitoring, logging, and infrastructure improvements#33073
Conversation
… monitoring, logging, and infrastructure improvements - Azure#30030: Add Application Insights connection string and instrumentation key examples to `webapp config appsettings set` help with guidance on enabling App Insights monitoring via app settings. - Azure#10043: Add Azure Blob Storage support for web server logging via `--web-server-logging azureblobstorage` with new `--web-server-log-sas-url` and `--web-server-log-retention` parameters in `webapp log config`. - Azure#29147: Add warning message and documentation to `webapp traffic-routing set` about potential app restart when updating traffic routing configuration (known platform behavior). - Azure#28987: Add VNet/private endpoint guidance to `webapp config backup create` and `webapp config backup restore` help text, documenting requirements for backup/restore with storage accounts behind VNets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
seligj95
requested review from
NoriZC,
teresaritorto and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
|
Hi @seligj95, |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| webapp log config | cmd webapp log config added parameter web_server_log_retention |
||
| webapp log config | cmd webapp log config added parameter web_server_log_sas_url |
||
| webapp log config | cmd webapp log config update parameter web_server_logging: updated property choices from ['filesystem', 'off'] to ['azureblobstorage', 'filesystem', 'off'] |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
jsntcy,
wangzelin007,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
bot
added
Web Apps
There was a problem hiding this comment.
Pull request overview
This PR improves the App Service (appservice) command module by adding user guidance and support for blob-backed web server logging, plus a user-facing warning about potential restarts when configuring traffic routing.
Changes:
- Extend
az webapp log configto support--web-server-logging azureblobstoragewith new SAS URL + retention parameters, and implement corresponding SDK configuration inconfig_diagnostics(). - Add a warning in
set_traffic_routing()about possible brief restarts when updating ramp-up rules. - Expand help text/examples for Application Insights app settings guidance and for backup/restore scenarios involving VNet/private endpoint storage, and add unit tests covering the new behaviors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Implements Azure Blob Storage HTTP logs configuration and adds traffic routing restart warning. |
src/azure-cli/azure/cli/command_modules/appservice/_params.py |
Adds new CLI args for blob-backed web server logs and expands the --web-server-logging enum. |
src/azure-cli/azure/cli/command_modules/appservice/_help.py |
Adds/updates long summaries and examples for monitoring/logging/backup guidance and traffic routing caveats. |
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py |
Adds unit tests for blob logging argument requirements and traffic routing warning emission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| mock_site_op.assert_called_once() | ||
| call_args = mock_site_op.call_args | ||
| site_log_config = call_args[0][5] |
There was a problem hiding this comment.
This test inspects the 6th positional argument of _generic_site_operation (call_args[0][5]) to get site_log_config, which is brittle if _generic_site_operation's signature changes. Prefer asserting against mock_site_op.call_args.args[-1] (or kwargs) and/or asserting the expected operation name/slot explicitly to make the test less order-dependent.
| site_log_config = call_args[0][5] | |
| site_log_config = call_args.args[-1] |
| # TODO: expose new blob support | ||
| def config_diagnostics(cmd, resource_group_name, name, level=None, |
There was a problem hiding this comment.
The TODO comment about "expos[ing] new blob support" is now outdated since this PR adds the azureblobstorage option and related parameters. Please remove or update the TODO to avoid misleading future maintainers.
| blob_log_config = AzureBlobStorageHttpLogsConfig( | ||
| sas_url=web_server_log_sas_url, | ||
| retention_in_days=retention, | ||
| enabled=True) | ||
| http_logs = HttpLogsConfig(file_system=filesystem_log_config, azure_blob_storage=blob_log_config) |
There was a problem hiding this comment.
web_server_log_sas_url is a credential-bearing SAS URL (contains sig=) and will likely be echoed back in the SiteLogsConfig returned from _generic_site_operation, which means az webapp log config output can leak the SAS token to the console/logs. Consider redacting/masking the SAS URL in the returned object (or returning a sanitized response) while still sending the full value in the request.
|
Consolidated into #33066 |
Description
This PR addresses four App Service infrastructure/monitoring issues:
Fix #30030: Application Insights CLI guidance
az webapp config appsettings setshowing how to enable Application Insights monitoring viaAPPLICATIONINSIGHTS_CONNECTION_STRING(recommended) andAPPINSIGHTS_INSTRUMENTATIONKEY(legacy).Fix #10043: Web server logging to Azure Blob Storage
azureblobstorageoption to--web-server-loggingparameter inaz webapp log config.--web-server-log-sas-urland--web-server-log-retentionparameters.AzureBlobStorageHttpLogsConfigsupport inconfig_diagnostics().Fix #29147: Traffic routing restart warning
logger.warning()message inset_traffic_routing()to inform users that updating traffic routing may cause a brief restart (known platform behavior).az webapp traffic-routing setdocumenting this limitation.Fix #28987: Backup/restore VNet guidance
az webapp config backup createandaz webapp config backup restore, documenting requirements for storage accounts behind VNets.Testing
test_webapp_commands_thru_mock.pyazdev style appservicepasses (pylint + flake8)azdev linter appservicehas pre-existing crash in batch module (not related to this PR)Files Changed
_help.py— Enhanced help text and examples for all four issues_params.py— Added blob storage logging params (--web-server-log-sas-url,--web-server-log-retention,azureblobstorageenum value)custom.py— Implemented blob storage HTTP logging, added traffic routing warningtest_webapp_commands_thru_mock.py— Added 3 new unit tests