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

Adding trace level in diag argument #1681

Merged
merged 16 commits into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
14 changes: 13 additions & 1 deletion src/Microsoft.TestPlatform.CoreUtilities/Tracing/EqtTrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,19 @@ public static bool IsWarningEnabled
/// </returns>
public static bool InitializeVerboseTrace(string customLogFile)
{
if (!traceImpl.InitializeVerboseTrace(customLogFile))
return InitializeTrace(customLogFile, PlatformTraceLevel.Verbose);
}

/// <summary>
/// Initializes the tracing with custom log file and trace level.
/// Overrides if any trace is set before.
/// </summary>
/// <param name="customLogFile">Customr log file for trace messages.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:custom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

/// <param name="traceLevel">Trace level.</param>
/// <returns>Trace initialized flag.</returns>
public static bool InitializeTrace(string customLogFile, PlatformTraceLevel traceLevel)
{
if (!traceImpl.InitializeTrace(customLogFile, traceLevel))
{
ErrorOnInitialization = PlatformEqtTrace.ErrorOnInitialization;
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public virtual bool SetupChannel(IEnumerable<string> sources)
}

var processId = this.processHelper.GetCurrentProcessId();
var connectionInfo = new TestRunnerConnectionInfo { Port = portNumber, ConnectionInfo = testHostConnectionInfo, RunnerProcessId = processId, LogFile = this.GetTimestampedLogFile(EqtTrace.LogFile) };
var connectionInfo = new TestRunnerConnectionInfo { Port = portNumber, ConnectionInfo = testHostConnectionInfo, RunnerProcessId = processId, LogFile = this.GetTimestampedLogFile(EqtTrace.LogFile), TraceLevel = (int)EqtTrace.TraceLevel };

// Subscribe to TestHost Event
this.testHostManager.HostLaunched += this.TestHostManagerHostLaunched;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal class ProxyDataCollectionManager : IProxyDataCollectionManager
private const string PortOption = "--port";
private const string DiagOption = "--diag";
private const string ParentProcessIdOption = "--parentprocessid";
private const string TraceLevelOption = "--tracelevel";
public const string DebugEnvironmentVaribleName = "VSTEST_DATACOLLECTOR_DEBUG";

private IDataCollectionRequestSender dataCollectionRequestSender;
Expand Down Expand Up @@ -299,6 +300,9 @@ private IList<string> GetCommandLineArguments(int portNumber)
{
commandlineArguments.Add(DiagOption);
commandlineArguments.Add(this.GetTimestampedLogFile(EqtTrace.LogFile));

commandlineArguments.Add(TraceLevelOption);
commandlineArguments.Add(((int)EqtTrace.TraceLevel).ToString());
}

return commandlineArguments;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ public string LogFile
set;
}

/// <summary>
/// Gets or sets the trace level of logs.
/// </summary>
public int TraceLevel
{
get;
set;
}

/// <summary>
/// Gets or sets the runner process id.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public static string ToCommandLineOptions(this TestRunnerConnectionInfo connecti
if (!string.IsNullOrEmpty(connectionInfo.LogFile))
{
options += " --diag " + connectionInfo.LogFile;
options += " --tracelevel " + connectionInfo.TraceLevel;
}

return options;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ bool DoNotInitialize
/// </returns>
bool InitializeVerboseTrace(string customLogFile);

/// <summary>
/// Initializes the tracing with custom log file and trace level.
/// Overrides if any trace is set before.
/// </summary>
/// <param name="customLogFile">Customr log file for trace messages.</param>
/// <param name="traceLevel">Trace level.</param>
/// <returns>Trace initialized flag.</returns>
bool InitializeTrace(string customLogFile, PlatformTraceLevel traceLevel);

/// <summary>
/// Gets a value indicating if tracing is enabled for a trace level.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,18 @@ private static TraceSource Source

/// <inheritdoc/>
public bool InitializeVerboseTrace(string customLogFile)
{
return this.InitializeTrace(customLogFile, PlatformTraceLevel.Verbose);
}

/// <inheritdoc/>
public bool InitializeTrace(string customLogFile, PlatformTraceLevel platformTraceLevel)
{
isInitialized = false;

LogFile = customLogFile;
TraceLevel = TraceLevel.Verbose;
Source.Switch.Level = SourceLevels.All;
TraceLevel = this.MapPlatformTraceToTrace(platformTraceLevel);
Source.Switch.Level = TraceSourceLevelsMap[TraceLevel];

// Ensure trace is initlized
return EnsureTraceIsInitialized();
Expand Down Expand Up @@ -351,7 +357,7 @@ private static bool EnsureTraceIsInitialized()
}
catch (Exception e)
{
UnInitializeVerboseTrace();
UnInitializeTrace();
ErrorOnInitialization = e.ToString();
return false;
}
Expand Down Expand Up @@ -419,7 +425,7 @@ private static int GetProcessId()
}
}

private static void UnInitializeVerboseTrace()
private static void UnInitializeTrace()
{
isInitialized = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ public bool InitializeVerboseTrace(string customLogFile)
throw new NotImplementedException();
}

public bool InitializeTrace(string customLogFile, PlatformTraceLevel traceLevel)
{
throw new NotImplementedException();
}

