From 860dc51d029c1cf88bb124b58d2b0307756479b7 Mon Sep 17 00:00:00 2001 From: kceiw Date: Mon, 25 Jan 2021 06:45:21 -0800 Subject: [PATCH] Improve getting command ast and parse parameters (#13864) * Improve getting command ast and parse parameters. - We use the RelatedAsts from PSReadLine to get the command AST as the user input. This is what our prediction will try to match. - Now we can parse the parameter in the format of "-Name:Value". - We don't parse positioinal parameters yet. * Incorporate PR feedback. --- .../AzPredictorServiceTests.cs | 44 +++--- .../AzPredictorTests.cs | 4 +- .../Mocks/MockAzPredictorTelemetryClient.cs | 7 +- .../Az.Tools.Predictor/Az.Tools.Predictor.sln | 5 +- .../Az.Tools.Predictor/AzPredictor.cs | 4 +- .../Az.Tools.Predictor/AzPredictorService.cs | 41 +++++- .../CommandLinePredictor.cs | 7 +- .../Az.Tools.Predictor/IAzPredictorService.cs | 8 +- .../Az.Tools.Predictor/ParameterSet.cs | 27 +++- .../ParameterValuePredictor.cs | 130 ++++++++++++++---- .../Telemetry/AzPredictorTelemetryClient.cs | 100 ++++++++------ .../Telemetry/GetSuggestionTelemetryData.cs | 4 +- .../Telemetry/HistoryTelemetryData.cs | 4 +- .../Telemetry/ITelemetryClient.cs | 8 +- .../Telemetry/ITelemetryData.cs | 4 +- .../Telemetry/ParameterMapTelemetryData.cs | 47 +++++++ .../RequestPredictionTelemetryData.cs | 4 +- .../SuggestionAcceptedTelemetryData.cs | 4 +- 18 files changed, 330 insertions(+), 122 deletions(-) create mode 100644 tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ParameterMapTelemetryData.cs diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorServiceTests.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorServiceTests.cs index 377f5d2836dc..1d31cd08e278 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorServiceTests.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorServiceTests.cs @@ -89,10 +89,10 @@ public void VerifyParameterValues() Action actual = () => this._service.GetSuggestion(null, 1, 1, CancellationToken.None); Assert.Throws(actual); - actual = () => this._service.GetSuggestion(predictionContext.InputAst, 0, 1, CancellationToken.None); + actual = () => this._service.GetSuggestion(predictionContext, 0, 1, CancellationToken.None); Assert.Throws(actual); - actual = () => this._service.GetSuggestion(predictionContext.InputAst, 1, 0, CancellationToken.None); + actual = () => this._service.GetSuggestion(predictionContext, 1, 0, CancellationToken.None); Assert.Throws(actual); } @@ -110,8 +110,8 @@ public void VerifyParameterValues() public void VerifyUsingCommandBasedPredictor(string userInput) { var predictionContext = PredictionContext.Create(userInput); - var commandAst = predictionContext.InputAst.FindAll(p => p is CommandAst, true).LastOrDefault() as CommandAst; - var commandName = (commandAst?.CommandElements?.FirstOrDefault() as StringConstantExpressionAst)?.Value; + var commandAst = predictionContext.RelatedAsts.OfType().LastOrDefault(); + var commandName = commandAst?.GetCommandName(); var inputParameterSet = new ParameterSet(commandAst); var rawUserInput = predictionContext.InputAst.Extent.Text; var presentCommands = new Dictionary(); @@ -123,7 +123,7 @@ public void VerifyUsingCommandBasedPredictor(string userInput) 1, CancellationToken.None); - var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + var actual = this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.NotNull(actual); Assert.True(actual.Count > 0); Assert.NotNull(actual.PredictiveSuggestions.First()); @@ -133,7 +133,7 @@ public void VerifyUsingCommandBasedPredictor(string userInput) Assert.Equal(expected.SourceTexts, actual.SourceTexts); Assert.All(actual.SuggestionSources, (source) => Assert.Equal(SuggestionSource.CurrentCommand, source)); - actual = this._noFallbackPredictorService.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + actual = this._noFallbackPredictorService.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.NotNull(actual); Assert.True(actual.Count > 0); Assert.NotNull(actual.PredictiveSuggestions.First()); @@ -153,7 +153,7 @@ public void VerifyUsingCommandBasedPredictor(string userInput) public void VerifyUsingFallbackPredictor(string userInput) { var predictionContext = PredictionContext.Create(userInput); - var commandAst = predictionContext.InputAst.FindAll(p => p is CommandAst, true).LastOrDefault() as CommandAst; + var commandAst = predictionContext.RelatedAsts.OfType().LastOrDefault(); var commandName = (commandAst?.CommandElements?.FirstOrDefault() as StringConstantExpressionAst)?.Value; var inputParameterSet = new ParameterSet(commandAst); var rawUserInput = predictionContext.InputAst.Extent.Text; @@ -166,7 +166,7 @@ public void VerifyUsingFallbackPredictor(string userInput) 1, CancellationToken.None); - var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + var actual = this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.NotNull(actual); Assert.True(actual.Count > 0); Assert.NotNull(actual.PredictiveSuggestions.First()); @@ -176,7 +176,7 @@ public void VerifyUsingFallbackPredictor(string userInput) Assert.Equal(expected.SourceTexts, actual.SourceTexts); Assert.All(actual.SuggestionSources, (source) => Assert.Equal(SuggestionSource.StaticCommands, source)); - actual = this._noCommandBasedPredictorService.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + actual = this._noCommandBasedPredictorService.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.NotNull(actual); Assert.True(actual.Count > 0); Assert.NotNull(actual.PredictiveSuggestions.First()); @@ -199,16 +199,28 @@ public void VerifyUsingFallbackPredictor(string userInput) public void VerifyNoPrediction(string userInput) { var predictionContext = PredictionContext.Create(userInput); - var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + var actual = this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.Equal(0, actual.Count); - actual = this._noFallbackPredictorService.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + actual = this._noFallbackPredictorService.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.Equal(0, actual.Count); - actual = this._noCommandBasedPredictorService.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + actual = this._noCommandBasedPredictorService.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.Equal(0, actual.Count); - actual = this._noPredictorService.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + actual = this._noPredictorService.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); + Assert.Null(actual); + } + + /// + /// Verify that it returns null when we cannot parse the user input. + /// + [Theory] + [InlineData("git status")] + public void VerifyFailToParseUserInput(string userInput) + { + var predictionContext = PredictionContext.Create(userInput); + var actual = this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); Assert.Null(actual); } @@ -216,16 +228,14 @@ public void VerifyNoPrediction(string userInput) /// Verify when we cannot parse the user input correctly. /// /// - /// When we can parse them correctly, please move the InlineData to the corresponding test methods, for example, "git status" - /// doesn't have any prediction so it should move to . + /// When we can parse them correctly, please move the InlineData to the corresponding test methods. /// [Theory] - [InlineData("git status")] [InlineData("Get-AzContext Name")] public void VerifyMalFormattedCommandLine(string userInput) { var predictionContext = PredictionContext.Create(userInput); - Action actual = () => this._service.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); + Action actual = () => this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); _ = Assert.Throws(actual); } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorTests.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorTests.cs index d1f9f3f4aa55..3fbdb769bedb 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorTests.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorTests.cs @@ -306,8 +306,8 @@ public void VerifySupportedAndUnsupportedCommands() public void VerifySuggestion(string userInput) { var predictionContext = PredictionContext.Create(userInput); - var expected = _service.GetSuggestion(predictionContext.InputAst, 1, 1, CancellationToken.None); - var actual = _azPredictor.GetSuggestion(predictionContext, CancellationToken.None); + var expected = this._service.GetSuggestion(predictionContext, 1, 1, CancellationToken.None); + var actual = this._azPredictor.GetSuggestion(predictionContext, CancellationToken.None); Assert.Equal(expected.Count, actual.Count); Assert.Equal(expected.PredictiveSuggestions.First().SuggestionText, actual.First().SuggestionText); diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs index 1728ac6ea0fc..61e78d73ad19 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs @@ -12,8 +12,8 @@ // limitations under the License. // ---------------------------------------------------------------------------------- -using System; using Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry; +using System; namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks { @@ -58,5 +58,10 @@ public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData) { } + /// + public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData) + { + } + } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor.sln b/tools/Az.Tools.Predictor/Az.Tools.Predictor.sln index cc8a579534a1..2d4c4c0e2df5 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor.sln +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor.sln @@ -7,7 +7,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Az.Tools.Predictor", "Az.To EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Az.Tools.Predictor.Test", "Az.Tools.Predictor.Test\Az.Tools.Predictor.Test.csproj", "{C7A3ED31-8F41-4643-ADCF-85DF032BD8AC}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MockPSConsole", "MockPSConsole\MockPSConsole.csproj", "{80AFAFC7-9542-4CEB-AB5B-D1385A28CCE5}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MockPSConsole", "MockPSConsole\MockPSConsole.csproj", "{80AFAFC7-9542-4CEB-AB5B-D1385A28CCE5}" + ProjectSection(ProjectDependencies) = postProject + {E4A5F697-086C-4908-B90E-A31EE47ECF13} = {E4A5F697-086C-4908-B90E-A31EE47ECF13} + EndProjectSection EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs index 9fa881d3ed49..600c388a5603 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs @@ -23,7 +23,7 @@ using System.Runtime.CompilerServices; using System.Threading; -[assembly:InternalsVisibleTo("Microsoft.Azure.PowerShell.Tools.AzPredictor.Test")] +[assembly: InternalsVisibleTo("Microsoft.Azure.PowerShell.Tools.AzPredictor.Test")] namespace Microsoft.Azure.PowerShell.Tools.AzPredictor { @@ -195,7 +195,7 @@ public List GetSuggestion(PredictionContext context, Cance { var localCancellationToken = Settings.ContinueOnTimeout ? CancellationToken.None : cancellationToken; - suggestions = _service.GetSuggestion(context.InputAst, _settings.SuggestionCount.Value, _settings.MaxAllowedCommandDuplicate.Value, localCancellationToken); + suggestions = _service.GetSuggestion(context, _settings.SuggestionCount.Value, _settings.MaxAllowedCommandDuplicate.Value, localCancellationToken); var returnedValue = suggestions?.PredictiveSuggestions?.ToList(); return returnedValue ?? new List(); diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs index cbe0f2c3786f..fb7086a55e13 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs @@ -18,6 +18,7 @@ using System.Collections.Generic; using System.Linq; using System.Management.Automation.Language; +using System.Management.Automation.Subsystem; using System.Net.Http; using System.Net.Http.Headers; using System.Text; @@ -81,7 +82,7 @@ private sealed class CommandRequestContext /// private HashSet _allPredictiveCommands; private CancellationTokenSource _predictionRequestCancellationSource; - private readonly ParameterValuePredictor _parameterValuePredictor = new ParameterValuePredictor(); + private readonly ParameterValuePredictor _parameterValuePredictor; private readonly ITelemetryClient _telemetryClient; private readonly IAzContext _azContext; @@ -98,6 +99,8 @@ public AzPredictorService(string serviceUri, ITelemetryClient telemetryClient, I Validation.CheckArgument(telemetryClient, $"{nameof(telemetryClient)} cannot be null."); Validation.CheckArgument(azContext, $"{nameof(azContext)} cannot be null."); + _parameterValuePredictor = new ParameterValuePredictor(telemetryClient); + _commandsEndpoint = $"{serviceUri}{AzPredictorConstants.CommandsEndpoint}?clientType={AzPredictorService.ClientType}&context.versionNumber={azContext.AzVersion}"; _predictionsEndpoint = serviceUri + AzPredictorConstants.PredictionsEndpoint; _telemetryClient = telemetryClient; @@ -143,22 +146,46 @@ protected virtual void Dispose(bool disposing) /// Tries to get the suggestions for the user input from the command history. If that doesn't find /// suggestions, it'll fallback to find the suggestion regardless of command history. /// - public CommandLineSuggestion GetSuggestion(Ast input, int suggestionCount, int maxAllowedCommandDuplicate, CancellationToken cancellationToken) + public CommandLineSuggestion GetSuggestion(PredictionContext context, int suggestionCount, int maxAllowedCommandDuplicate, CancellationToken cancellationToken) { - Validation.CheckArgument(input, $"{nameof(input)} cannot be null"); + Validation.CheckArgument(context, $"{nameof(context)} cannot be null"); Validation.CheckArgument(suggestionCount > 0, $"{nameof(suggestionCount)} must be larger than 0."); Validation.CheckArgument(maxAllowedCommandDuplicate > 0, $"{nameof(maxAllowedCommandDuplicate)} must be larger than 0."); - var commandAst = input.FindAll(p => p is CommandAst, true).LastOrDefault() as CommandAst; - var commandName = (commandAst?.CommandElements?.FirstOrDefault() as StringConstantExpressionAst)?.Value; + var relatedAsts = context.RelatedAsts; + CommandAst commandAst = null; + + for (var i = relatedAsts.Count - 1; i >= 0; --i) + { + if (relatedAsts[i] is CommandAst c) + { + commandAst = c; + break; + } + } + + var commandName = commandAst?.GetCommandName(); if (string.IsNullOrWhiteSpace(commandName)) { return null; } - var inputParameterSet = new ParameterSet(commandAst); - var rawUserInput = input.Extent.Text; + ParameterSet inputParameterSet = null; + + try + { + inputParameterSet = new ParameterSet(commandAst); + } + catch when (!IsSupportedCommand(commandName)) + { + // We only ignore the exception when the command name is not supported. + // For the supported ones, this most likely happens when positional parameters are used. + // We want to collect the telemetry about the exception how common a positional parameter is used. + return null; + } + + var rawUserInput = context.InputAst.ToString(); var presentCommands = new Dictionary(); var commandBasedPredictor = _commandBasedPredictor; var commandToRequestPrediction = _commandToRequestPrediction; diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/CommandLinePredictor.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/CommandLinePredictor.cs index 840226a4dac4..0cdfcf12ddf4 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/CommandLinePredictor.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/CommandLinePredictor.cs @@ -65,12 +65,13 @@ public CommandLinePredictor(IList modelPredictions, Parameter { var predictionText = CommandLineUtilities.EscapePredictionText(predictiveCommand.Command); Ast ast = Parser.ParseInput(predictionText, out Token[] tokens, out _); - var commandAst = (ast.Find((ast) => ast is CommandAst, searchNestedScriptBlocks: false) as CommandAst); + var commandAst = ast.Find((ast) => ast is CommandAst, searchNestedScriptBlocks: false) as CommandAst; + var commandName = commandAst?.GetCommandName(); - if (commandAst?.CommandElements[0] is StringConstantExpressionAst commandName) + if (!string.IsNullOrWhiteSpace(commandName)) { var parameterSet = new ParameterSet(commandAst); - this._commandLinePredictions.Add(new CommandLine(commandName.Value, predictiveCommand.Description, parameterSet)); + this._commandLinePredictions.Add(new CommandLine(commandName, predictiveCommand.Description, parameterSet)); } } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/IAzPredictorService.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/IAzPredictorService.cs index 80825fd43fc7..e943bc13c1ee 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/IAzPredictorService.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/IAzPredictorService.cs @@ -12,9 +12,9 @@ // limitations under the License. // ---------------------------------------------------------------------------------- -using System; using System.Collections.Generic; using System.Management.Automation.Language; +using System.Management.Automation.Subsystem; using System.Threading; namespace Microsoft.Azure.PowerShell.Tools.AzPredictor @@ -27,12 +27,12 @@ public interface IAzPredictorService /// /// Gest the suggestions for the user input. /// - /// User input from PSReadLine. + /// User input context from PSReadLine. /// The number of suggestion to return. /// The cancellation token /// The maximum amount of the same commnds in the list of predictions. - /// The suggestions for . The maximum number of suggestions is . - public CommandLineSuggestion GetSuggestion(Ast input, int suggestionCount, int maxAllowedCommandDuplicate, CancellationToken cancellationToken); + /// The suggestions for . The maximum number of suggestions is . A null will be returned if there the user input context isn't valid/supported at all. + public CommandLineSuggestion GetSuggestion(PredictionContext context, int suggestionCount, int maxAllowedCommandDuplicate, CancellationToken cancellationToken); /// /// Requests predictions, given a command string. diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterSet.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterSet.cs index 57f0f82c91e8..db529404d04c 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterSet.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterSet.cs @@ -24,6 +24,9 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor /// does not matter to resulting prediction - the prediction should adapt to the /// order of the parameters typed by the user. /// + /// + /// This doesn't handle the positional parameters yet. + /// sealed class ParameterSet { /// @@ -36,11 +39,14 @@ public ParameterSet(CommandAst commandAst) Validation.CheckArgument(commandAst, $"{nameof(commandAst)} cannot be null."); var parameters = new List(); - var elements = commandAst.CommandElements.Skip(1); CommandParameterAst param = null; Ast arg = null; - foreach (Ast elem in elements) + + // Loop through all the parameters. The first element of CommandElements is the command name, so skip it. + for (var i = 1; i < commandAst.CommandElements.Count(); ++i) { + var elem = commandAst.CommandElements[i]; + if (elem is CommandParameterAst p) { AddParameter(param, arg); @@ -68,11 +74,22 @@ public ParameterSet(CommandAst commandAst) Parameters = parameters; - void AddParameter(CommandParameterAst parameterName, Ast parameterValue) + void AddParameter(CommandParameterAst parameter, Ast parameterValue) { - if (parameterName != null) + if (parameter != null) { - parameters.Add(new Parameter(parameterName.ParameterName, (parameterValue == null) ? null : CommandLineUtilities.UnescapePredictionText(parameterValue.ToString()))); + var value = parameterValue?.ToString(); + if (value == null) + { + value = parameter.Argument?.ToString(); + } + + if (value != null) + { + value = CommandLineUtilities.UnescapePredictionText(value); + } + + parameters.Add(new Parameter(parameter.ParameterName, value)); } } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterValuePredictor.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterValuePredictor.cs index 9d2d3246a8c4..e0d3d48c63d7 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterValuePredictor.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterValuePredictor.cs @@ -1,13 +1,25 @@ -using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities; +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry; +using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities; using System; -using System.IO; -using System.Text; -using System.Text.Json; -using System.Text.Json.Serialization; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Linq; +using System.IO; using System.Management.Automation.Language; +using System.Text.Json; @@ -22,12 +34,30 @@ sealed class ParameterValuePredictor private readonly Dictionary> _command_param_to_resource_map; - public ParameterValuePredictor() + private ITelemetryClient _telemetryClient; + + public ParameterValuePredictor(ITelemetryClient telemetryClient) { + Validation.CheckArgument(telemetryClient, $"{nameof(telemetryClient)} cannot be null."); + + _telemetryClient = telemetryClient; + var fileInfo = new FileInfo(typeof(Settings).Assembly.Location); var directory = fileInfo.DirectoryName; var mappingFilePath = Path.Join(directory, "command_param_to_resource_map.json"); - _command_param_to_resource_map = JsonSerializer.Deserialize>>(File.ReadAllText(mappingFilePath), JsonUtilities.DefaultSerializerOptions); + Exception exception = null; + + try + { + _command_param_to_resource_map = JsonSerializer.Deserialize>>(File.ReadAllText(mappingFilePath), JsonUtilities.DefaultSerializerOptions); + } + catch (Exception e) + { + // We don't want it to crash the module when the file doesn't exist or when it's mal-formatted. + exception = e; + } + + _telemetryClient.OnLoadParameterMap(new ParameterMapTelemetryData(exception)); } /// @@ -38,7 +68,7 @@ public void ProcessHistoryCommand(CommandAst command) { if (command != null) { - ExtractLocalParameters(command.CommandElements); + ExtractLocalParameters(command); } } @@ -55,22 +85,27 @@ public void ProcessHistoryCommand(CommandAst command) /// The parameter value from the history command. Null if that is not available. public string GetParameterValueFromAzCommand(string commandNoun, string parameterName) { - if (_command_param_to_resource_map.ContainsKey(commandNoun)) + parameterName = parameterName.ToLower(); + var key = parameterName; + Dictionary commandNounMap = null; + + if (_command_param_to_resource_map?.TryGetValue(commandNoun, out commandNounMap) == true) { - parameterName = parameterName.ToLower(); - if (_command_param_to_resource_map[commandNoun].ContainsKey(parameterName)) + if (commandNounMap.TryGetValue(parameterName, out var parameterNameMappedValue)) { - var key = _command_param_to_resource_map[commandNoun][parameterName]; - if (_localParameterValues.TryGetValue(key, out var value)) - { - return value; - } + key = parameterNameMappedValue; } } + + if (_localParameterValues.TryGetValue(key, out var value)) + { + return value; + } + return null; } - + public static string GetAzCommandNoun(string commandName) { var monikerIndex = commandName?.IndexOf(AzPredictorConstants.AzCommandMoniker, StringComparison.OrdinalIgnoreCase); @@ -94,34 +129,69 @@ public static string GetAzCommandNoun(string commandName) /// ResourceGroupName => Hello /// Location => 'EastUS' /// - /// The command ast elements - private void ExtractLocalParameters(System.Collections.ObjectModel.ReadOnlyCollection command) + /// The command ast element + /// + /// This doesn't support positional parameter. + /// + private void ExtractLocalParameters(CommandAst command) { // Azure PowerShell command is in the form of {Verb}-Az{Noun}, e.g. New-AzResource. // We need to extract the noun to construct the parameter name. - var commandName = command.FirstOrDefault()?.ToString(); + var commandName = command.GetCommandName(); var commandNoun = ParameterValuePredictor.GetAzCommandNoun(commandName)?.ToLower(); if (commandNoun == null) { return; } - for (int i = 2; i < command.Count; i += 2) + Dictionary commandNounMap = null; + _command_param_to_resource_map?.TryGetValue(commandNoun, out commandNounMap); + + for (int i = 1; i < command.CommandElements.Count;) { - if (command[i - 1] is CommandParameterAst parameterAst && command[i] is StringConstantExpressionAst) + if (command.CommandElements[i] is CommandParameterAst parameterAst) { - var parameterName = command[i - 1].ToString().ToLower().Trim('-'); - if (_command_param_to_resource_map.ContainsKey(commandNoun)) + var parameterName = parameterAst.ParameterName.ToLower(); + string parameterValue = null; + + // In the form of "-Name:Value" + if (parameterAst.Argument != null) + { + parameterValue = parameterAst.Argument.ToString(); + ++i; + } + else if (i + 1 < command.CommandElements.Count) + { + // We don't support positional parameter. + // The next element is either + // 1. The value of this parameter name. + // 2. This parameter is a switch parameter which doesn't have a value. The next element is a parameter. + + var nextElement = command.CommandElements[i + 1]; + + if (nextElement is CommandParameterAst) + { + ++i; + continue; + } + + parameterValue = command.CommandElements[i + 1].ToString(); + i += 2; + } + + var parameterKey = parameterName; + + if (commandNounMap != null) { - if (_command_param_to_resource_map[commandNoun].ContainsKey(parameterName)) + if (commandNounMap.TryGetValue(parameterName, out var mappedValue)) { - var key = _command_param_to_resource_map[commandNoun][parameterName]; - var parameterValue = command[i].ToString(); - this._localParameterValues.AddOrUpdate(key, parameterValue, (k, v) => parameterValue); + parameterKey = mappedValue; } } - } + + _localParameterValues.AddOrUpdate(parameterKey, parameterValue, (k, v) => parameterValue); + } } } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs index 52c1378600cc..cbcf3bf490b6 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs @@ -80,15 +80,7 @@ public AzPredictorTelemetryClient(IAzContext azContext) /// public void OnHistory(HistoryTelemetryData telemetryData) { - if (!IsDataCollectionAllowed()) - { - return; - } - - telemetryData.SessionId = SessionId; - telemetryData.CorrelationId = CorrelationId; - - _telemetryDispatcher.Post(telemetryData); + PostTelemetryData(telemetryData); #if TELEMETRY_TRACE && DEBUG System.Diagnostics.Trace.WriteLine("Recording CommandHistory"); @@ -98,17 +90,7 @@ public void OnHistory(HistoryTelemetryData telemetryData) /// public void OnRequestPrediction(RequestPredictionTelemetryData telemetryData) { - if (!IsDataCollectionAllowed()) - { - return; - } - - CorrelationId = Guid.NewGuid().ToString(); - - telemetryData.SessionId = SessionId; - telemetryData.CorrelationId = CorrelationId; - - _telemetryDispatcher.Post(telemetryData); + PostTelemetryData(telemetryData); #if TELEMETRY_TRACE && DEBUG System.Diagnostics.Trace.WriteLine("Recording RequestPrediction"); @@ -118,15 +100,7 @@ public void OnRequestPrediction(RequestPredictionTelemetryData telemetryData) /// public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData) { - if (!IsDataCollectionAllowed()) - { - return; - } - - telemetryData.SessionId = SessionId; - telemetryData.CorrelationId = CorrelationId; - - _telemetryDispatcher.Post(telemetryData); + PostTelemetryData(telemetryData); #if TELEMETRY_TRACE && DEBUG System.Diagnostics.Trace.WriteLine("Recording AcceptSuggestion"); @@ -136,18 +110,20 @@ public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData) /// public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData) { - if (!IsDataCollectionAllowed()) - { - return; - } + PostTelemetryData(telemetryData); - telemetryData.SessionId = SessionId; - telemetryData.CorrelationId = CorrelationId; +#if TELEMETRY_TRACE && DEBUG + System.Diagnostics.Trace.WriteLine("Recording GetSuggestion"); +#endif + } - _telemetryDispatcher.Post(telemetryData); + /// + public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData) + { + PostTelemetryData(telemetryData); #if TELEMETRY_TRACE && DEBUG - System.Diagnostics.Trace.WriteLine("Recording GetSuggestion"); + System.Diagnostics.Trace.WriteLine("Recording LoadParameterMap"); #endif } @@ -165,6 +141,38 @@ private static bool IsDataCollectionAllowed() return false; } + /// + /// Construct a string from an exception and the string is sent to telemetry. + /// The string should have minimum data that can help us to diagnose the exception + /// but not too excessive that may have PII. + /// + /// The exception to construct the string from. + /// A string to send to telemetry. + private static string FormatException(Exception exception) + { + if (exception == null) + { + return string.Empty; + } + + // The exception message may contain data such as file path if it is IO related exception. + // It's this solution to throw the exception, the type and the stack trace only contain information related to the solution. + return string.Format($"Type: {exception.GetType().ToString()}\nStack Trace: {exception.StackTrace?.ToString()}"); +; } + + private void PostTelemetryData(ITelemetryData telemetryData) + { + if (!IsDataCollectionAllowed()) + { + return; + } + + telemetryData.SessionId = SessionId; + telemetryData.CorrelationId = CorrelationId; + + _telemetryDispatcher.Post(telemetryData); + } + /// /// Dispatches according to its implementation. /// @@ -184,6 +192,9 @@ private void DispatchTelemetryData(ITelemetryData telemetryData) case SuggestionAcceptedTelemetryData suggestionAccepted: SendTelemetry(suggestionAccepted); break; + case ParameterMapTelemetryData parameterMap: + SendTelemetry(parameterMap); + break; default: throw new NotImplementedException(); } @@ -210,7 +221,7 @@ private void SendTelemetry(RequestPredictionTelemetryData telemetryData) var properties = CreateProperties(telemetryData); properties.Add("Command", telemetryData.Commands ?? string.Empty); properties.Add("HttpRequestSent", telemetryData.HasSentHttpRequest.ToString(CultureInfo.InvariantCulture)); - properties.Add("Exception", telemetryData.Exception?.ToString() ?? string.Empty); + properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception)); _telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/RequestPrediction", properties); } @@ -237,7 +248,7 @@ private void SendTelemetry(GetSuggestionTelemetryData telemetryData) properties.Add("UserInput", maskedUserInput ?? string.Empty); properties.Add("Suggestion", sourceTexts != null ? JsonSerializer.Serialize(sourceTexts.Zip(suggestionSource).Select((s) => Tuple.Create(s.First, s.Second)), JsonUtilities.TelemetrySerializerOptions) : string.Empty); properties.Add("IsCancelled", telemetryData.IsCancellationRequested.ToString(CultureInfo.InvariantCulture)); - properties.Add("Exception", telemetryData.Exception?.ToString() ?? string.Empty); + properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception)); _telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/GetSuggestion", properties); } @@ -258,6 +269,17 @@ private void SendTelemetry(SuggestionAcceptedTelemetryData telemetryData) _telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/AcceptSuggestion", properties); } + /// + /// Sends the telemetry with the parameter map file loading information. + /// + private void SendTelemetry(ParameterMapTelemetryData telemetryData) + { + var properties = CreateProperties(telemetryData); + properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception)); + + _telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/LoadParameterMap", properties); + } + /// /// Add the common properties to the telemetry event. /// diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs index 9d2908c13690..edd1ee223043 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs @@ -23,10 +23,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry public sealed class GetSuggestionTelemetryData : ITelemetryData { /// - public string SessionId { get; internal set; } + string ITelemetryData.SessionId { get; set; } /// - public string CorrelationId { get; internal set; } + string ITelemetryData.CorrelationId { get; set; } /// /// Gets the user input. diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/HistoryTelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/HistoryTelemetryData.cs index 91b94209be40..4f4096965f3f 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/HistoryTelemetryData.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/HistoryTelemetryData.cs @@ -20,10 +20,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry public sealed class HistoryTelemetryData : ITelemetryData { /// - public string SessionId { get; internal set; } + string ITelemetryData.SessionId { get; set; } /// - public string CorrelationId { get; internal set; } + string ITelemetryData.CorrelationId { get; set; } /// /// Gets the history command line. diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryClient.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryClient.cs index 037102beaa58..d38040ae7380 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryClient.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryClient.cs @@ -48,9 +48,15 @@ public interface ITelemetryClient public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData); /// - /// Collects when we return a suggestion + /// Collects when we return a suggestion. /// /// The data to collect. public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData); + + /// + /// Collects when we load parameter map file. + /// + /// The data to collect. + public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData); } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryData.cs index a326a328cbe8..5e2661453e0e 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryData.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryData.cs @@ -23,11 +23,11 @@ public interface ITelemetryData /// /// Gets the session id. /// - string SessionId { get; } + string SessionId { get; internal set; } /// /// Gets the correlation id. /// - string CorrelationId { get; } + string CorrelationId { get; internal set; } } } diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ParameterMapTelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ParameterMapTelemetryData.cs new file mode 100644 index 000000000000..91ba42baff51 --- /dev/null +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ParameterMapTelemetryData.cs @@ -0,0 +1,47 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +using System; + +namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry +{ + /// + /// The data to collect in . + /// + public sealed class ParameterMapTelemetryData : ITelemetryData + { + /// + string ITelemetryData.SessionId { get; set; } + + /// + string ITelemetryData.CorrelationId { get; set; } + + /// + /// Gets the exception if there is an error during the operation. + /// + /// + /// OperationCanceledException isn't considered an error. + /// + public Exception Exception { get; } + + /// + /// Creates a new instance of . + /// + /// The exception that is thrown if there is an error. + public ParameterMapTelemetryData(Exception exception) + { + Exception = exception; + } + } +} diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/RequestPredictionTelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/RequestPredictionTelemetryData.cs index dec60ac2f21c..64ffa71d2610 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/RequestPredictionTelemetryData.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/RequestPredictionTelemetryData.cs @@ -22,10 +22,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry public sealed class RequestPredictionTelemetryData : ITelemetryData { /// - public string SessionId { get; internal set; } + string ITelemetryData.SessionId { get; set; } /// - public string CorrelationId { get; internal set; } + string ITelemetryData.CorrelationId { get; set; } /// /// Gets the masked command lines that are used to request prediction. diff --git a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/SuggestionAcceptedTelemetryData.cs b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/SuggestionAcceptedTelemetryData.cs index 133e97edfcbb..e863f11aa947 100644 --- a/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/SuggestionAcceptedTelemetryData.cs +++ b/tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/SuggestionAcceptedTelemetryData.cs @@ -20,10 +20,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry public sealed class SuggestionAcceptedTelemetryData : ITelemetryData { /// - public string SessionId { get; internal set; } + string ITelemetryData.SessionId { get; set; } /// - public string CorrelationId { get; internal set; } + string ITelemetryData.CorrelationId { get; set; } /// /// Gets the suggestion that's accepted by the user.