Feature Request: allow to extend and/or parametrize RPlotExporter, or at least specify custom R script

[NinjaCross]

The current implementation (v0.14.0) of RPlotExporter (src/BenchmarkDotNet/Exporters/RPlotExporter.cs) works OK and IMHO is nicely implemented, but lacks for flexibility, because:

1. almost all methods are non-virtual and/or static
2. depends on internal dependencies (i.e. IExporterDependencies, RuntimeInformation, ExporterBase.GetArtifactFullName, and AsyncProcessOutputReader)
3. doesn't allow to specify a custom R script (since it's loaded from the assembly resources BenchmarkDotNet)

This greately limits the usability of such wonderfull feature.
I propose to:

1. Allow to parametrize the file name/path of the R script, so that one can create its own and use it as desired
2. Slightly change the way that the class RPlotExporter is implemented, so that it can be easily inherited and customized by overriding some methods
3. Make IExporterDependencies, RuntimeInformation, and AsyncProcessOutputReader as public instead of internal, so that they can be referred by inherited classes and other custom-made exporters

I'm available to prepare a PR, if the proposal is deemed usefull.

[adamsitnik]

Hello @NinjaCross !

I am fine with the first two points, but when it comes to making RuntimeInformation or AsyncProcessOutputReader public I would prefer not to do that, as we simply change these two quite often and exposing them would limit us in the future (we prefer not to break the users if we can).

What kind of customizations would you like to make?

When it comes to specifying custom R script we could achieve that by moving the logic that computes the path to a virtual property, so just overriding a single property would get you what is needed. An alternative would be to make the path a parameter of the RPlotExporter ctor. But I need to know all requirements to suggest the best approach.

Thanks!

[NinjaCross]

Hi @adamsitnik , thanks for ther feedback !

but when it comes to making RuntimeInformation or AsyncProcessOutputReader public I would prefer not to do that, as we simply change these two quite often and exposing them would limit us in the future (we prefer not to break the users if we can).

I understand, it's an absolutely reasonable motivation.

What kind of customizations would you like to make?

In my specific case, I needed to use a dynamically-generated custom R script.
In order to do that, I had to:

1. Import and change a bunch of classes inside my project (RPlotExporter, ResourceHelper, AsyncProcessOutputReader, and a part of RuntimeInformation)
2. Duplicate the code of CsvMeasurementsExporter.Default.GetArtifactFullName, which is internal too.

All that, could be avoided by allowing to inherit from RPlotExporter and override a minuscule part of it logics.

When it comes to specifying custom R script we could achieve that by moving the logic that computes the path to a virtual property, so just overriding a single property would get you what is needed. An alternative would be to make the path a parameter of the RPlotExporter ctor.

That would indeed solve some trivial scenarios where the R script is static, but since generating charts is a complex topic, it would not be enough for all users, and for more complex senarios.
In my case, the custom script needs to be transformed, and contextualized using informations available inside RPlotExporter.
So, just having the possibility to change the path of the script it not enough.

This is my proposal, with a very minimal impact:

        protected virtual (string Content, string FileName) GetScript(Summary summary, ILogger consoleLogger, string csvFullPath)
        {
            var scriptFileName = "BuildPlots.R";
            var content = ResourceHelper.
                LoadTemplate(scriptFileName).
                Replace("$BenchmarkDotNetVersion$", BenchmarkDotNetInfo.Instance.BrandTitle).
                Replace("$CsvSeparator$", CsvMeasurementsExporter.Default.Separator);

            return (content, scriptFileName);
        }

        public IEnumerable<string> ExportToFiles(Summary summary, ILogger consoleLogger)
        {
            string csvFullPath = CsvMeasurementsExporter.Default.GetArtifactFullName(summary);
            var (script, scriptFileName) = GetScript(summary, consoleLogger, csvFullPath);

            yield return Path.Combine(summary.ResultsDirectoryPath, scriptFileName);

            string scriptFullPath = Path.Combine(summary.ResultsDirectoryPath, scriptFileName);

            const string logFileName = "BuildPlots.log";
            string logFullPath = Path.Combine(summary.ResultsDirectoryPath, logFileName);     

I would be glad to prepare a PR, if you want.

[adamsitnik]

I would be glad to prepare a PR, if you want.

Please go ahead and send the PR, I am happy to review it.