public bool ShouldTrace(PlatformTraceLevel traceLevel)
{
throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ public void WriteLine(PlatformTraceLevel level, string message)

/// <inheritdoc/>
public bool InitializeVerboseTrace(string customLogFile)
{
return this.InitializeTrace(customLogFile, PlatformTraceLevel.Verbose);
}

/// <inheritdoc/>
public bool InitializeTrace(string customLogFile, PlatformTraceLevel traceLevel)
{
string logFileName = string.Empty;
try
Expand All @@ -81,7 +87,7 @@ public bool InitializeVerboseTrace(string customLogFile)
}

LogFile = Path.Combine(Path.GetTempPath(), logFileName + ".TpTrace.log");
TraceLevel = PlatformTraceLevel.Verbose;
TraceLevel = traceLevel;

return this.TraceInitialized();
}
Expand Down Expand Up @@ -154,7 +160,7 @@ private bool TraceInitialized()
}
catch (Exception ex)
{
this.UnInitializeVerboseTrace();
this.UnInitializeTrace();
ErrorOnInitialization = ex.Message;
return false;
}
Expand All @@ -163,7 +169,7 @@ private bool TraceInitialized()
}
}

private void UnInitializeVerboseTrace()
private void UnInitializeTrace()
{
isInitialized = false;
LogFile = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public ConsoleParameters(IFileHelper fileHelper)
this.fileHelper = fileHelper;
}

/// <summary>
/// Trace level for logs.
/// </summary>
public PlatformTraceLevel TraceLevel { get; set; } = PlatformTraceLevel.Verbose;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I use System.Diagnostics.TraceLevel here OR Microsoft.VisualStudio.TestPlatform.ObjectModel.PlatformTraceLevel.

Internally we use PlatformTraceLevel wherever TraceLevel is not present. Example: NETSTANDARD1_4

Copy link
Contributor

Choose a reason for hiding this comment

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

PlatformTraceLevel is in PlatformAbstraction, this will mandate translationlayer users to have a reference to PlatformAbstraction as well.

If possible we should remove all the usage for System.Diagnostics.TraceLevel, rather have another TraceLevel enum in OM for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed dependency on platform abstractions.
Cant remove System.Diagnostics.TraceLevel as used by exe config trace level.


/// <summary>
/// Full path for the log file
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class VsTestConsoleProcessManager : IProcessManager
/// Diagnostics argument for Vstest CLI
/// Enables Diagnostic logging for Vstest CLI and TestHost - Optional
/// </summary>
private const string DIAG_ARGUMENT = "/diag:{0}";
private const string DIAG_ARGUMENT = "/diag:{0};tracelevel={1}";

private string vstestConsolePath;
private object syncObject = new object();
Expand Down Expand Up @@ -135,7 +135,7 @@ private string[] BuildArguments(ConsoleParameters parameters)
if(!string.IsNullOrEmpty(parameters.LogFilePath))
{
// Extra args: --diag|/diag:<PathToLogFile>
args.Add(string.Format(CultureInfo.InvariantCulture, DIAG_ARGUMENT, parameters.LogFilePath));
args.Add(string.Format(CultureInfo.InvariantCulture, DIAG_ARGUMENT, parameters.LogFilePath, parameters.TraceLevel));
}

return args.ToArray();
Expand Down
8 changes: 7 additions & 1 deletion src/datacollector/DataCollectorMain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public class DataCollectorMain
/// </summary>
private const string LogFileArgument = "--diag";

/// <summary>
/// Trace level for logs.
/// </summary>
private const string TraceLevelArgument = "--tracelevel";

private IProcessHelper processHelper;

private IEnvironment environment;
Expand Down Expand Up @@ -65,7 +70,8 @@ public void Run(string[] args)
string logFile;
if (argsDictionary.TryGetValue(LogFileArgument, out logFile))
{
EqtTrace.InitializeVerboseTrace(logFile);
var traceLevel = CommandLineArgumentsHelper.GetIntArgFromDict(argsDictionary, TraceLevelArgument);
EqtTrace.InitializeTrace(logFile, (PlatformTraceLevel)traceLevel);
}
else
{
Expand Down
5 changes: 4 additions & 1 deletion src/testhost.x86/DefaultEngineInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ internal class DefaultEngineInvoker :

private const string LogFileArgument = "--diag";

private const string TraceLevelArgument = "--tracelevel";

private const string DataCollectionPortArgument = "--datacollectionport";

private const string TelemetryOptedIn = "--telemetryoptedin";
Expand Down Expand Up @@ -228,7 +230,8 @@ private static void InitializeEqtTrace(IDictionary<string, string> argsDictionar
// Setup logging if enabled
if (argsDictionary.TryGetValue(LogFileArgument, out string logFile))
{
EqtTrace.InitializeVerboseTrace(logFile);
var traceLevelInt = CommandLineArgumentsHelper.GetIntArgFromDict(argsDictionary, TraceLevelArgument);
Copy link
Contributor

@mayankbansal018 mayankbansal018 Jul 9, 2018

Choose a reason for hiding this comment

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

verify for invalid tracelevel values,
Also verify vstest.console with older testhost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check for invalid tracelevel values
Verified with older testhost as well.

EqtTrace.InitializeTrace(logFile, (PlatformTraceLevel)traceLevelInt);
}
else
{
Expand Down
Loading