Allow overriding task parameter logging options in task classes #5236
Conversation
This is a new virtual method on Task that allows task implementations to customize how and whether to log its parameters. The default implementation preserves existing behavior, which is, log everything if LogTaskInputs is true. Overrides on specific tasks customize this behavior to avoid logging item metadata where not relevant, or not log some parameters at all. For example since the Hash task only looks at the ItemSpec, there's no point to log metadata for the input items. The tasks were chosen by the most output. These cause the majoriy of logging for task inputs and outputs. This does not touch tasks important for build investigations such as Rar and Csc and NuGet-related tasks. Tasks modified were carefully chosen to ensure we don't lose useful information from the logs.
Tasks can customize the parameter logging behavior. All new API is internal.
|
How about keeping both mechanisms? Tasks that know for sure it some of its items or metadata do not add any logging value can remove them via this PR. For tasks that cannot be changed, there's #5235 |
|
I don't mind ;) Will fix conflicts shortly. Was curious what the team thinks since I'm not 100% on either approach. |
|
I'm thinking I like the UsingTask approach much better as it's more extensible and doesn't require to change the task classes. Let's keep this PR around for a bit but if all goes well I'll just abandon it. |
|
We discussed it a bit in the PR review today. Using task is seen as not that great, because it's hard for the user to override UsingTask elements. Unfortunately first one wins, not last one wins. This assumes the scenario where the user wants to tone down a task and has to add using task to do so. He has to make sure his using task element is the first. Regarding the TaskExtension approach, I don't remember any points against it, other than it should be in Task, not TaskExtension. I'll invite you to the next PR review so we can brain storm. |
|
I think there was a reason it is on
|
|
Also I honestly don't foresee users needing to change that all that often - the mechanism is predominantly for us, and I don't even think we'll need to extend it in the future, maybe just for NuGet tasks. And for that we can just do that at the original UsingTask. |
|
Here's another approach, via simple properties: |
|
Team triage: closing in favor of #5268. |
This is an alternative approach to #5235.
Instead of specifying the task parameter logging options in UsingTask XML, we introduce an internal virtual method on TaskExtension to let implementations override the defaults.
A new internal interface has to be added to Framework to connect the task implementations to the TaskLoggingHelper that needs to consume the new method.