Log TaskParameterEvent for scalar task parameters as well #9827
Assignees
Labels
Comments
|
@KirillOsenkov please let me know what you think. |
|
Seems like a good idea. I'd be surprised if anyone depended on the textual message. |
ladipro
added a commit
that referenced
this issue
Apr 3, 2024
Fixes #9827 ### Context `TaskParameterEvent` with `TaskParameterMessageKind.TaskInput` is currently used only for parameters that are lists. Parameters that are simple strings are logged as a specially formatted low-importance message. The binlog viewer contains logic to recognize this special message and recover the Name and Value to be rendered in the viewer UI. Since we will use this event for analyzers, it would be unfortunate to add one more place with this suboptimal processing. ### Changes Made Unified the logic in `TaskExecutionHost` to log all parameters as `TaskParameterEvent` with `TaskParameterMessageKind.TaskInput`. The change is under a change wave check. ### Testing - Added a new unit test. - Compared diagnostic-level output with task parameter logging enabled with and without the change. No differences were found when passing null, empty, false, true, numeric, string, or stringified item list parameters. - Compared the appearance in binlog for the same sample values as above. No differences were observed. - Compared OrchardCore binlogs with and without the change. They're the same size and the only difference I found was in rendering the `SolutionConfigurationContents` parameter which is a string but the content _looks_ formatted so it was rendered incorrectly as a list of items. ### Notes As @KirillOsenkov pointed out in the issue, we don't really depend on the textual messages so we don't have to do any double-logging. The viewer can remove parsing the textual messages on its own schedule.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
TaskParameterEventwithTaskParameterMessageKind.TaskInputis currently used only for parameters that are lists. Parameters that are simple strings are logged as a specially formatted low-importance message.where the string
Task Parameteris localized.The binlog viewer contains logic to recognize this special message and recover the Name and Value to be rendered in the viewer UI. Since we will use this event for analyzers, it would be unfortunate to add one more place with this suboptimal processing. We should look into logging
TaskParameterEventfor all parameters.Preliminary measurements show binlogs getting ~1% bigger if we log both
TaskParameterEventand the textual message, and slightly smaller if we log only the structured event.The tentative plan is to implement double-logging now, and make a note to remove the textual message after some time when we're comfortable breaking old viewers.
The text was updated successfully, but these errors were encountered: