Formatter and PeekDefinition Telemetry (#245)

- Add telemetry point for format requests - Update PeekDefinition telemetry so that it captures whether the attempt succeeded and if it was connected. This will help us understand whether our current behavior of just notifying success/failure to the output window was useful or not. It will also help us understand whether we're doing a good job implementing this / if we need to improve reliability and preformance.
2026-01-28 17:24:27 -05:00 · 2017-02-21 21:26:10 -08:00
parent 55a56be316
commit e1c8cc5ac3
8 changed files with 210 additions and 18 deletions
--- a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs
@@ -1026,7 +1026,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
                        {
                            Properties = new Dictionary<string, string>
                            {
-                                {"IsAzure", connectionInfo.IsAzure ? "1" : "0"}
+                                { TelemetryPropertyNames.IsAzure, connectionInfo.IsAzure.ToOneOrZeroString() }
                            },
                            EventName = TelemetryEventNames.IntellisenseQuantile,
                            Measures = connectionInfo.IntellisenseMetrics.Quantile
--- a/src/Microsoft.SqlTools.ServiceLayer/Formatter/TSqlFormatterService.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/Formatter/TSqlFormatterService.cs
@@ -13,6 +13,7 @@ using Microsoft.SqlTools.ServiceLayer.Extensibility;
 using Microsoft.SqlTools.ServiceLayer.Formatter.Contracts;
 using Microsoft.SqlTools.ServiceLayer.Hosting;
 using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol;
+using Microsoft.SqlTools.ServiceLayer.LanguageServices;
 using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
 using Microsoft.SqlTools.ServiceLayer.SqlContext;
 using Microsoft.SqlTools.ServiceLayer.Utility;
@@ -40,9 +41,6 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
        {
            serviceHost.SetRequestHandler(DocumentFormattingRequest.Type, HandleDocFormatRequest);
            serviceHost.SetRequestHandler(DocumentRangeFormattingRequest.Type, HandleDocRangeFormatRequest);
-
-
-
            WorkspaceService?.RegisterConfigChangeCallback(HandleDidChangeConfigurationNotification);
        }

@@ -75,6 +73,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
                return FormatAndReturnEdits(docFormatParams);
            };
            await HandleRequest(requestHandler, requestContext, "HandleDocFormatRequest");
+
+            DocumentStatusHelper.SendTelemetryEvent(requestContext, CreateTelemetryProps(isDocFormat: true));
        }

        public async Task HandleDocRangeFormatRequest(DocumentRangeFormattingParams docRangeFormatParams, RequestContext<TextEdit[]> requestContext)
@@ -84,6 +84,20 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
                return FormatRangeAndReturnEdits(docRangeFormatParams);
            };
            await HandleRequest(requestHandler, requestContext, "HandleDocRangeFormatRequest");
+
+            DocumentStatusHelper.SendTelemetryEvent(requestContext, CreateTelemetryProps(isDocFormat: false));
+        }
+        private static TelemetryProperties CreateTelemetryProps(bool isDocFormat)
+        {
+            return new TelemetryProperties
+            {
+                Properties = new Dictionary<string, string>
+                {
+                    { TelemetryPropertyNames.FormatType,
+                        isDocFormat ? TelemetryPropertyNames.DocumentFormatType : TelemetryPropertyNames.RangeFormatType }
+                },
+                EventName = TelemetryEventNames.FormatCode
+            };
        }

        private async Task<TextEdit[]> FormatRangeAndReturnEdits(DocumentRangeFormattingParams docFormatParams)
@@ -141,7 +155,6 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
            }
            UpdateFormatOptionsFromSettings(options, settings);
            return options;
-
        }

        internal static void UpdateFormatOptionsFromSettings(FormatOptions options, FormatterSettings settings)
--- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Contracts/TelemetryNotification.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Contracts/TelemetryNotification.cs
@@ -52,8 +52,49 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts
        public const string IntellisenseQuantile = "IntellisenseQuantile";

        /// <summary>
-        /// telemetry even name for when definition is requested
+        /// telemetry event name for when definition is requested
        /// </summary>
        public const string PeekDefinitionRequested = "PeekDefinitionRequested";
