Skip to content

Commit a4750b5

Browse files
kceiwkceiw
committed
Incorporate PR feedback.
1 parent ef2435c commit a4750b5

13 files changed

+170
-58
lines changed

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorService.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using System;
1615
using System.Collections.Generic;
16+
using System.Management.Automation.Language;
1717

1818
namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks
1919
{
@@ -62,5 +62,10 @@ protected override void RequestAllPredictiveCommands()
6262
{
6363
// Do nothing since we've set the command and suggestion predictors.
6464
}
65+
66+
/// <inheritdoc/>
67+
public override void RecordHistory(CommandAst history)
68+
{
69+
}
6570
}
6671
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using System;
1615
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry;
16+
using System;
1717

1818
namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks
1919
{
@@ -58,5 +58,10 @@ public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData)
5858
{
5959
}
6060

61+
/// <inheritdoc/>
62+
public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData)
63+
{
64+
}
65+
6166
}
6267
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ private sealed class CommandRequestContext
8282
/// </summary>
8383
private HashSet<string> _allPredictiveCommands;
8484
private CancellationTokenSource _predictionRequestCancellationSource;
85-
private readonly ParameterValuePredictor _parameterValuePredictor = new ParameterValuePredictor();
85+
private readonly ParameterValuePredictor _parameterValuePredictor;
8686

