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

[CLI][Spring-Cloud] Move to GA for Java In-process Agent #3623

Merged
merged 44 commits into from
Sep 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
228fd9b
In param help, change instrumentation key to connection string
jiec-msft Sep 1, 2021
8bca996
update dependency of azure cli to have connection_string in data model
jiec-msft Sep 1, 2021
0221de0
update param description for app_insights_key
jiec-msft Sep 1, 2021
73b96a9
update history and setup version for IPA GA CLI
jiec-msft Sep 1, 2021
c78d71a
rename term distributed tracing to Application Insights in help
jiec-msft Sep 1, 2021
9b9c6f1
remove Java In-process Agent or (IPA) in help
jiec-msft Sep 1, 2021
8c12e56
deco `disable_distributed_tracing` parameter in
jiec-msft Sep 1, 2021
b2f9797
mark `enable_java_agent` as deprecated. And ignore this parameter. By…
jiec-msft Sep 1, 2021
fd3927b
By default enable java agent in az spring-cloud update command group
jiec-msft Sep 1, 2021
281669c
separate ai parameters from asc create and asc update
jiec-msft Sep 1, 2021
5a3ecfd
merge same parameter for asc create and asc update
jiec-msft Sep 1, 2021
cf22ece
remove help for asc update against ai settings
jiec-msft Sep 1, 2021
52a524d
add warn log for `enable-java-agent`
jiec-msft Sep 1, 2021
8434be0
support connection string in help info
jiec-msft Sep 1, 2021
0dd21b6
refine too long string help info
jiec-msft Sep 1, 2021
ffba10d
remove preview tag for asc app-insights command group
jiec-msft Sep 1, 2021
8f92e98
rename trace_properties to monitoring_setting_properties
jiec-msft Sep 1, 2021
5f94b7c
support both connection string and instrumentation key
jiec-msft Sep 1, 2021
46d7d46
support sampling rate
jiec-msft Sep 2, 2021
a5b8968
refine validator error message
jiec-msft Sep 2, 2021
7c4604b
add validator back for enable_java_agent and refine the wording.
jiec-msft Sep 2, 2021
db2d1c7
refine history for the changes.
jiec-msft Sep 2, 2021
0b5c48b
fix bug for app-insights update
jiec-msft Sep 2, 2021
0494dbd
fix bug for app-insights update
jiec-msft Sep 2, 2021
f038e1d
update version
jiec-msft Sep 3, 2021
08bc626
Fix bug for asc update disable ai
jiec-msft Sep 4, 2021
63e4403
Merge branch 'jiec/connection_str' of github.com:ECNUJason/azure-cli-…
jiec-msft Sep 4, 2021
4eb1d0e
add support no-wait for ipa ga
jiec-msft Sep 4, 2021
7effb0f
add scenario test for asc create and update
jiec-msft Sep 5, 2021
2945c70
add scenario test for asc app insights update
jiec-msft Sep 6, 2021
b4ade2b
add negative cases for asc create
jiec-msft Sep 6, 2021
c94641f
add negative cases for asc update
jiec-msft Sep 6, 2021
f4616ec
add negative cases for asc app-insights update
jiec-msft Sep 6, 2021
fbc3a29
enhance scenario for asc app-insights update
jiec-msft Sep 6, 2021
50ca642
enhance scenario for asc create
jiec-msft Sep 6, 2021
2de872f
add scenario for asc create with ai disabled
jiec-msft Sep 6, 2021
a395598
Merge branch 'masonOrigin/main' into jiec/connection_str
jiec-msft Sep 23, 2021
9775398
fix for style check
jiec-msft Sep 23, 2021
6d10c54
remove comment
jiec-msft Sep 23, 2021
595b76a
remove dependency on other modules
jiec-msft Sep 23, 2021
4615e23
replace CLIError with concrete error class
jiec-msft Sep 24, 2021
31ce16c
add track work item for refactoring the validators
jiec-msft Sep 24, 2021
4f3fed1
add comment to explain record_only
jiec-msft Sep 24, 2021
e3fdc06
update history
jiec-msft Sep 24, 2021
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
13 changes: 13 additions & 0 deletions src/spring-cloud/HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
Release History
===============
2.10.0
-----
* Support functions for Java In-Process Agent feature General Available.
* For Application Insights configuration, support both `connection_string` and `instrumentation_key`,
and we recommended to use `connection_string`.
* Enabling In-Process Agent is equivalent to enabling application insights.
* Mark `enable-java-agent` as deprecated, since IPA is GA-ed.
* Mark application insights related parameter as deprecated in `az spring-cloud update`,
it's still supported, but will de decommissioned in the future,
and we recommended to use `az spring-cloud app-insights update`.
* Support `--sampling-rate` in `az spring-cloud create`.
* Decommissioned `disable-distributed-tracing` parameter.