-    } 
+
+        /// <summary>
+        /// telemetry event name for when definition is requested
+        /// </summary>
+        public const string FormatCode = "FormatCode";
+    }
+
+    /// <summary>
+    ///  List of properties used in telemetry events
+    /// </summary>
+    public static class TelemetryPropertyNames
+    {
+        /// <summary>
+        /// Is a connection to an Azure database or not
+        /// </summary>
+        public const string IsAzure = "IsAzure";
+
+        /// <summary>
+        /// Did an event succeed or not
+        /// </summary>
+        public const string Succeeded = "Succeeded";
+
+        /// <summary>
+        /// Was the action against a connected file or similar resource, or not
+        /// </summary>
+        public const string Connected = "Connected";
+
+        /// <summary>
+        /// Format type property - should be one of <see cref="DocumentFormatType"/> or <see cref="RangeFormatType"/>
+        /// </summary>
+        public const string FormatType = "FormatType";
+
+        /// <summary>
+        /// A full document format
+        /// </summary>
+        public const string DocumentFormatType = "Document";
+
+        /// <summary>
+        /// A document range format
+        /// </summary>
+        public const string RangeFormatType = "Range";
+    }
 }
--- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DocumentStatusHelper.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DocumentStatusHelper.cs
@@ -4,9 +4,9 @@
 //

 using System.Threading.Tasks;
-using Microsoft.SqlTools.ServiceLayer.Hosting;
 using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol;
 using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
+using Microsoft.SqlTools.ServiceLayer.Utility;
 using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;

 namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
@@ -43,6 +43,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
        /// </summary>
        public static void SendTelemetryEvent<T>(RequestContext<T> requestContext, string telemetryEvent)
        {
+            Validate.IsNotNull(nameof(requestContext), requestContext);
+            Validate.IsNotNullOrWhitespaceString(nameof(telemetryEvent), telemetryEvent);
            Task.Factory.StartNew(async () =>
            {
                await requestContext.SendEvent(TelemetryNotification.Type, new TelemetryParams()
@@ -54,5 +56,22 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                });
            });
        }
+
+        /// <summary>
+        /// Sends a telemetry event for specific document using the existing request context
+        /// </summary>
+        public static void SendTelemetryEvent<T>(RequestContext<T> requestContext, TelemetryProperties telemetryProps)
+        {
+            Validate.IsNotNull(nameof(requestContext), requestContext);
+            Validate.IsNotNull(nameof(telemetryProps), telemetryProps);
+            Validate.IsNotNullOrWhitespaceString("telemetryProps.EventName", telemetryProps.EventName);
+            Task.Factory.StartNew(async () =>
+            {
+                await requestContext.SendEvent(TelemetryNotification.Type, new TelemetryParams()
+                {
+                    Params = telemetryProps
+                });
+            });
+        }
    }
 }
--- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs
@@ -312,7 +312,6 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                    EventName = TelemetryEventNames.PeekDefinitionRequested
                }
            });
-            DocumentStatusHelper.SendTelemetryEvent(requestContext, TelemetryEventNames.PeekDefinitionRequested);
            DocumentStatusHelper.SendStatusChange(requestContext, textDocumentPosition, DocumentStatusHelper.DefinitionRequested);

            if (WorkspaceService<SqlToolsSettings>.Instance.CurrentSettings.IsIntelliSenseEnabled)
@@ -320,7 +319,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                // Retrieve document and connection
                ConnectionInfo connInfo;
                var scriptFile = LanguageService.WorkspaceServiceInstance.Workspace.GetFile(textDocumentPosition.TextDocument.Uri);
-                LanguageService.ConnectionServiceInstance.TryFindConnection(scriptFile.ClientFilePath, out connInfo);
+                bool isConnected = LanguageService.ConnectionServiceInstance.TryFindConnection(scriptFile.ClientFilePath, out connInfo);
+                bool succeeded = false;
                DefinitionResult definitionResult = LanguageService.Instance.GetDefinition(textDocumentPosition, scriptFile, connInfo);
                if (definitionResult != null)
                {
@@ -331,12 +331,29 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                    else
                    {
                        await requestContext.SendResult(definitionResult.Locations);
+                        succeeded = true;
                    }
                }
+
+                DocumentStatusHelper.SendTelemetryEvent(requestContext, CreatePeekTelemetryProps(succeeded, isConnected));
            }
