Gearama/profile debug

[gearama]

Enable the passing of the debug option to the setup method for languages (it was missing) also enable running valgrind tool for profiling for cpp . Should i do the install of the tool here or in the pipeline? i tend for the pipeline since i don't know exactly what linux i'm runing and what package tool it it using.
Also added a profiler options param. valgrind has several profilers that can be selected through command line, thus i need this param to be able to execute properly.

Added tests for the method creating the command lines to be executed to build. Since the Util methods are static in a static class they cannot be mocked, thus i had to work around it to get the parameters of the method calls to be able to compare them with expected values.

[mikeharder]

@gearama: The code changes look good to me, however before merging this PR I'd like to verify it works end-to-end, and I have a few questions related to this.

1. Should profiling work correctly if ProfilerOptions is not set? If it currently does not, can we define a reasonable set of defaults so it does?
2. Should we add a top-level ProfilerOptions to the C++ pipelines (and the tools - perf-automation pipeline)? Or were you thinking users would set these via the AdditionalArguments textbox that already exists?

For installing the valgrind tool, I would add this to the pipeline YML in both the tools repo (for the tools - perf-automation pipeline) and the C++ repo (for your service pipelines). For an example, see how .NET is installing their profiling tool:

https://github.com/Azure/azure-sdk-tools/pull/5572/files#diff-db1428e1537bee9d1dbbf6d610c0bc30233911688ebe327425ab73c96536e39dR163

https://github.com/Azure/azure-sdk-for-net/pull/34554/files#diff-fe881f2374762f98475e30354d6f9c3e23c2265394a0b55e50a9e6c238532384R41

As part of the C++ PR to install valgrind, you can also add the Profile parameter (and optionally the ProfilerOptions parameter) to your service pipelines (like .NET did).

[gearama]

Should profiling work correctly if ProfilerOptions is not set? If it currently does not, can we define a reasonable set of defaults so it does?
[[gearama]] yes, in the case of c++ it defaults to memory leaks checks.

Should we add a top-level ProfilerOptions to the C++ pipelines (and the tools - perf-automation pipeline)? Or were you thinking users would set these via the AdditionalArguments textbox that already exists?

[[gearama]] i would make it a separate field if needed, if it works as aditional params i would not bother adding it as a separata field.
For installing the valgrind tool, I would add this to the pipeline YML in both the tools repo (for the tools - perf-automation pipeline) and the C++ repo (for your service pipelines). For an example, see how .NET is installing their profiling tool:

[gearama]

Azure/azure-sdk-for-cpp#4398

[mikeharder]

I am able to run this PR with no errors, and I can verify valgrind was run, but the generated profile zip is empty, because no profile output files were generated under the profile directory. You can obtain the path to this directory using this util method:

azure-sdk-tools/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Util.cs

Lines 120 to 123 in da16722

	public static string GetProfileDirectory(string repoRoot)
	{
	return Path.Combine(repoRoot, "profile");
	}

You will need to generate an output filename, example in JS here:

azure-sdk-tools/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/JavaScript.cs

Lines 150 to 152 in da16722

	var profileFilename = $"{packageVersions[primaryPackage]}_{testName}_{formattedArgs}_{profileCount++}.cpuprofile";
	var profileDir = Util.GetProfileDirectory(WorkingDirectory);
	var profileOutputPath = Path.GetFullPath(Path.Combine(profileDir, stripPackageName, profileFilename));

Then either pass this filename to valgrind so it knows where to write the profile, or move the file generated by valgrind to this path.

Example build: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2605373&view=logs&j=eb9934df-ce1d-5189-09d1-97b6d9cd110c&t=cdee2656-81cf-5bb2-d238-c888b119d811

[mikeharder]

If a profile output file should be generated by default, this PR needs additional changes.

[gearama]

by default valgrind outputs to stderr , i prefer to leave it as it is for the moment until the need for a file presents itself. personally i don't see the need for one, all the data is in the log.
IMHO a log file is more headache then it's worth, i have to hunt down some file in who knows what location, to which i may or may not have access, then i have to figure out whom to beg for access in case i don't have it(most likely) just to get data that is already in the log :) . does not seem worth the hastle when i can just scroll the log.
Besides the log file format is known by everyone , thus makes it strait forward for people to understand.
Also i prefer to see the output as part of the whole execution log, as it can be corelated with the rest of the logs for a big picture view.

[mikeharder]

by default valgrind outputs to stderr , i prefer to leave it as it is for the moment until the need for a file presents itself. personally i don't see the need for one, all the data is in the log. IMHO a log file is more headache then it's worth, i have to hunt down some file in who knows what location, to which i may or may not have access, then i have to figure out whom to beg for access in case i don't have it(most likely) just to get data that is already in the log :) . does not seem worth the hastle when i can just scroll the log. Besides the log file format is known by everyone , thus makes it strait forward for people to understand. Also i prefer to see the output as part of the whole execution log, as it can be corelated with the rest of the logs for a big picture view.

If the data you need is in the console log, I agree publishing to an artifact file is optional. Most profilers in other languages output a binary file that needs to be loaded in a tool to view the data, so these are published in an artifact zip file, since there would be no way to view the data in the console log.

[mikeharder]

This LGTM, we can merge this now and add more features to cpp profiling in future PRs.