ShichaoQiu marked this conversation as resolved.
Show resolved Hide resolved
2.9.0
-----
* Add --source-path argument into 'az spring-cloud app deploy' and 'az spring-cloud app deployment create'
Expand Down
18 changes: 9 additions & 9 deletions src/spring-cloud/azext_spring_cloud/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
examples:
- name: Create a new Azure Spring Cloud in westus.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus
- name: Create a new Azure Spring Cloud in westus with an existing Application Insights by using the instrumentation key.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus --app-insights-key MyInstrumentationKey
- name: Create a new Azure Spring Cloud in westus with an existing Application Insights and enable Java In-Process Agent.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus --enable-java-agent true --app-insights MyInstrumentationName
- name: Create a new Azure Spring Cloud with distributed tracing disabled.
- name: Create a new Azure Spring Cloud in westus with an existing Application Insights by using the Connection string (recommended) or Instrumentation key.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus --app-insights-key \"MyConnectionString\"
- name: Create a new Azure Spring Cloud in westus with an existing Application Insights.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus --app-insights appInsightsName
- name: Create a new Azure Spring Cloud in westus with an existing Application Insights and specify the sampling rate.
text: az spring-cloud create -n MyService -g MyResourceGroup -l westus --app-insights appInsightsName --sampling-rate 10
- name: Create a new Azure Spring Cloud with Application Insights disabled.
text: az spring-cloud create -n MyService -g MyResourceGroup --disable-app-insights
- name: Create a new Azure Spring Cloud with VNet-injected via giving VNet name in current resource group
text: az spring-cloud create -n MyService -g MyResourceGroup --vnet MyVNet --app-subnet MyAppSubnet --service-runtime-subnet MyServiceRuntimeSubnet
Expand All @@ -35,8 +37,6 @@
examples:
- name: Update pricing tier.
text: az spring-cloud update -n MyService --sku Standard -g MyResourceGroup
- name: Enable the distributed tracing of the existing Azure Spring Cloud.
text: az spring-cloud update -n MyService -g MyResourceGroup --disable-app-insights false
- name: Update the tags of the existing Azure Spring Cloud.
text: az spring-cloud update -n MyService -g MyResourceGroup --tags key1=value1 key2=value2
"""
Expand Down Expand Up @@ -461,8 +461,8 @@
type: command
short-summary: Update Application Insights settings.
examples:
- name: Enable Application Insights and Java In-process Agent.
text: az spring-cloud app-insights update -n MyService -g MyResourceGroup --app-insights-key MyInstrumentationKey --sampling-rate 100
- name: Enable Application Insights by using the Connection string (recommended) or Instrumentation key.
text: az spring-cloud app-insights update -n MyService -g MyResourceGroup --app-insights-key \"MyConnectionString\" --sampling-rate 100
- name: Disable Application Insights.
text: az spring-cloud app-insights update -n MyService -g MyResourceGroup --disable
"""
79 changes: 57 additions & 22 deletions src/spring-cloud/azext_spring_cloud/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
validate_name, validate_app_name, validate_deployment_name, validate_log_lines,
validate_log_limit, validate_log_since, validate_sku, validate_jvm_options,
validate_vnet, validate_vnet_required_parameters, validate_node_resource_group,
validate_tracing_parameters, validate_app_insights_parameters, validate_java_agent_parameters,
validate_instance_count, validate_jar)
validate_tracing_parameters_asc_create, validate_tracing_parameters_asc_update,
validate_app_insights_parameters, validate_instance_count, validate_java_agent_parameters,
validate_jar)
from ._utils import ApiType