+
            DocumentStatusHelper.SendStatusChange(requestContext, textDocumentPosition, DocumentStatusHelper.DefinitionRequestCompleted);
        }

+        private static TelemetryProperties CreatePeekTelemetryProps(bool succeeded, bool connected)
+        {
+            return new TelemetryProperties
+            {
+                Properties = new Dictionary<string, string>
+                {
+                    { TelemetryPropertyNames.Succeeded, succeeded.ToOneOrZeroString() },
+                    { TelemetryPropertyNames.Connected, connected.ToOneOrZeroString() }
+                },
+                EventName = TelemetryEventNames.PeekDefinitionRequested
+            };
+        }
+
 // turn off this code until needed (10/28/2016)
 #if false
        private static async Task HandleReferencesRequest(
--- a/src/Microsoft.SqlTools.ServiceLayer/Utility/Extensions.cs
+++ b/src/Microsoft.SqlTools.ServiceLayer/Utility/Extensions.cs
@@ -30,5 +30,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Utility

            return str;
        }
+
+        /// <summary>
+        /// Converts a boolean to a "1" or "0" string. Particularly helpful when sending telemetry
+        /// </summary>
+        public static string ToOneOrZeroString(this bool isTrue)
+        {
+            return isTrue ? "1" : "0";
+        }
    }
 }
--- a/test/Microsoft.SqlTools.ServiceLayer.Test/Formatter/TSqlFormatterServiceTests.cs
+++ b/test/Microsoft.SqlTools.ServiceLayer.Test/Formatter/TSqlFormatterServiceTests.cs
@@ -5,8 +5,10 @@

 using System;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.SqlTools.ServiceLayer.Formatter.Contracts;
+using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol;
 using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
 using Microsoft.SqlTools.ServiceLayer.Test.Common;
 using Microsoft.SqlTools.ServiceLayer.Test.Utility;
@@ -21,6 +23,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Formatter
        private Mock<ServiceLayer.Workspace.Workspace> workspaceMock = new Mock<ServiceLayer.Workspace.Workspace>();
        private TextDocumentIdentifier textDocument;
        DocumentFormattingParams docFormatParams;
+        DocumentRangeFormattingParams rangeFormatParams;

        public TSqlFormatterServiceTests()
        {
@@ -33,6 +36,17 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Formatter
                TextDocument = textDocument,
                Options = new FormattingOptions() { InsertSpaces = true, TabSize = 4 }
            };
+            rangeFormatParams = new DocumentRangeFormattingParams()
+            {
+                TextDocument = textDocument,
+                Options = new FormattingOptions() { InsertSpaces = true, TabSize = 4 },
+                Range = new ServiceLayer.Workspace.Contracts.Range()
+                {
+                    // From first "(" to last ")"
+                    Start = new Position { Line = 0, Character = 16 },
+                    End = new Position { Line = 0, Character = 56 }
+                }
+            };
        }

        private string defaultSqlContents = TestUtilities.NormalizeLineEndings(@"create TABLE T1 ( C1 int NOT NULL, C2 nvarchar(50) NULL)");
@@ -56,10 +70,94 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Formatter
                    // Then expect a single edit to be returned and for it to match the standard formatting
                    Assert.Equal(1, edits.Length);
                    AssertFormattingEqual(formattedSqlContents, edits[0].NewText);
-
                }));
        }