8787
private readonly ITelemetryClient _telemetryClient;
8888
private readonly IAzContext _azContext;
@@ -99,6 +99,8 @@ public AzPredictorService(string serviceUri, ITelemetryClient telemetryClient, I
9999
Validation.CheckArgument(telemetryClient, $"{nameof(telemetryClient)} cannot be null.");
100100
Validation.CheckArgument(azContext, $"{nameof(azContext)} cannot be null.");
101101

102+
_parameterValuePredictor = new ParameterValuePredictor(telemetryClient);
103+
102104
_commandsEndpoint = $"{serviceUri}{AzPredictorConstants.CommandsEndpoint}?clientType={AzPredictorService.ClientType}&context.versionNumber={azContext.AzVersion}";
103105
_predictionsEndpoint = serviceUri + AzPredictorConstants.PredictionsEndpoint;
104106
_telemetryClient = telemetryClient;

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/IAzPredictorService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface IAzPredictorService
3131
/// <param name="suggestionCount">The number of suggestion to return.</param>
3232
/// <param name="cancellationToken">The cancellation token</param>
3333
/// <param name="maxAllowedCommandDuplicate">The maximum amount of the same commnds in the list of predictions.</param>
34-
/// <returns>The suggestions for <paramref name="context"/>. The maximum number of suggestions is <paramref name="suggestionCount"/>.</returns>
34+
/// <returns>The suggestions for <paramref name="context"/>. The maximum number of suggestions is <paramref name="suggestionCount"/>. A null will be returned if there the user input context isn't valid/supported at all.</returns>
3535
public CommandLineSuggestion GetSuggestion(PredictionContext context, int suggestionCount, int maxAllowedCommandDuplicate, CancellationToken cancellationToken);
3636

3737
/// <summary>

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterValuePredictor.cs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,19 @@
1-
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities;
1+
// ----------------------------------------------------------------------------------
2+
//
3+
// Copyright Microsoft Corporation
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
// ----------------------------------------------------------------------------------
14+
15+
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry;
16+
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities;
217
using System;
318
using System.Collections.Concurrent;
419
using System.Collections.Generic;
@@ -19,20 +34,30 @@ sealed class ParameterValuePredictor
1934

2035
private readonly Dictionary<string, Dictionary<string, string>> _command_param_to_resource_map;
2136

22-
public ParameterValuePredictor()
37+
private ITelemetryClient _telemetryClient;
38+
39+
public ParameterValuePredictor(ITelemetryClient telemetryClient)
2340
{
41+
Validation.CheckArgument(telemetryClient, $"{nameof(telemetryClient)} cannot be null.");
42+
43+
_telemetryClient = telemetryClient;
44+
2445
var fileInfo = new FileInfo(typeof(Settings).Assembly.Location);
2546
var directory = fileInfo.DirectoryName;
2647
var mappingFilePath = Path.Join(directory, "command_param_to_resource_map.json");
48+
Exception exception = null;
2749

2850
try
2951
{
3052
_command_param_to_resource_map = JsonSerializer.Deserialize<Dictionary<string, Dictionary<string, string>>>(File.ReadAllText(mappingFilePath), JsonUtilities.DefaultSerializerOptions);
3153
}
32-
catch
54+
catch (Exception e)
3355
{
3456
// We don't want it to crash the module when the file doesn't exist or when it's mal-formatted.
57+
exception = e;
3558
}
59+
60+
_telemetryClient.OnLoadParameterMap(new ParameterMapTelemetryData(exception));
3661
}
3762

3863
/// <summary>
@@ -80,7 +105,7 @@ public string GetParameterValueFromAzCommand(string commandNoun, string paramete
80105
return null;
81106
}
82107

83-
108+
84109
public static string GetAzCommandNoun(string commandName)
85110
{
86111
var monikerIndex = commandName?.IndexOf(AzPredictorConstants.AzCommandMoniker, StringComparison.OrdinalIgnoreCase);

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,7 @@ public AzPredictorTelemetryClient(IAzContext azContext)
8080
/// <inheritdoc/>
8181
public void OnHistory(HistoryTelemetryData telemetryData)
8282
{
83-
if (!IsDataCollectionAllowed())
84-
{
85-
return;
86-
}
87-
88-
telemetryData.SessionId = SessionId;
89-
telemetryData.CorrelationId = CorrelationId;
90-
91-
_telemetryDispatcher.Post(telemetryData);
83+
PostTelemetryData(telemetryData);
9284

9385
#if TELEMETRY_TRACE && DEBUG
9486
System.Diagnostics.Trace.WriteLine("Recording CommandHistory");
@@ -98,17 +90,7 @@ public void OnHistory(HistoryTelemetryData telemetryData)
9890
/// <inheritdoc/>
9991
public void OnRequestPrediction(RequestPredictionTelemetryData telemetryData)
10092
{
101-
if (!IsDataCollectionAllowed())
102-
{
103-
return;
104-
}
105-
106-
CorrelationId = Guid.NewGuid().ToString();
107-
108-
telemetryData.SessionId = SessionId;
109-
telemetryData.CorrelationId = CorrelationId;
110-
111-
_telemetryDispatcher.Post(telemetryData);
93+
PostTelemetryData(telemetryData);
11294

11395
#if TELEMETRY_TRACE && DEBUG
11496
System.Diagnostics.Trace.WriteLine("Recording RequestPrediction");
@@ -118,15 +100,7 @@ public void OnRequestPrediction(RequestPredictionTelemetryData telemetryData)
118100
/// <inheritdoc/>
119101
public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData)
120102
{
121-
if (!IsDataCollectionAllowed())
122-
{
123-
return;
124-
}
125-
126-
telemetryData.SessionId = SessionId;
127-
telemetryData.CorrelationId = CorrelationId;
128-
129-
_telemetryDispatcher.Post(telemetryData);
103+
PostTelemetryData(telemetryData);
130104

131105
#if TELEMETRY_TRACE && DEBUG
132106
System.Diagnostics.Trace.WriteLine("Recording AcceptSuggestion");
@@ -136,18 +110,20 @@ public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData)
136110
/// <inheritdoc/>
137111
public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData)
138112
{
139-
if (!IsDataCollectionAllowed())
140-
{
141-
return;
142-
}
113+
PostTelemetryData(telemetryData);
143114

144-
telemetryData.SessionId = SessionId;
145-
telemetryData.CorrelationId = CorrelationId;
115+
#if TELEMETRY_TRACE && DEBUG
116+
System.Diagnostics.Trace.WriteLine("Recording GetSuggestion");
117+
#endif
118+
}
146119

147-
_telemetryDispatcher.Post(telemetryData);
120+
/// <inheritdoc/>
121+
public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData)
122+
{
123+
PostTelemetryData(telemetryData);
148124

149125
#if TELEMETRY_TRACE && DEBUG
150-
System.Diagnostics.Trace.WriteLine("Recording GetSuggestion");
126+
System.Diagnostics.Trace.WriteLine("Recording LoadParameterMap");
151127
#endif
152128
}
153129

@@ -165,6 +141,38 @@ private static bool IsDataCollectionAllowed()
165141
return false;
166142
}
167143

144+
/// <summary>
145+
/// Construct a string from an exception and the string is sent to telemetry.
146+
/// The string should have minimum data that can help us to diagnose the exception
147+
/// but not too excessive that may have PII.
148+
/// </summary>
149+
/// <param name="exception">The exception to construct the string from.</param>
150+
/// <returns>A string to send to telemetry.</returns>
151+
private static string FormatException(Exception exception)
152+
{
153+
if (exception == null)
154+
{
155+
return string.Empty;
156+
}
157+
158+
// The exception message may contain data such as file path if it is IO related exception.
159+
// It's this solution to throw the exception, the type and the stack trace only contain information related to the solution.
160+
return string.Format($"Type: {exception.GetType().ToString()}\nStack Trace: {exception.StackTrace?.ToString()}");
161+
; }
162+
163+
private void PostTelemetryData(ITelemetryData telemetryData)
164+
{
165+
if (!IsDataCollectionAllowed())
166+
{
167+
return;
168+
}
169+
170+
telemetryData.SessionId = SessionId;
171+
telemetryData.CorrelationId = CorrelationId;
172+
173+
_telemetryDispatcher.Post(telemetryData);
174+
}
175+
168176
/// <summary>
169177
/// Dispatches <see cref="ITelemetryData"/> according to its implementation.
170178
/// </summary>
@@ -184,6 +192,9 @@ private void DispatchTelemetryData(ITelemetryData telemetryData)
184192
case SuggestionAcceptedTelemetryData suggestionAccepted:
185193
SendTelemetry(suggestionAccepted);
186194
break;
195+
case ParameterMapTelemetryData parameterMap:
196+
SendTelemetry(parameterMap);
197+
break;
187198
default:
188199
throw new NotImplementedException();
189200
}
@@ -210,7 +221,7 @@ private void SendTelemetry(RequestPredictionTelemetryData telemetryData)
210221
var properties = CreateProperties(telemetryData);
211222
properties.Add("Command", telemetryData.Commands ?? string.Empty);
212223
properties.Add("HttpRequestSent", telemetryData.HasSentHttpRequest.ToString(CultureInfo.InvariantCulture));
213-
properties.Add("Exception", telemetryData.Exception?.ToString() ?? string.Empty);
224+
properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception));
214225

