Skip to content

Commit

Permalink
improve telemetry (#13025)
Browse files Browse the repository at this point in the history
* Correlate the telemetry event we send.

- We have these telemetry entry points when we provide suggestion, when
  a suggestion is accepted, and when the user executes some command. We
  add a SuggestionId to the telemetry events so that we can correlate
  those three events. Doing that we know what suggestions are provided,
  accepted, and executed.

* Collect and correlate our telemetry event.

- With this change, we have these telemetry events;
  * RequestPrediction
  * GetSuggestion
  * AcceptSuggestion
  * CommandHistory
- The events can be correlated by the session id and correlate id that
  are associate with the events.

* Fix the request body.

* Rename the prediction source.

* Collect telemetry when an error occurs.

* Remove some PII field

* Collect user input in GetSuggestion event.

* Use a common telemetry event prefix.

* Remove the .gitignore that shouldn't be added.

* Clean up the code.
  • Loading branch information
kceiw authored Sep 24, 2020
1 parent 57cab6d commit ce42552
Show file tree
Hide file tree
Showing 14 changed files with 283 additions and 170 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class AzPredictorServiceTests
public AzPredictorServiceTests(ModelFixture fixture)
{
this._fixture = fixture;
var startHistory = $"{AzPredictorConstants.CommandHistoryPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandHistoryPlaceholder}";
var startHistory = $"{AzPredictorConstants.CommandPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandPlaceholder}";
this._suggestionsPredictor = new Predictor(this._fixture.PredictionCollection[startHistory], null);
this._commandsPredictor = new Predictor(this._fixture.CommandCollection, null);

Expand Down Expand Up @@ -66,7 +66,7 @@ public void VerifyUsingSuggestion(string userInput)
Assert.NotNull(actual);
Assert.NotNull(actual.Item1);
Assert.Equal(expected, actual.Item1);
Assert.Equal(PredictionSource.CurrentHistory, actual.Item2);
Assert.Equal(PredictionSource.CurrentCommand, actual.Item2);
}

/// <summary>
Expand All @@ -83,14 +83,14 @@ public void VerifyUsingCommand(string userInput)
Assert.NotNull(actual);
Assert.NotNull(actual.Item1);
Assert.Equal(expected, actual.Item1);
Assert.Equal(PredictionSource.Commands, actual.Item2);
Assert.Equal(PredictionSource.StaticCommands, actual.Item2);
}