+        [Fact]
+        public async Task FormatDocumentTelemetryShouldIncludeFormatTypeProperty()
+        {
+            await RunAndVerifyTelemetryTest(
+                // Given a document that we want to format
+                preRunSetup: () => SetupScriptFile(defaultSqlContents),
+                // When format document is called
+                test: (requestContext) => FormatterService.HandleDocFormatRequest(docFormatParams, requestContext),
+                verify: (result, actualParams) =>
+                {
+                    // Then expect a telemetry event to have been sent with the right format definition
+                    Assert.NotNull(actualParams);
+                    Assert.Equal(TelemetryEventNames.FormatCode, actualParams.Params.EventName);
+                    Assert.Equal(TelemetryPropertyNames.DocumentFormatType, actualParams.Params.Properties[TelemetryPropertyNames.FormatType]);
+                });
+        }
+
+        [Fact]
+        public async Task FormatRangeTelemetryShouldIncludeFormatTypeProperty()
+        {
+            await RunAndVerifyTelemetryTest(
+                // Given a document that we want to format
+                preRunSetup: () => SetupScriptFile(defaultSqlContents),
+                // When format range is called
+                test: (requestContext) => FormatterService.HandleDocRangeFormatRequest(rangeFormatParams, requestContext),
+                verify: (result, actualParams) =>
+                {
+                    // Then expect a telemetry event to have been sent with the right format definition
+                    Assert.NotNull(actualParams);
+                    Assert.Equal(TelemetryEventNames.FormatCode, actualParams.Params.EventName);
+                    Assert.Equal(TelemetryPropertyNames.RangeFormatType, actualParams.Params.Properties[TelemetryPropertyNames.FormatType]);
+
+                    // And expect range to have been correctly formatted
+                    Assert.Equal(1, result.Length);
+                    AssertFormattingEqual(formattedSqlContents, result[0].NewText);
+                });
+        }
+
+        private async Task RunAndVerifyTelemetryTest(
+            Action preRunSetup,
+            Func<RequestContext<TextEdit[]>, Task> test,
+            Action<TextEdit[], TelemetryParams> verify)
+        {
+            SemaphoreSlim semaphore = new SemaphoreSlim(0, 1);
+            TelemetryParams actualParams = null;
+            TextEdit[] result = null;
+            var contextMock = RequestContextMocks.Create<TextEdit[]>(r =>
+            {
+                result = r;
+            })
+            .AddErrorHandling(null)
+            .AddEventHandling(TelemetryNotification.Type, (e, p) =>
+            {
+                actualParams = p;
+                semaphore.Release();
+            });
+
+            // Given a document that we want to format
+            SetupScriptFile(defaultSqlContents);
+
+            // When format document is called
+            await RunAndVerify<TextEdit[]>(
+                test: test,
+                contextMock: contextMock,
+                verify: () =>
+                {
+                    // Wait for the telemetry notification to be processed on a background thread
+                    semaphore.Wait(TimeSpan.FromSeconds(10));
+                    verify(result, actualParams);
+                });
+        }
+
+        public static async Task RunAndVerify<T>(Func<RequestContext<T>, Task> test, Mock<RequestContext<T>> contextMock, Action verify)
+        {
+            await test(contextMock.Object);
+            VerifyResult(contextMock, verify);
+        }
+        
+        public static void VerifyResult<T>(Mock<RequestContext<T>> contextMock, Action verify)
+        {
+            contextMock.Verify(c => c.SendResult(It.IsAny<T>()), Times.Once);
+            contextMock.Verify(c => c.SendError(It.IsAny<string>()), Times.Never);
+            verify();
+        }
+
        private static void AssertFormattingEqual(string expected, string actual)
        {
            if (string.Compare(expected, actual) != 0)
--- a/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/PeekDefinitionTests.cs
+++ b/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/PeekDefinitionTests.cs
@@ -3,12 +3,10 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 using System;
-using System.IO;
 using System.Collections.Generic;
-using System.Threading;
-using System.Threading.Tasks;
+using System.IO;
 using System.Runtime.InteropServices;
-using Microsoft.SqlServer.Management.Common;
+using System.Threading.Tasks;
 using Microsoft.SqlServer.Management.SqlParser.Binder;
 using Microsoft.SqlServer.Management.SqlParser.Intellisense;
 using Microsoft.SqlServer.Management.SqlParser.MetadataProvider;
@@ -18,16 +16,14 @@ using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol;
 using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol.Contracts;
 using Microsoft.SqlTools.ServiceLayer.LanguageServices;
 using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
-using Microsoft.SqlTools.ServiceLayer.SqlContext;
 using Microsoft.SqlTools.ServiceLayer.QueryExecution;
-using Microsoft.SqlTools.ServiceLayer.Test.QueryExecution;
+using Microsoft.SqlTools.ServiceLayer.SqlContext;
 using Microsoft.SqlTools.ServiceLayer.Workspace;
 using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
 using Microsoft.SqlTools.Test.Utility;
 using Moq;
 using Xunit;
 using Location = Microsoft.SqlTools.ServiceLayer.Workspace.Contracts.Location;
-using Microsoft.SqlTools.ServiceLayer.Test.Common;

 namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices
 {