215226
_telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/RequestPrediction", properties);
216227
}
@@ -237,7 +248,7 @@ private void SendTelemetry(GetSuggestionTelemetryData telemetryData)
237248
properties.Add("UserInput", maskedUserInput ?? string.Empty);
238249
properties.Add("Suggestion", sourceTexts != null ? JsonSerializer.Serialize(sourceTexts.Zip(suggestionSource).Select((s) => Tuple.Create(s.First, s.Second)), JsonUtilities.TelemetrySerializerOptions) : string.Empty);
239250
properties.Add("IsCancelled", telemetryData.IsCancellationRequested.ToString(CultureInfo.InvariantCulture));
240-
properties.Add("Exception", telemetryData.Exception?.ToString() ?? string.Empty);
251+
properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception));
241252

242253
_telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/GetSuggestion", properties);
243254
}
@@ -258,6 +269,17 @@ private void SendTelemetry(SuggestionAcceptedTelemetryData telemetryData)
258269
_telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/AcceptSuggestion", properties);
259270
}
260271

272+
/// <summary>
273+
/// Sends the telemetry with the parameter map file loading information.
274+
/// </summary>
275+
private void SendTelemetry(ParameterMapTelemetryData telemetryData)
276+
{
277+
var properties = CreateProperties(telemetryData);
278+
properties.Add("Exception", AzPredictorTelemetryClient.FormatException(telemetryData.Exception));
279+
280+
_telemetryClient.TrackEvent($"{AzPredictorTelemetryClient.TelemetryEventPrefix}/LoadParameterMap", properties);
281+
}
282+
261283
/// <summary>
262284
/// Add the common properties to the telemetry event.
263285
/// </summary>

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry
2323
public sealed class GetSuggestionTelemetryData : ITelemetryData
2424
{
2525
/// <inheritdoc/>
26-
public string SessionId { get; internal set; }
26+
string ITelemetryData.SessionId { get; set; }
2727

2828
/// <inheritdoc/>
29-
public string CorrelationId { get; internal set; }
29+
string ITelemetryData.CorrelationId { get; set; }
3030

3131
/// <summary>
3232
/// Gets the user input.

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/HistoryTelemetryData.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Telemetry
2020
public sealed class HistoryTelemetryData : ITelemetryData
2121
{
2222
/// <inheritdoc/>
23-
public string SessionId { get; internal set; }
23+
string ITelemetryData.SessionId { get; set; }
2424

2525
/// <inheritdoc/>
26-
public string CorrelationId { get; internal set; }
26+
string ITelemetryData.CorrelationId { get; set; }
2727

2828
/// <summary>
2929
/// Gets the history command line.

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryClient.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,15 @@ public interface ITelemetryClient
4848
public void OnSuggestionAccepted(SuggestionAcceptedTelemetryData telemetryData);
4949

5050
/// <summary>
51-
/// Collects when we return a suggestion
51+
/// Collects when we return a suggestion.
5252
/// </summary>
5353
/// <param name="telemetryData">The data to collect.</param>
5454
public void OnGetSuggestion(GetSuggestionTelemetryData telemetryData);
55+
56+
/// <summary>
57+
/// Collects when we load parameter map file.
58+
/// </summary>
59+
/// <param name="telemetryData">The data to collect.</param>
60+
public void OnLoadParameterMap(ParameterMapTelemetryData telemetryData);
5561
}
5662
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/ITelemetryData.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ public interface ITelemetryData
2323
/// <summary>
2424
/// Gets the session id.
2525
/// </summary>
26-
string SessionId { get; }
26+
string SessionId { get; internal set; }
2727

2828
/// <summary>
2929
/// Gets the correlation id.
3030
/// </summary>
31-
string CorrelationId { get; }
31+
string CorrelationId { get; internal set; }
3232
}
3333
}

0 commit comments

Comments
 (0)