/// <summary>
/// Verify that no prediction for the user input, meaning it's not in the prediction list or the command list.
/// </summary>
[Theory]
[InlineData(AzPredictorConstants.CommandHistoryPlaceholder)]
[InlineData(AzPredictorConstants.CommandPlaceholder)]
[InlineData("git status")]
[InlineData("Get-ChildItem")]
[InlineData("new-azresourcegroup -NoExistingParam")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public sealed class AzPredictorTests
public AzPredictorTests(ModelFixture modelFixture)
{
this._fixture = modelFixture;
var startHistory = $"{AzPredictorConstants.CommandHistoryPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandHistoryPlaceholder}";
var startHistory = $"{AzPredictorConstants.CommandPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandPlaceholder}";

this._service = new MockAzPredictorService(startHistory, this._fixture.PredictionCollection[startHistory], this._fixture.CommandCollection);
this._telemetryClient = new MockAzPredictorTelemetryClient();
Expand Down Expand Up @@ -69,7 +69,7 @@ public void VerifyWithNonSupportedCommand(string historyLine)
this._azPredictor.StartEarlyProcessing(history);

Assert.True(this._service.IsPredictionRequested);
Assert.Null(this._telemetryClient.RecordedSuggestion);
Assert.NotNull(this._telemetryClient.RecordedSuggestion);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ sealed class MockAzPredictorService : AzPredictorService
/// <param name="commands">The commands collection</param>
public MockAzPredictorService(string history, IList<string> suggestions, IList<string> commands)
{
SetHistory(history);
SetPredictionCommand(history);
SetCommandsPredictor(commands);
SetSuggestionPredictor(history, suggestions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Collections.Generic;

namespace Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks
Expand All @@ -21,34 +22,49 @@ sealed class MockAzPredictorTelemetryClient : ITelemetryClient
public class RecordedSuggestionForHistory
{
public string HistoryLine { get; set; }
public int? SuggestionIndex { get; set; }
public int? FallbackIndex { get; set; }
public IEnumerable<string> TopSuggestions { get; set; }
}

/// <inheritdoc/>
public string SessionId { get; } = Guid.NewGuid().ToString();

/// <inheritdoc/>
public string CorrelationId { get; private set; } = Guid.NewGuid().ToString();

public RecordedSuggestionForHistory RecordedSuggestion { get; set; }
public int SuggestionAccepted { get; set; }

/// <inheritdoc/>
public void OnSuggestionForHistory(string historyLine, int? suggestionIndex, int? fallbackIndex, IEnumerable<string> topSuggestions)
public void OnHistory(string historyLine)
{
this.RecordedSuggestion = new RecordedSuggestionForHistory()
{
HistoryLine = historyLine,
SuggestionIndex = suggestionIndex,
FallbackIndex = fallbackIndex,
TopSuggestions = topSuggestions
};
}

/// <inheritdoc/>
public void OnRequestPrediction(string command)
{
}

/// <inheritdoc/>
public void OnRequestPredictionError(string command, Exception e)
{
}

/// <inheritdoc/>
public void OnSuggestionAccepted(string acceptedSuggestion)
{
++this.SuggestionAccepted;
}

/// <inheritdoc/>
public void OnGetSuggestion(PredictionSource predictionSource)
public void OnGetSuggestion(string maskedUserInput, IEnumerable<Tuple<string, PredictionSource>> suggestions, bool isCancelled)
{
}

/// <inheritdoc/>
public void OnGetSuggestionError(Exception e)
{
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class PredictorTests
public PredictorTests(ModelFixture fixture)
{
this._fixture = fixture;
var startHistory = $"{AzPredictorConstants.CommandHistoryPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandHistoryPlaceholder}";
var startHistory = $"{AzPredictorConstants.CommandPlaceholder}{AzPredictorConstants.CommandConcatenator}{AzPredictorConstants.CommandPlaceholder}";
this._predictor = new Predictor(this._fixture.PredictionCollection[startHistory], null);
}

Expand All @@ -44,7 +44,7 @@ public PredictorTests(ModelFixture fixture)
[InlineData("NEW-AZCONTEXT")]
[InlineData("Get-AzStorageAccount ")] // A complete command and we have exact the same on in the model.
[InlineData("get-azaccount ")]
[InlineData(AzPredictorConstants.CommandHistoryPlaceholder)]
[InlineData(AzPredictorConstants.CommandPlaceholder)]
[InlineData("git status")]
[InlineData("Get-ChildItem")]
public void GetNullPredictionWithCommandName(string userInput)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
<GenerateSerializationAssemblies>Auto</GenerateSerializationAssemblies>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
<DefineConstants>DEBUG;TRACE</DefineConstants>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.ApplicationInsights" Version="2.14.0" />
<PackageReference Include="System.Management.Automation" Version="7.1.0-preview.7" />
Expand Down
47 changes: 25 additions & 22 deletions tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void StartEarlyProcessing(IReadOnlyList<string> history)

while (historyLines.Count() < AzPredictorConstants.CommandHistoryCountToProcess)
{
historyLines = historyLines.Prepend(AzPredictorConstants.CommandHistoryPlaceholder);
historyLines = historyLines.Prepend(AzPredictorConstants.CommandPlaceholder);
}

var commandAsts = historyLines.Select((h) =>
Expand All @@ -93,23 +93,13 @@ public void StartEarlyProcessing(IReadOnlyList<string> history)

if (!_service.IsSupportedCommand(commandName))
{
return AzPredictorConstants.CommandHistoryPlaceholder;
return AzPredictorConstants.CommandPlaceholder;
}

return AzPredictor.MaskCommandLine(c);
});

var lastMaskedHistoryLines = maskedHistoryLines.Last();

if (lastMaskedHistoryLines != AzPredictorConstants.CommandHistoryPlaceholder)
{
var commandName = (commandAsts.LastOrDefault()?.CommandElements?.FirstOrDefault() as StringConstantExpressionAst)?.Value;
var suggestionIndex = _service.GetRankOfSuggestion(commandName);
var fallbackIndex = _service.GetRankOfFallback(commandName);
var topFiveSuggestion = _service.GetTopNSuggestions(AzPredictor.SuggestionCountForTelemetry);
_telemetryClient.OnSuggestionForHistory(maskedHistoryLines.LastOrDefault(), suggestionIndex, fallbackIndex, topFiveSuggestion);
}

_telemetryClient.OnHistory(maskedHistoryLines.Last());
_service.RecordHistory(commandAsts);
_service.RequestPredictions(maskedHistoryLines);
}
Expand All @@ -131,17 +121,30 @@ public List<PredictiveSuggestion> GetSuggestion(PredictionContext context, Cance
// is prefixed with `userInput`, it should be ordered before result that is not prefixed
// with `userInput`.

var userInput = context.InputAst.Extent.Text;
var result = _service.GetSuggestion(context.InputAst, cancellationToken);
Tuple<string, PredictionSource> result = Tuple.Create<string, PredictionSource>(null, PredictionSource.None);

if (result?.Item1 != null)
try
{
cancellationToken.ThrowIfCancellationRequested();
var fullSuggestion = MergeStrings(userInput, result.Item1);
return new List<PredictiveSuggestion>() { new PredictiveSuggestion(fullSuggestion) };
}
result = _service.GetSuggestion(context.InputAst, cancellationToken);

this._telemetryClient.OnGetSuggestion(result?.Item2 ?? PredictionSource.None);
if (result?.Item1 != null)
{
cancellationToken.ThrowIfCancellationRequested();
var userInput = context.InputAst.Extent.Text;
var fullSuggestion = MergeStrings(userInput, result.Item1);
return new List<PredictiveSuggestion>() { new PredictiveSuggestion(fullSuggestion) };
}
}
catch (Exception e) when (!(e is OperationCanceledException))
{
this._telemetryClient.OnGetSuggestionError(e);
}
finally
{
var maskedCommandLine = MaskCommandLine(context.InputAst.FindAll((ast) => ast is CommandAst, true).LastOrDefault() as CommandAst);
_telemetryClient.OnGetSuggestion(maskedCommandLine, new Tuple<string, PredictionSource>[] { result },
cancellationToken.IsCancellationRequested);
}

return null;
}
Expand Down Expand Up @@ -228,7 +231,7 @@ public void OnImport()
{
var settings = Settings.GetSettings();
var telemetryClient = new AzPredictorTelemetryClient();
var azPredictorService = new AzPredictorService(settings.ServiceUri);
var azPredictorService = new AzPredictorService(settings.ServiceUri, telemetryClient);
var predictor = new AzPredictor(azPredictorService, telemetryClient);
SubsystemManager.RegisterSubsystem<ICommandPredictor, AzPredictor>(predictor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal static class AzPredictorConstants
/// <summary>
/// The value to use when the command isn't an Az command.
/// </summary>
public const string CommandHistoryPlaceholder = "start_of_snippet";
public const string CommandPlaceholder = "start_of_snippet";

/// <summary>
/// The value to check to determine if it's an Az command.
Expand Down
Loading

0 comments on commit ce42552

Please sign in to comment.