from .vendored_sdks.appplatform.v2020_07_01.models import RuntimeVersion, TestKeyType
Expand All @@ -33,36 +34,69 @@ def load_arguments(self, _):
c.argument('name', options_list=[
'--name', '-n'], help='Name of Azure Spring Cloud.')

# A refactoring work item to move validators to command level to reduce the duplications.
# https://dev.azure.com/msazure/AzureDMSS/_workitems/edit/11002857/
with self.argument_context('spring-cloud create') as c:
c.argument('location', arg_type=get_location_type(self.cli_ctx), validator=validate_location)
c.argument('sku', type=str, validator=validate_sku, help='Name of SKU, the value is "Basic" or "Standard"')
c.argument('reserved_cidr_range', help='Comma-separated list of IP address ranges in CIDR format. The IP ranges are reserved to host underlying Azure Spring Cloud infrastructure, which should be 3 at least /16 unused IP ranges, must not overlap with any Subnet IP ranges.', validator=validate_vnet_required_parameters)
c.argument('vnet', help='The name or ID of an existing Virtual Network into which to deploy the Spring Cloud instance.', validator=validate_vnet_required_parameters)
c.argument('app_subnet', help='The name or ID of an existing subnet in "vnet" into which to deploy the Spring Cloud app. Required when deploying into a Virtual Network. Smaller subnet sizes are supported, please refer: https://aka.ms/azure-spring-cloud-smaller-subnet-vnet-docs', validator=validate_vnet_required_parameters)
c.argument('service_runtime_subnet', options_list=['--service-runtime-subnet', '--svc-subnet'], help='The name or ID of an existing subnet in "vnet" into which to deploy the Spring Cloud service runtime. Required when deploying into a Virtual Network.', validator=validate_vnet)
c.argument('service_runtime_network_resource_group', options_list=['--service-runtime-network-resource-group', '--svc-nrg'], help='The resource group where all network resources for Azure Spring Cloud service runtime will be created in.', validator=validate_node_resource_group)
c.argument('app_network_resource_group', options_list=['--app-network-resource-group', '--app-nrg'], help='The resource group where all network resources for apps will be created in.', validator=validate_node_resource_group)
c.argument('enable_java_agent', is_preview=True, arg_type=get_three_state_flag(), help="Enable java in-process agent", validator=validate_java_agent_parameters)
c.argument('enable_java_agent',
arg_type=get_three_state_flag(),
help="Java in process agent is now GA-ed and used by default when Application Insights enabled. "
"This parameter is no longer needed and will be removed in future release.",
validator=validate_java_agent_parameters,
deprecate_info=c.deprecate(target='--enable-java-agent', hide=True))
c.argument('app_insights_key',
help="Connection string (recommended) or Instrumentation key of the existing Application Insights.",
validator=validate_tracing_parameters_asc_create)
c.argument('app_insights',
help="Name of the existing Application Insights in the same Resource Group. "
"Or Resource ID of the existing Application Insights in a different Resource Group.",
validator=validate_tracing_parameters_asc_create)
c.argument('sampling_rate',
type=float,
help="Sampling Rate of application insights. Minimum is 0, maximum is 100.",
validator=validate_tracing_parameters_asc_create)
c.argument('disable_app_insights',
arg_type=get_three_state_flag(),
help="Disable Application Insights, "
"if not disabled and no existing Application Insights specified with "
"--app-insights-key or --app-insights, "
"will create a new Application Insights instance in the same resource group.",
validator=validate_tracing_parameters_asc_create)
Comment on lines +53 to +70
Copy link
Contributor

@zhoxing-ms zhoxing-ms Sep 24, 2021

Choose a reason for hiding this comment

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

Could we make the validate_tracing_parameters_asc_create to be a command level validator instead of calling the same validator for multiple parameters? We can call the different validator methods with different parameters at the command level validator.


