Closed Prunkles closed 1 year ago
And I think, to keep things consistent, it would be a good idea to make the ThemedJsonPropertyValueRenderer
class public, not internal
Thanks for dropping by!
Would it be possible to include a bit more info about the usage scenario you have in mind? I can see from the PR how the extension point fits in, but I'm not sure whether the extra surface area is worthwhile given that if you're prepared to write custom code then many scenarios could possibly be achieved directly via ITextFormatter
without the involvement of this library.
Thanks!
I tried to implement ITextFormatter
but I quickly realized that if I want just a slightly extended version of this library (including all its features like expressions in output templates), I would need to rewrite the entire library from scratch. I really have a lack of tuning.
My vision is that this library provides a powerful set of features for filtering and formatting with ExpressionTemplate, but I think it should not constrain other aspects of logging. And now it's strongly fixed to use json object formatting. For instance, I like how default StructureValue are rendered, there is no "$type": "Type"
field at the very end, but a fancy Type { … }
which imho is more readable
Also, I think that it would be nice to make the abstraction one more level up and add somethink like interface IMessageRenderer { void Render(MessageTemplate, LogEvent, TextWriter) }
, and inside it will use IPropertyValueRenderer
. Because I found it strange that there is some edge case: the logic below (about scalar rendering) is duplicated there and in ThemedJsonValueFormatter itself. I think this logic should be coupled in one place like IMessageRenderer
Thanks for your reply, I appreciate the thought and effort that's gone into the PR.
At this stage I don't think it's a part of the design space I'm ready to push into; the custom functions extension point is the limit of the extensibility I had in mind for Serilog.Expressions, specifically because it covers a wide range of scenarios with a very narrow surface area. Over time this should set the library up for easy maintenance and evolution, while not limiting things too much.
Thanks for putting this forward, all the same. Please drop me a line if you need a hand pushing ahead in a fork or via some other approach.
I found it frustrating that there are no options to configure objects to be rendered as in the default
LogEventPropertyValue.Render(…)
method and apply fancy themes to them. Although it would be preferable to add this feature to Serilog itself, doing so would have numerous consequences. Therefore, I decided to implement it in this project.The main addition is the
IPropertyValueRenderer
interface andThemedDefaultPropertyValueRenderer
class, which is a direct copy of the corresponding(ScalarValue/StructureValue/SequenceValue/etc).Render(…)
method implementations, but with added theming capabilities.ThemedJsonPropertyValueRenderer
(renamedThemedJsonValueFormatter
) implements this interface too.Additionally, I included the
format
andformatProvider
parameters since they were used in the default renderers. It appears that they are actually necessary in this context based on the design.TODO