-
Notifications
You must be signed in to change notification settings - Fork 323
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
Changes from 3 commits
a4039b6
acb01d4
68c5434
525d674
984805e
71b2e42
549488e
6a446aa
14ff291
33d386c
8e5a82a
393d8b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
{ | ||
/// <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 |
---|---|---|
@@ -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.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 const string CorProfilerTelemetryTemplate = "VS.TestPlatform.DataCollector.{0}.CorProfiler"; | ||
jakubch1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private const string CoreClrProfilerTelemetryTemplate = "VS.TestPlatform.DataCollector.{0}.CoreClrProfiler"; | ||
|
||
private readonly IRequestData requestData; | ||
|
||
internal DataCollectionTelemetryManager(IRequestData requestData) | ||
{ | ||
this.requestData = requestData; | ||
} | ||
|
||
/// <inheritdoc/> | ||
public void OnEnvironmentVariableAdded(DataCollectorInformation dataCollectorInformation, string name, string value) | ||
{ | ||
AddProfilerMetricForNewVariable(CorProfilerVariable, CorProfilerTelemetryTemplate, dataCollectorInformation, name, value); | ||
AddProfilerMetricForNewVariable(CoreClrProfilerVariable, CoreClrProfilerTelemetryTemplate, dataCollectorInformation, name, value); | ||
} | ||
|
||
/// <inheritdoc/> | ||
public void OnEnvironmentVariableConflict(DataCollectorInformation dataCollectorInformation, string name, string existingValue) | ||
{ | ||
AddProfilerMetricForConflictedVariable(CorProfilerVariable, CorProfilerTelemetryTemplate, dataCollectorInformation, name, existingValue); | ||
AddProfilerMetricForConflictedVariable(CoreClrProfilerVariable, CoreClrProfilerTelemetryTemplate, dataCollectorInformation, name, existingValue); | ||
} | ||
|
||
/// <inheritdoc/> | ||
public IRequestData RequestData => requestData; | ||
|
||
private void AddProfilerMetricForNewVariable(string profilerVariable, string telemetryTemplateName, DataCollectorInformation dataCollectorInformation, string name, string value) | ||
{ | ||
if (!string.Equals(profilerVariable, name, StringComparison.Ordinal)) | ||
{ | ||
return; | ||
} | ||
|
||
requestData.MetricsCollection.Add(string.Format(telemetryTemplateName, dataCollectorInformation.DataCollectorConfig?.TypeUri?.ToString()), GetProfilerName(value)); | ||
} | ||
|
||
private void AddProfilerMetricForConflictedVariable(string profilerVariable, string telemetryTemplateName, DataCollectorInformation dataCollectorInformation, string name, string existingValue) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: Could we merge with the above and make this a for-loop over the environment variables of interest? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In such case we need to copy logic which we already have to detect conflicts and so on. I would keep it like it is now. I want to keep this class making only simple metrics, nothing more There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made it a little bit simpler. We have 2 methods: RecordEnvVariableAddition and RecordEnvVariableConflict. |
||
{ | ||
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(string.Format(telemetryTemplateName, dataCollectorInformation.DataCollectorConfig?.TypeUri?.ToString()), ClrIeProfilerName); | ||
return; | ||
} | ||
} | ||
|
||
requestData.MetricsCollection.Add(string.Format(telemetryTemplateName, dataCollectorInformation.DataCollectorConfig?.TypeUri?.ToString()), OverwrittenProfilerName); | ||
AbhitejJohn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private static string GetProfilerName(string profilerGuid) | ||
{ | ||
if (string.Equals(profilerGuid, VanguardProfilerGuid, StringComparison.OrdinalIgnoreCase)) | ||
jakubch1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
return VanguardProfilerName; | ||
} | ||
else if (string.Equals(profilerGuid, ClrIeProfilerGuid, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
return ClrIeProfilerName; | ||
} | ||
else if (string.Equals(profilerGuid, IntellitraceProfilerGuid, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
return IntellitraceProfilerName; | ||
} | ||
|
||
return UnknownProfilerName; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
// 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.ObjectModel.Client; | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollector.Interfaces | ||
{ | ||
/// <summary> | ||
/// The IDataCollectionTelemetryManager Interface. | ||
/// </summary> | ||
internal interface IDataCollectionTelemetryManager | ||
{ | ||
/// <summary> | ||
/// Stores 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 OnEnvironmentVariableAdded(DataCollectorInformation dataCollectorInformation, string name, string value); | ||
|
||
/// <summary> | ||
/// Stores 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="existingValue"> | ||
/// Environment variable value that was requested previously. | ||
/// </param> | ||
void OnEnvironmentVariableConflict(DataCollectorInformation dataCollectorInformation, string name, string existingValue); | ||
|
||
/// <summary> | ||
/// Provides telemetry object. | ||
/// </summary> | ||
/// <returns> | ||
/// IRequestData object. | ||
/// </returns> | ||
IRequestData RequestData { get; } | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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