with self.argument_context('spring-cloud update') as c:
c.argument('sku', type=str, validator=validate_sku, help='Name of SKU, the value is "Basic" or "Standard"')
c.argument('app_insights_key',
help="Connection string (recommended) or Instrumentation key of the existing Application Insights.",
validator=validate_tracing_parameters_asc_update,
deprecate_info=c.deprecate(target='az spring-cloud update --app-insights-key',
redirect='az spring-cloud app-insights update --app-insights-key',
hide=True))
c.argument('app_insights',
help="Name of the existing Application Insights in the same Resource Group. "
"Or Resource ID of the existing Application Insights in a different Resource Group.",
validator=validate_tracing_parameters_asc_update,
deprecate_info=c.deprecate(target='az spring-cloud update --app-insights',
redirect='az spring-cloud app-insights update --app-insights',
hide=True))
c.argument('disable_app_insights',
arg_type=get_three_state_flag(),
help="Disable Application Insights, "
"if not disabled and no existing Application Insights specified with "
"--app-insights-key or --app-insights, "
"will create a new Application Insights instance in the same resource group.",
validator=validate_tracing_parameters_asc_update,
deprecate_info=c.deprecate(target='az spring-cloud update --disable-app-insights',
redirect='az spring-cloud app-insights update --disable',
hide=True))

for scope in ['spring-cloud create', 'spring-cloud update']:
with self.argument_context(scope) as c:
c.argument('app_insights_key',
help="Instrumentation key of the existing Application Insights.",
validator=validate_tracing_parameters)
c.argument('app_insights',
help="Name of the existing Application Insights in the same Resource Group. Or Resource ID of the existing Application Insights in a different Resource Group.",
validator=validate_tracing_parameters)
c.argument('disable_distributed_tracing',
arg_type=get_three_state_flag(),
help="Disable distributed tracing, if not disabled and no existing Application Insights specified with --app-insights-key or --app-insights, will create a new Application Insights instance in the same resource group.",
validator=validate_tracing_parameters,
deprecate_info=c.deprecate(target='--disable-distributed-tracing', redirect='--disable-app-insights', hide=True))
c.argument('disable_app_insights',
arg_type=get_three_state_flag(),
help="Disable Application Insights, if not disabled and no existing Application Insights specified with --app-insights-key or --app-insights, will create a new Application Insights instance in the same resource group.",
validator=validate_tracing_parameters)
c.argument('sku', type=str, validator=validate_sku, help='Name of SKU, the value is "Basic" or "Standard"')
c.argument('tags', arg_type=tags_type)

with self.argument_context('spring-cloud test-endpoint renew-key') as c:
Expand Down Expand Up @@ -268,10 +302,11 @@ def prepare_logs_argument(c):

