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

Add metrics for datacollector.exe - provides information about profilers #2705

Merged
merged 12 commits into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from 8 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
2 changes: 1 addition & 1 deletion scripts/build/TestPlatform.Dependencies.props
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<JsonNetVersion>9.0.1</JsonNetVersion>
<MoqVersion>4.7.63</MoqVersion>
<TestPlatformExternalsVersion>16.9.0-preview-4267359</TestPlatformExternalsVersion>
<CodeCoverageExternalsVersion>16.9.0-beta.21064.1</CodeCoverageExternalsVersion>
<CodeCoverageExternalsVersion>16.9.0-beta.21072.1</CodeCoverageExternalsVersion>
<MicrosoftFakesVersion>16.9.0-beta.20628.1</MicrosoftFakesVersion>

<MicrosoftBuildPackageVersion>16.0.461</MicrosoftBuildPackageVersion>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollection
{
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Runtime.Serialization;

/// <summary>
/// Payload object that is used to exchange data between datacollector process and runner process.
/// </summary>
[DataContract]
public class AfterTestRunEndResult
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we require this to be public?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. When tried internal than I got:
Severity Code Description Project File Line Suppression State
Error CS0050 Inconsistent accessibility: return type 'AfterTestRunEndResult' is less accessible than method 'DataCollectionRequestSender.SendAfterTestRunEndAndGetResult(ITestMessageEventHandler, bool)' Microsoft.TestPlatform.CommunicationUtilities (net451), Microsoft.TestPlatform.CommunicationUtilities (netstandard1.3), Microsoft.TestPlatform.CommunicationUtilities (netstandard2.0) D:\vstest\src\Microsoft.TestPlatform.CommunicationUtilities\DataCollectionRequestSender.cs 153 Active

SendAfterTestRunEndAndGetResult is part of interface so it cannot be done internal

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that is sad. Adding @cvpoienaru to ensure that we aren't making/breaking any compat promises with this. From what I know, I think we are good.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've already discussed it with @cvpoienaru

{
/// <summary>
/// Initializes a new instance of the <see cref="AfterTestRunEndResult"/> class.
/// </summary>
/// <param name="environmentVariables">
/// The collection of attachment sets.
/// </param>
/// <param name="dataCollectionEventsPort">
jakubch1 marked this conversation as resolved.
Show resolved Hide resolved
/// The metrics.
/// </param>
public AfterTestRunEndResult(Collection<AttachmentSet> attachmentSets, IDictionary<string, object> metrics)
{
this.AttachmentSets = attachmentSets;
this.Metrics = metrics;
}

/// <summary>
/// Gets the environment variable dictionary.
jakubch1 marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
[DataMember]
public Collection<AttachmentSet> AttachmentSets { get; private set; }

/// <summary>
/// Get the Metrics
/// </summary>
[DataMember]
public IDictionary<string, object> Metrics { get; private set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollector
using System.Collections.ObjectModel;
using System.Globalization;
using System.Linq;

using Microsoft.VisualStudio.TestPlatform.Common.DataCollector.Interfaces;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
using Microsoft.VisualStudio.TestPlatform.Common.Logging;
using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
Expand Down Expand Up @@ -61,13 +61,18 @@ internal class DataCollectionManager : IDataCollectionManager
/// </summary>
private DataCollectorExtensionManager dataCollectorExtensionManager;

/// <summary>
/// Request data
/// </summary>
private IDataCollectionTelemetryManager dataCollectionTelemetryManager;

/// <summary>
/// Initializes a new instance of the <see cref="DataCollectionManager"/> class.
/// </summary>
/// <param name="messageSink">
/// The message Sink.
/// </param>
internal DataCollectionManager(IMessageSink messageSink) : this(new DataCollectionAttachmentManager(), messageSink)
internal DataCollectionManager(IMessageSink messageSink, IRequestData requestData) : this(new DataCollectionAttachmentManager(), messageSink, new DataCollectionTelemetryManager(requestData))
{
}

Expand All @@ -83,13 +88,14 @@ internal DataCollectionManager(IMessageSink messageSink) : this(new DataCollecti
/// <remarks>
/// The constructor is not public because the factory method should be used to get instances of this class.
/// </remarks>
protected DataCollectionManager(IDataCollectionAttachmentManager datacollectionAttachmentManager, IMessageSink messageSink)
protected DataCollectionManager(IDataCollectionAttachmentManager datacollectionAttachmentManager, IMessageSink messageSink, IDataCollectionTelemetryManager dataCollectionTelemetryManager)
{
this.attachmentManager = datacollectionAttachmentManager;
this.messageSink = messageSink;
this.events = new TestPlatformDataCollectionEvents();
this.dataCollectorExtensionManager = null;
this.RunDataCollectors = new Dictionary<Type, DataCollectorInformation>();
this.dataCollectionTelemetryManager = dataCollectionTelemetryManager;
}

/// <summary>
Expand Down Expand Up @@ -128,15 +134,15 @@ private DataCollectorExtensionManager DataCollectorExtensionManager
/// <returns>
/// The <see cref="DataCollectionManager"/>.
/// </returns>
public static DataCollectionManager Create(IMessageSink messageSink)
public static DataCollectionManager Create(IMessageSink messageSink, IRequestData requestData)
{
if (Instance == null)
{
lock (syncObject)
{
if (Instance == null)
{
Instance = new DataCollectionManager(messageSink);
Instance = new DataCollectionManager(messageSink, requestData);
}
}
}
Expand Down Expand Up @@ -673,6 +679,8 @@ private void AddCollectorEnvironmentVariables(
dataCollectionWrapper.Logger.LogError(this.dataCollectionEnvironmentContext.SessionDataCollectionContext, message);
}
}

dataCollectionTelemetryManager.RecordEnvironmentVariableConflict(dataCollectionWrapper, namevaluepair.Key, namevaluepair.Value, alreadyRequestedVariable.Value);
}
else
{
Expand All @@ -685,6 +693,8 @@ private void AddCollectorEnvironmentVariables(
dataCollectorEnvironmentVariables.Add(
namevaluepair.Key,
new DataCollectionEnvironmentVariable(namevaluepair, collectorFriendlyName));

dataCollectionTelemetryManager.RecordEnvironmentVariableAddition(dataCollectionWrapper, namevaluepair.Key, namevaluepair.Value);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.VisualStudio.TestPlatform.Common.DataCollector.Interfaces;
using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using System;
using System.Linq;

namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollector
{
/// <summary>
/// Stores and provides telemetry information for data collection.
/// </summary>
internal class DataCollectionTelemetryManager : IDataCollectionTelemetryManager
AbhitejJohn marked this conversation as resolved.
Show resolved Hide resolved
{
private const string CorProfilerVariable = "COR_PROFILER";
private const string CoreClrProfilerVariable = "CORECLR_PROFILER";
private const string ClrIeInstrumentationMethodConfigurationPrefix32Variable = "MicrosoftInstrumentationEngine_ConfigPath32_";
private const string ClrIeInstrumentationMethodConfigurationPrefix64Variable = "MicrosoftInstrumentationEngine_ConfigPath64_";

private const string VanguardProfilerGuid = "{e5f256dc-7959-4dd6-8e4f-c11150ab28e0}";
private const string ClrIeProfilerGuid = "{324f817a-7420-4e6d-b3c1-143fbed6d855}";
private const string IntellitraceProfilerGuid = "{9317ae81-bcd8-47b7-aaa1-a28062e41c71}";

private const string VanguardProfilerName = "vanguard";
private const string ClrIeProfilerName = "clrie";
private const string IntellitraceProfilerName = "intellitrace";
private const string UnknownProfilerName = "unknown";
private const string OverwrittenProfilerName = "overwritten";

private readonly IRequestData requestData;

internal DataCollectionTelemetryManager(IRequestData requestData)
{
this.requestData = requestData;
}

/// <inheritdoc/>
public void RecordEnvironmentVariableAddition(DataCollectorInformation dataCollectorInformation, string name, string value)
{
RecordProfilerMetricForNewVariable(CorProfilerVariable, TelemetryDataConstants.DataCollectorsCorProfiler, dataCollectorInformation, name, value);
RecordProfilerMetricForNewVariable(CoreClrProfilerVariable, TelemetryDataConstants.DataCollectorsCoreClrProfiler, dataCollectorInformation, name, value);
}

/// <inheritdoc/>
public void RecordEnvironmentVariableConflict(DataCollectorInformation dataCollectorInformation, string name, string value, string existingValue)
{
RecordProfilerMetricForConflictedVariable(CorProfilerVariable, TelemetryDataConstants.DataCollectorsCorProfiler, dataCollectorInformation, name, value, existingValue);
RecordProfilerMetricForConflictedVariable(CoreClrProfilerVariable, TelemetryDataConstants.DataCollectorsCoreClrProfiler, dataCollectorInformation, name, value, existingValue);
}

private void RecordProfilerMetricForNewVariable(string profilerVariable, string telemetryPrefix, DataCollectorInformation dataCollectorInformation, string name, string value)
{
if (!string.Equals(profilerVariable, name, StringComparison.Ordinal))
{
return;
}

requestData.MetricsCollection.Add(GetTelemetryKey(telemetryPrefix, dataCollectorInformation), GetProfilerName(value));
}

private void RecordProfilerMetricForConflictedVariable(string profilerVariable, string telemetryPrefix, DataCollectorInformation dataCollectorInformation, string name, string value, string existingValue)
{
// If data collector is requesting same profiler record it same as new
if (string.Equals(value, existingValue, StringComparison.OrdinalIgnoreCase))
{
RecordProfilerMetricForNewVariable(profilerVariable, telemetryPrefix, dataCollectorInformation, name, value);
return;
}

if (!string.Equals(profilerVariable, name, StringComparison.Ordinal))
{
return;
}

if (string.Equals(existingValue, ClrIeProfilerGuid, StringComparison.OrdinalIgnoreCase))
{
if (dataCollectorInformation.TestExecutionEnvironmentVariables != null &&
dataCollectorInformation.TestExecutionEnvironmentVariables.Any(pair => pair.Key.StartsWith(ClrIeInstrumentationMethodConfigurationPrefix32Variable)) &&
jakubch1 marked this conversation as resolved.
Show resolved Hide resolved
dataCollectorInformation.TestExecutionEnvironmentVariables.Any(pair => pair.Key.StartsWith(ClrIeInstrumentationMethodConfigurationPrefix64Variable)))
{
requestData.MetricsCollection.Add(GetTelemetryKey(telemetryPrefix, dataCollectorInformation), ClrIeProfilerName);
return;
}
}

requestData.MetricsCollection.Add(GetTelemetryKey(telemetryPrefix, dataCollectorInformation), $"{OverwrittenProfilerName}({GetProfilerName(value)})");
}

private static string GetProfilerName(string profilerGuid)
{
return profilerGuid.ToLower() switch
{
VanguardProfilerGuid => VanguardProfilerName,
ClrIeProfilerGuid => ClrIeProfilerName,
IntellitraceProfilerGuid => IntellitraceProfilerName,
_ => UnknownProfilerName
};
}

private static string GetTelemetryKey(string telemetryPrefix, DataCollectorInformation dataCollectorInformation)
{
return string.Format("{0}.{1}", telemetryPrefix, dataCollectorInformation.DataCollectorConfig?.TypeUri?.ToString());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollector.Interfaces
{
/// <summary>
/// The IDataCollectionTelemetryManager Interface.
/// </summary>
internal interface IDataCollectionTelemetryManager
{
/// <summary>
/// Record telemetry regarding environment variable added.
/// </summary>
/// <param name="dataCollectorInformation">
/// Data collector information which requested environment variable.
/// </param>
/// <param name="name">
/// Environment variable name.
/// </param>
/// <param name="value">
/// Environment variable value.
/// </param>
void RecordEnvironmentVariableAddition(DataCollectorInformation dataCollectorInformation, string name, string value);

/// <summary>
/// Record telemetry regarding environment variable is conflicting.
/// </summary>
/// <param name="dataCollectorInformation">
/// Data collector information which requested environment variable.
/// </param>
/// <param name="name">
/// Environment variable name.
/// </param>
/// <param name="value">
/// Environment variable value.
/// </param>
/// <param name="existingValue">
/// Environment variable value that was requested previously.
/// </param>
void RecordEnvironmentVariableConflict(DataCollectorInformation dataCollectorInformation, string name, string value, string existingValue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<TargetFrameworks>netstandard2.0;netstandard1.3;net451</TargetFrameworks>
<TargetFrameworks Condition=" '$(DotNetBuildFromSource)' == 'true' ">netstandard2.0;netstandard1.3</TargetFrameworks>
<WarningsAsErrors>true</WarningsAsErrors>
<LangVersion>8.0</LangVersion>
jakubch1 marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>
<ItemGroup>
<EmbeddedResource Include="Resources\Resources.resx" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ public static class TelemetryDataConstants

public static string DataCollectorsEnabled = "VS.TestRun.DataCollectorsEnabled";

public const string DataCollectorsCorProfiler = "VS.TestPlatform.DataCollector.CorProfiler";
jakubch1 marked this conversation as resolved.
Show resolved Hide resolved

public const string DataCollectorsCoreClrProfiler = "VS.TestPlatform.DataCollector.CoreClrProfiler";

public static string RunState = "VS.TestRun.RunState";

public static string NumberOfSourcesSentForRun = "VS.TestRun.NumberOfSources";
Expand Down
Loading