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

Produce telemetry on yellow triangles in the dependency tree #6593

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ public void PostProperties(string eventName, IEnumerable<(string propertyName, o
var telemetryEvent = new TelemetryEvent(eventName);
foreach ((string propertyName, object propertyValue) in properties)
{
telemetryEvent.Properties.Add(propertyName, propertyValue);
if (propertyValue is ComplexPropertyValue complexProperty)
{
telemetryEvent.Properties.Add(propertyName, new TelemetryComplexProperty(complexProperty.Data));
}
else
{
telemetryEvent.Properties.Add(propertyName, propertyValue);
}
}

PostTelemetryEvent(telemetryEvent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,7 @@ async Task<TreeUpdateResult> UpdateTreeAsync(
{
dependenciesNode = await viewProvider.BuildTreeAsync(dependenciesNode, snapshot, cancellationToken);

if (_treeTelemetryService.IsActive)
{
await _treeTelemetryService.ObserveTreeUpdateCompletedAsync(snapshot.MaximumVisibleDiagnosticLevel != DiagnosticLevel.None);
}
await _treeTelemetryService.ObserveTreeUpdateCompletedAsync(snapshot.MaximumVisibleDiagnosticLevel != DiagnosticLevel.None);
}

return new TreeUpdateResult(dependenciesNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.ComponentModel.Composition;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies.Snapshot;
using Microsoft.VisualStudio.Telemetry;

namespace Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies
Expand All @@ -22,13 +24,14 @@ namespace Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies
/// </remarks>
[Export(typeof(IDependencyTreeTelemetryService))]
[AppliesTo(ProjectCapability.DependenciesTree)]
internal sealed class DependencyTreeTelemetryService : IDependencyTreeTelemetryService
internal sealed class DependencyTreeTelemetryService : IDependencyTreeTelemetryService, IDisposable
{
private const int MaxEventCount = 10;

private readonly UnconfiguredProject _project;
private readonly ITelemetryService? _telemetryService;
private readonly ISafeProjectGuidService _safeProjectGuidService;
private readonly Stopwatch _projectLoadTime = Stopwatch.StartNew();
private readonly object _stateUpdateLock = new object();

/// <summary>
Expand All @@ -39,6 +42,7 @@ internal sealed class DependencyTreeTelemetryService : IDependencyTreeTelemetryS

private string? _projectId;
private int _eventCount;
private DependenciesSnapshot? _dependenciesSnapshot;

[ImportingConstructor]
public DependencyTreeTelemetryService(
Expand All @@ -56,8 +60,6 @@ public DependencyTreeTelemetryService(
}
}

public bool IsActive => _stateByFramework != null;

public void InitializeTargetFrameworkRules(ImmutableArray<TargetFramework> targetFrameworks, IReadOnlyCollection<string> rules)
{
if (_stateByFramework == null)
Expand Down Expand Up @@ -103,11 +105,13 @@ public void ObserveTargetFrameworkRules(TargetFramework targetFramework, IEnumer
}
}

public async Task ObserveTreeUpdateCompletedAsync(bool hasUnresolvedDependency)
public async ValueTask ObserveTreeUpdateCompletedAsync(bool hasUnresolvedDependency)
{
if (_stateByFramework == null)
return;

Assumes.NotNull(_telemetryService);

bool observedAllRules;
lock (_stateUpdateLock)
{
Expand All @@ -126,15 +130,15 @@ public async Task ObserveTreeUpdateCompletedAsync(bool hasUnresolvedDependency)

if (hasUnresolvedDependency)
{
_telemetryService!.PostProperties(TelemetryEventName.TreeUpdatedUnresolved, new[]
_telemetryService.PostProperties(TelemetryEventName.TreeUpdatedUnresolved, new[]
{
(TelemetryPropertyName.TreeUpdatedUnresolvedProject, (object)_projectId),
(TelemetryPropertyName.TreeUpdatedUnresolvedObservedAllRules, observedAllRules)
});
}
else
{
_telemetryService!.PostProperties(TelemetryEventName.TreeUpdatedResolved, new[]
_telemetryService.PostProperties(TelemetryEventName.TreeUpdatedResolved, new[]
{
(TelemetryPropertyName.TreeUpdatedResolvedProject, (object)_projectId),
(TelemetryPropertyName.TreeUpdatedResolvedObservedAllRules, observedAllRules)
Expand All @@ -153,11 +157,83 @@ async Task<string> GetProjectIdAsync()
}
else
{
return _telemetryService!.HashValue(_project.FullPath);
return _telemetryService.HashValue(_project.FullPath);
}
}
}

public void ObserveSnapshot(DependenciesSnapshot dependenciesSnapshot)
{
_dependenciesSnapshot = dependenciesSnapshot;
}

public void Dispose()
{
if (_telemetryService != null && _dependenciesSnapshot != null && _projectId != null)
{
var data = new Dictionary<string, Dictionary<string, DependencyCount>>();
int totalDependencyCount = 0;
int unresolvedDependencyCount = 0;

// Scan the snapshot and tally dependencies
foreach ((TargetFramework targetFramework, TargetedDependenciesSnapshot targetedSnapshot) in _dependenciesSnapshot.DependenciesByTargetFramework)
{
var countsByType = new Dictionary<string, DependencyCount>();

data[targetFramework.ShortName] = countsByType;

foreach (IDependency dependency in targetedSnapshot.Dependencies)
{
// Only include visible dependencies in telemetry counts
if (!dependency.Visible)
{
continue;
}

if (!countsByType.TryGetValue(dependency.ProviderType, out DependencyCount counts))
{
counts = new DependencyCount();
}

countsByType[dependency.ProviderType] = counts.Add(dependency.Resolved);

totalDependencyCount++;

if (!dependency.Resolved)
{
unresolvedDependencyCount++;
}
}
}

_telemetryService.PostProperties(TelemetryEventName.ProjectUnloadDependencies, new (string, object)[]
{
(TelemetryPropertyName.ProjectUnloadDependenciesProject, _projectId),
(TelemetryPropertyName.ProjectUnloadProjectAgeMillis, _projectLoadTime.ElapsedMilliseconds),
drewnoakes marked this conversation as resolved.
Show resolved Hide resolved
(TelemetryPropertyName.ProjectUnloadTotalDependencyCount, totalDependencyCount),
(TelemetryPropertyName.ProjectUnloadUnresolvedDependencyCount, unresolvedDependencyCount),
(TelemetryPropertyName.ProjectUnloadTargetFrameworkCount, _dependenciesSnapshot.DependenciesByTargetFramework.Count),
(TelemetryPropertyName.ProjectUnloadDependencyBreakdown, new ComplexPropertyValue(data))
});
}
}

private readonly struct DependencyCount
{
public int TotalCount { get; }
public int UnresolvedCount { get; }

public DependencyCount(int totalCount, int unresolvedCount)
{
TotalCount = totalCount;
UnresolvedCount = unresolvedCount;
}

public DependencyCount Add(bool isResolved) => new DependencyCount(
TotalCount + 1,
isResolved ? UnresolvedCount : UnresolvedCount + 1);
}

/// <summary>
/// Maintain state for a single target framework.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Immutable;
using System.Threading.Tasks;
using Microsoft.VisualStudio.Composition;
using Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies.Snapshot;

namespace Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies
{
Expand All @@ -13,13 +14,6 @@ namespace Microsoft.VisualStudio.ProjectSystem.Tree.Dependencies
[ProjectSystemContract(ProjectSystemContractScope.UnconfiguredProject, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)]
internal interface IDependencyTreeTelemetryService
{
/// <summary>
/// Gets a value indicating whether this telemetry service is active.
/// If not, then it will remain inactive and no methods need be called on it.
/// Note that an instance may become inactive during its lifetime.
/// </summary>
bool IsActive { get; }

/// <summary>
/// Initialize telemetry state with the set of target frameworks and rules we expect to observe.
/// </summary>
Expand All @@ -36,6 +30,13 @@ internal interface IDependencyTreeTelemetryService
/// Fire telemetry when dependency tree completes an update
/// </summary>
/// <param name="hasUnresolvedDependency">indicates if the snapshot used for the update had any unresolved dependencies</param>
Task ObserveTreeUpdateCompletedAsync(bool hasUnresolvedDependency);
ValueTask ObserveTreeUpdateCompletedAsync(bool hasUnresolvedDependency);

/// <summary>
/// Provides an updated dependency snapshot so that telemetry may be reported about the
/// state of the project's dependencies.
/// </summary>
/// <param name="dependenciesSnapshot">The dependency snapshot.</param>
void ObserveSnapshot(DependenciesSnapshot dependenciesSnapshot);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,23 @@ public SnapshotUpdater(IProjectThreadingService projectThreadingService, Cancell
/// Executes <paramref name="updateFunc"/> on the current snapshot within a lock. If a new snapshot
/// object is returned, <see cref="Current"/> is updated and the update is posted to <see cref="Source"/>.
/// </summary>
public void TryUpdate(Func<DependenciesSnapshot, DependenciesSnapshot> updateFunc, CancellationToken token = default)
/// <returns>The updated snapshot, or <see langword="null" /> if no update occurred.</returns>
public DependenciesSnapshot? TryUpdate(Func<DependenciesSnapshot, DependenciesSnapshot> updateFunc, CancellationToken token = default)
{
if (_isDisposed != 0)
{
throw new ObjectDisposedException(nameof(SnapshotUpdater));
}

DependenciesSnapshot updatedSnapshot;

lock (_lock)
{
DependenciesSnapshot updatedSnapshot = updateFunc(_currentSnapshot);
updatedSnapshot = updateFunc(_currentSnapshot);

if (ReferenceEquals(_currentSnapshot, updatedSnapshot))
{
return;
return null;
}

_currentSnapshot = updatedSnapshot;
Expand All @@ -83,6 +86,8 @@ public void TryUpdate(Func<DependenciesSnapshot, DependenciesSnapshot> updateFun

return Task.CompletedTask;
}, token);

return updatedSnapshot;
}

public void Dispose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ internal sealed partial class DependenciesSnapshotProvider : OnceInitializedOnce
private readonly IUnconfiguredProjectCommonServices _commonServices;
private readonly IUnconfiguredProjectTasksService _tasksService;
private readonly IActiveConfiguredProjectSubscriptionService _activeConfiguredProjectSubscriptionService;
private readonly IDependencyTreeTelemetryService _dependencyTreeTelemetryService;

[ImportMany] private readonly OrderPrecedenceImportCollection<IDependencyCrossTargetSubscriber> _dependencySubscribers;
[ImportMany] private readonly OrderPrecedenceImportCollection<IProjectDependenciesSubTreeProvider> _subTreeProviders;
Expand Down Expand Up @@ -70,13 +71,15 @@ public DependenciesSnapshotProvider(
IUnconfiguredProjectTasksService tasksService,
IActiveConfiguredProjectSubscriptionService activeConfiguredProjectSubscriptionService,
IActiveProjectConfigurationRefreshService activeProjectConfigurationRefreshService,
ITargetFrameworkProvider targetFrameworkProvider)
ITargetFrameworkProvider targetFrameworkProvider,
IDependencyTreeTelemetryService dependencyTreeTelemetryService)
: base(commonServices.ThreadingService.JoinableTaskContext)
{
_commonServices = commonServices;
_tasksService = tasksService;
_activeConfiguredProjectSubscriptionService = activeConfiguredProjectSubscriptionService;
_targetFrameworkProvider = targetFrameworkProvider;
_dependencyTreeTelemetryService = dependencyTreeTelemetryService;

_dependencySubscribers = new OrderPrecedenceImportCollection<IDependencyCrossTargetSubscriber>(
projectCapabilityCheckProvider: commonServices.Project);
Expand Down Expand Up @@ -276,7 +279,7 @@ private void UpdateDependenciesSnapshot(

IImmutableSet<string>? projectItemSpecs = GetProjectItemSpecs(catalogs?.Project?.ProjectInstance.Items);

_snapshot.TryUpdate(
DependenciesSnapshot? updatedSnapshot = _snapshot.TryUpdate(
previousSnapshot => DependenciesSnapshot.FromChanges(
previousSnapshot,
changedTargetFramework,
Expand All @@ -289,6 +292,11 @@ private void UpdateDependenciesSnapshot(
projectItemSpecs),
token);

if (updatedSnapshot != null)
Copy link
Member

Choose a reason for hiding this comment

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

This could result in us sending an incomplete snapshot right? ie You do a big branch switch, close VS before the tree has finished we might send unresolved states that are misleading (ie its just that we never received design-time build data).

Perhaps we should only update the snapshot state when we're up-to-date with the latest design-time build data?

Copy link
Member Author

@drewnoakes drewnoakes Sep 8, 2020

Choose a reason for hiding this comment

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

No, the behaviour is the same as before. updatedSnapshot is only non-null if the TryUpdate method actually made a change in response to the update.

Reading your comment again I think I see what you're saying. I'll take a look to see whether we can only flow this through when we get a JointRule update.

Copy link
Member

Choose a reason for hiding this comment

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

I think its more than when we get a JointRule; we can get inbetween states that might fixed themselves once we've caught up. For example, go change the version number a commonly used package in Package.targets to see this - you'll a bunch of errors that will eventually go away, you don't ever want to report a telemetry event during that because we'll get misleading data (if had let it finish, they would have been resolved).

I think we'll want to make sure that we're up-to-date with the latest version of JointRule or perhaps operation progress. @lifengl Thoughts?

{
_dependencyTreeTelemetryService.ObserveSnapshot(updatedSnapshot);
}

return;

// Gets the set of items defined directly the project, and not included by imports.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information.

namespace Microsoft.VisualStudio.Telemetry
{
/// <summary>
/// Wrapper for complex (non-scalar) property values being reported
/// via <see cref="ITelemetryService"/>.
/// </summary>
internal readonly struct ComplexPropertyValue
{
public object Data { get; }

public ComplexPropertyValue(object data)
{
Data = data;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,25 @@ internal static class TelemetryEventName
public static readonly string DesignTimeBuildComplete = BuildEventName("DesignTimeBuildComplete");

/// <summary>
/// Indicates that .NET Core SDK version.
/// Indicates the .NET Core SDK version.
/// </summary>
public static readonly string SDKVersion = BuildEventName("SDKVersion");

/// <summary>
/// Indicates that the TempPE compilation queue has been processed
/// Indicates that the TempPE compilation queue has been processed.
/// </summary>
public static readonly string TempPEProcessQueue = BuildEventName("TempPE/ProcessCompileQueue");

/// <summary>
/// Indicates that the TempPE compilation has occurred on demand from a designer
/// Indicates that the TempPE compilation has occurred on demand from a designer.
/// </summary>
public static readonly string TempPECompileOnDemand = BuildEventName("TempPE/CompileOnDemand");

/// <summary>
/// Indicates that the summary of a project's dependencies is being reported during project unload.
/// </summary>
public static readonly string ProjectUnloadDependencies = BuildEventName("ProjectUnload/Dependencies");

private static string BuildEventName(string eventName)
{
return Prefix + "/" + eventName.ToLowerInvariant();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,42 @@ internal static class TelemetryPropertyName
/// </summary>
public static readonly string TreeUpdatedUnresolvedProject = BuildPropertyName(TelemetryEventName.TreeUpdatedUnresolved, "Project");

/// <summary>
/// Identifies the project to which data in the telemetry event applies.
/// </summary>
public static readonly string ProjectUnloadDependenciesProject = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "Project");

/// <summary>
/// Identifies the time between project load and unload, in milliseconds.
/// </summary>
public static readonly string ProjectUnloadProjectAgeMillis = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "ProjectAgeMillis");

/// <summary>
/// Identifies the total number of visible dependencies in the project.
/// If a project multi-targets (i.e. <see cref="ProjectUnloadTargetFrameworkCount"/> is greater than one) then the count of dependencies
/// in each target is summed together to produce this single value. If a breakdown is required, <see cref="ProjectUnloadDependencyBreakdown"/>
/// may be used.
/// </summary>
public static readonly string ProjectUnloadTotalDependencyCount = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "TotalDependencyCount");

/// <summary>
/// Identifies the total number of visible unresolved dependencies in the project.
/// If a project multi-targets (i.e. <see cref="ProjectUnloadTargetFrameworkCount"/> is greater than one) then the count of unresolved dependencies
/// in each target is summed together to produce this single value. If a breakdown is required, <see cref="ProjectUnloadDependencyBreakdown"/>
/// may be used.
/// </summary>
public static readonly string ProjectUnloadUnresolvedDependencyCount = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "UnresolvedDependencyCount");

/// <summary>
/// Identifies the number of frameworks this project targets.
/// </summary>
public static readonly string ProjectUnloadTargetFrameworkCount = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "TargetFrameworkCount");

/// <summary>
/// Contains structured data describing the number of total/unresolved dependencies broken down by target framework and dependency type.
/// </summary>
public static readonly string ProjectUnloadDependencyBreakdown = BuildPropertyName(TelemetryEventName.ProjectUnloadDependencies, "DependencyBreakdown");

/// <summary>
/// Indicates whether seen all rules initialized when the dependency tree is updated with all resolved dependencies.
/// </summary>
Expand Down