with self.argument_context('spring-cloud app-insights update') as c:
c.argument('app_insights_key',
help="Instrumentation key of the existing Application Insights",
help="Connection string (recommended) or Instrumentation key of the existing Application Insights.",
validator=validate_app_insights_parameters)
c.argument('app_insights',
help="Name of the existing Application Insights in the same Resource Group. Or Resource ID of the existing Application Insights in a different Resource Group.",
help="Name of the existing Application Insights in the same Resource Group. "
"Or Resource ID of the existing Application Insights in a different Resource Group.",
validator=validate_app_insights_parameters)
c.argument('sampling_rate',
type=float,
Expand Down
58 changes: 42 additions & 16 deletions src/spring-cloud/azext_spring_cloud/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,34 +155,60 @@ def validate_jvm_options(namespace):
namespace.jvm_options = namespace.jvm_options.strip('\'')


def validate_tracing_parameters(namespace):
if namespace.disable_app_insights and namespace.disable_distributed_tracing:
raise InvalidArgumentValueError("Conflict detected: '--disable-app-insights' can not be set with '--disable-distributed-tracing'.")
def validate_tracing_parameters_asc_create(namespace):
if (namespace.app_insights or namespace.app_insights_key or namespace.sampling_rate is not None) \
and namespace.disable_app_insights:
raise InvalidArgumentValueError(
"Conflict detected: '--app-insights' or '--app-insights-key' or '--sampling-rate' "
"can not be set with '--disable-app-insights'.")
_validate_app_insights_parameters(namespace)


def validate_tracing_parameters_asc_update(namespace):
if (namespace.app_insights or namespace.app_insights_key) and namespace.disable_app_insights:
raise InvalidArgumentValueError("Conflict detected: '--app-insights' or '--app-insights-key'"
"can not be set with '--disable-app-insights'.")
if (namespace.app_insights or namespace.app_insights_key) and namespace.disable_distributed_tracing:
raise InvalidArgumentValueError("Conflict detected: '--app-insights' or '--app-insights-key'"
"can not be set with '--disable-distributed-tracing'.")
raise InvalidArgumentValueError(
"Conflict detected: '--app-insights' or '--app-insights-key' "
"can not be set with '--disable-app-insights'.")
if namespace.app_insights and namespace.app_insights_key:
raise InvalidArgumentValueError("Conflict detected: '--app-insights' and '--app-insights-key' can not be set at the same time.")
raise InvalidArgumentValueError(
"Conflict detected: '--app-insights' and '--app-insights-key' can not be set at the same time.")
if namespace.app_insights == "":
raise InvalidArgumentValueError("Conflict detected: '--app-insights' can not be empty.")


def validate_java_agent_parameters(namespace):
"""TODO (jiec) Deco this function when 'enable-java-agent' is decommissioned.
"""
if namespace.disable_app_insights and namespace.enable_java_agent:
raise InvalidArgumentValueError("Conflict detected: '--enable-java-in-process-agent' and '--disable-app-insights' can not be set at the same time.")
raise InvalidArgumentValueError(
"Conflict detected: '--enable-java-agent' and '--disable-app-insights' "
"can not be set at the same time.")


def validate_app_insights_parameters(namespace):
if (namespace.app_insights or namespace.app_insights_key or namespace.sampling_rate) and namespace.disable:
raise InvalidArgumentValueError("Conflict detected: '--app-insights' or '--app-insights-key' or '--sampling-rate'"
"can not be set with '--disable'.")
if (namespace.app_insights or namespace.app_insights_key or namespace.sampling_rate is not None) \
and namespace.disable:
raise InvalidArgumentValueError(
"Conflict detected: '--app-insights' or '--app-insights-key' or '--sampling-rate' "
"can not be set with '--disable'.")
if not namespace.app_insights \
and not namespace.app_insights_key \
and namespace.sampling_rate is None \
and not namespace.disable:
raise InvalidArgumentValueError("Invalid value: nothing is updated for application insights.")
_validate_app_insights_parameters(namespace)


def _validate_app_insights_parameters(namespace):
if namespace.app_insights and namespace.app_insights_key:
raise InvalidArgumentValueError("Conflict detected: '--app-insights' and '--app-insights-key' can not be set at the same time.")
if namespace.sampling_rate and (namespace.sampling_rate < 0 or namespace.sampling_rate > 100):
raise InvalidArgumentValueError("Sampling Rate must be in the range [0,100].")
raise InvalidArgumentValueError(
"Conflict detected: '--app-insights' and '--app-insights-key' can not be set at the same time.")
if namespace.app_insights == "":
raise InvalidArgumentValueError("Invalid value: '--app-insights' can not be empty.")
if namespace.app_insights_key == "":
raise InvalidArgumentValueError("Invalid value: '--app-insights-key' can not be empty.")
if namespace.sampling_rate is not None and (namespace.sampling_rate < 0 or namespace.sampling_rate > 100):
raise InvalidArgumentValueError("Invalid value: Sampling Rate must be in the range [0,100].")


def validate_vnet(cmd, namespace):
Expand Down
2 changes: 1 addition & 1 deletion src/spring-cloud/azext_spring_cloud/azext_metadata.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"azext.isPreview": false,
"azext.minCliCoreVersion": "2.0.67"
"azext.minCliCoreVersion": "2.25.0"
jiec-msft marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 2 additions & 2 deletions src/spring-cloud/azext_spring_cloud/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ def load_command_table(self, _):
g.custom_command('update', 'domain_update')
g.custom_command('unbind', 'domain_unbind')

with self.command_group('spring-cloud app-insights', is_preview=True,
with self.command_group('spring-cloud app-insights',
client_factory=cf_spring_cloud_20201101preview,
exception_handler=handle_asc_exception) as g:
g.custom_command('update', 'app_insights_update')
g.custom_command('update', 'app_insights_update', supports_no_wait=True)
g.custom_show_command('show', 'app_insights_show')

with self.command_group('spring-cloud', exception_handler=handle_asc_exception):
Expand Down
Loading