-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a4039b6
Initial version of metrics for DC profiling
acb01d4
Tests
68c5434
Push fixes to make linux build
525d674
provide telemetry opted in flag
984805e
Small refactoring
71b2e42
Change name
549488e
Merge remote-tracking branch 'upstream/master' into dev/jachocho/tele…
6a446aa
Upgrade CC comp
14ff291
Move to guids
33d386c
Revert lang change
8e5a82a
Fixing descriptions
393d8b7
Last changes
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
src/Microsoft.TestPlatform.Common/DataCollection/AfterTestRunEndResult.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// 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="attachmentSets"> | ||
/// The collection of attachment sets. | ||
/// </param> | ||
/// <param name="metrics"> | ||
/// The metrics. | ||
/// </param> | ||
public AfterTestRunEndResult(Collection<AttachmentSet> attachmentSets, IDictionary<string, object> metrics) | ||
{ | ||
this.AttachmentSets = attachmentSets; | ||
this.Metrics = metrics; | ||
} | ||
|
||
[DataMember] | ||
public Collection<AttachmentSet> AttachmentSets { get; private set; } | ||
|
||
[DataMember] | ||
public IDictionary<string, object> Metrics { get; private set; } | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
102 changes: 102 additions & 0 deletions
102
src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionTelemetryManager.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
// 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 static readonly Guid ClrIeProfilerGuid = Guid.Parse("{324f817a-7420-4e6d-b3c1-143fbed6d855}"); | ||
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), GetProfilerGuid(value).ToString()); | ||
} | ||
|
||
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; | ||
} | ||
|
||
var existingProfilerGuid = GetProfilerGuid(existingValue); | ||
|
||
if (ClrIeProfilerGuid == existingProfilerGuid) | ||
{ | ||
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), ClrIeProfilerGuid.ToString()); | ||
return; | ||
} | ||
} | ||
|
||
requestData.MetricsCollection.Add(GetTelemetryKey(telemetryPrefix, dataCollectorInformation), $"{existingProfilerGuid}({OverwrittenProfilerName}:{GetProfilerGuid(value)})"); | ||
} | ||
|
||
private static Guid GetProfilerGuid(string profilerGuid) | ||
{ | ||
Guid guid; | ||
if (Guid.TryParse(profilerGuid, out guid)) | ||
{ | ||
return guid; | ||
} | ||
|
||
return Guid.Empty; | ||
} | ||
|
||
private static string GetTelemetryKey(string telemetryPrefix, DataCollectorInformation dataCollectorInformation) | ||
{ | ||
return string.Format("{0}.{1}", telemetryPrefix, dataCollectorInformation.DataCollectorConfig?.TypeUri?.ToString()); | ||
} | ||
} | ||
} |
42 changes: 42 additions & 0 deletions
42
...icrosoft.TestPlatform.Common/DataCollection/Interfaces/IDataCollectionTelemetryManager.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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