-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support layouts (e.g. JSON) and inner text in <variable> #3459
Conversation
4dc6ec9
to
46f58c0
Compare
@snakefoot I have the idea you don't like this change 👼 |
Think it is good idea. Just need to do it right so the same variable is not used multiple places when not threadsafe.
|
The problem with not being threadsafe. Is NOT that multiple threads can get in trouble by "updating" the layout-value (no longer a issue) . But the problem is when multiple targets uses the same thread-unsafe layout, then you get unexpected output when two targets shares the same layout object.
So your warning comment in variable-layout-renderer is misleading.
|
thanks for the feedback -> #3462 |
After seeing you have implemented a breaking change in the config-element-interface that is not compatible with appsettings json then I must agree with you that I don't like this PR.
You have enforced XML-behavior by adding value-property. Please remove this change.
I like the idea of Json-Layout in variable. But not like this. Please fix
|
The value property could just return null in the case of JSON |
Then you suddenly have something that only works with XML-config
|
'Value' isn't xml specific? Anyway, do you have a proposal how to solve this different? One way or another, this needs a breaking change. |
Yes it will always be a breaking change for the Variables-container. Anyway created #3466 |
Btw. could be nice if one could make a static reference to a JsonLayout. So one is not required to use
Then one could skip a dictionary lookup for every logevent-write. And one would be able to make use of optimizations like |
Can I use
and then
in my target when reading configuration from appsettings.json ? |
Maybe add example to wiki-page, when NLog 5 is released: NLog JsonLayout as NLog Config Variable Example <nlog>
<variable name="defaultJsonLayout" >
<layout type="JsonLayout">
<attribute name="message" value="${messsage}">
</layout>
</variable>
<targets>
<target name="debug" type="Debug" layout="${var:defaultJsonLayout}" />
</targets>
<rules>
<logger name="*" minlevel="Debug" writeTo="debug" />
</rules>
</nlog> APIvar newJsonLayout = new NLog.Layouts.JsonLayout() { IncludeAllProperties = true };
LogManager.Configuration.Variables["defaultJsonLayout"] = newJsonLayout; |
fixes #1577 and fixes #1312