XmlLayout - Render LogEventInfo.Properties as XML #2670

snakefoot · 2018-04-22T19:55:05Z

Ex. SQL XML Column

<properties>
  <property key="Action">GetUsers</property>
  <property key="Controller">UserController</property>
</properties>

SELECT 	[Message]
  , [TimeStamp]
  , [Exception]
  , [Properties].value('(//property[@key="Action"]/node())[1]', 'nvarchar(max)') as Action
FROM [Logs]
WHERE [Properties].value('(//property[@key="Controller"]/node())[1]', 'nvarchar(max)') = 'UserController'

Ex. SMTP mails with advanced XHTML-formatting (colors etc):

msg.BodyFormat = MailFormat.Html;

codecov · 2018-04-22T20:23:15Z

Codecov Report

Merging #2670 into dev will decrease coverage by <1%.
The diff coverage is 78%.

@@          Coverage Diff           @@
##             dev   #2670    +/-   ##
======================================
- Coverage     80%     80%   -<1%     
======================================
  Files        331     333     +2     
  Lines      25641   26034   +393     
  Branches    3321    3404    +83     
======================================
+ Hits       20560   20844   +284     
- Misses      4145    4227    +82     
- Partials     936     963    +27

snakefoot · 2018-04-23T05:11:32Z

Still work-in-progress

304NotModified · 2018-04-23T09:02:10Z

nice!

snakefoot · 2018-04-23T19:33:08Z

Now I think it is ready for review.

304NotModified · 2018-04-24T20:37:25Z

src/NLog/Internal/XmlHelper.cs

+                {
+                    if (i == 0 && (chr == 'x' || chr == 'X') && xmlElementName.Length >= 3)
+                    {
+                        if (char.ToLowerInvariant(xmlElementName[1]) == 'm' && char.ToLowerInvariant(xmlElementName[2]) == 'l')


What are we testing here? The element <xml? And it will be <_xml ?

Please add this to the code comments, thanks!

There is a very nice comment at the top explaining the rules for XML-element-names.

Could we add an unit test for this?

src/NLog/LayoutRenderers/Wrappers/XmlEncodeLayoutRendererWrapper.cs

304NotModified · 2018-04-24T20:41:28Z

src/NLog/Layouts/XmlLayout.cs

+    [Layout("XmlLayout")]
+    [ThreadAgnostic]
+    [ThreadSafe]
+    public class XmlLayout : Layout


shouldn't we implement the Interface IIncludeContext ?

Just like JsonLayout, then XmlLayout doesn't include IncludeNdc and IncludeNdlc as properties.

src/NLog/LayoutRenderers/Wrappers/XmlEncodeLayoutRendererWrapper.cs

304NotModified · 2018-04-29T13:01:16Z

src/NLog/Layouts/XmlAttribute.cs

+        /// Determines wether or not this attribute will be Json encoded.
+        /// </summary>
+        /// <docgen category='XML Attribute Options' order='100' />
+        public bool Encode


maybe default it to true, as 2 of the 3 ctors set it to true

The default is true, as it gets the default value from XmlEncodeLayoutRendererWrapper

src/NLog/Layouts/XmlLayout.cs

304NotModified

looks great! Would be nice if we could add some more unit tests for it.

I could help on that, but I'm not sure for every case how to cover it.

PS: im trying create a codecov report for 3d7f3ee

304NotModified · 2018-05-02T21:10:32Z

src/NLog/Layouts/XmlLayout.cs

+            RenderXmlFormattedMessage(logEvent, target);
+            if (target.Length == orgLength && IncludeEmptyValue && !string.IsNullOrEmpty(ElementName))
+            {
+                target.Append('<');


any idea if we could add an unit test for this case?

304NotModified · 2018-05-02T21:10:50Z

src/NLog/Layouts/XmlLayout.cs

+                    int beforeAttribLength = sb.Length;
+                    if (!RenderAppendXmlAttributeValue(attrib, logEvent, sb, sb.Length == orgLength))
+                    {
+                        sb.Length = beforeAttribLength;


any idea if we could add an unit test for this case?

304NotModified · 2018-05-02T21:11:44Z

src/NLog/Layouts/XmlLayout.cs

+            string propNameElement = null;
+            if (_propertiesElementNameHasFormat)
+            {
+                propNameElement = XmlHelper.XmlConvertToStringSafe(propName);


any idea if we could add an unit test for this case?

304NotModified · 2018-05-02T21:12:00Z

src/NLog/Layouts/XmlLayout.cs

+            }
+            else
+            {
+                sb.Append('>');


any idea if we could add an unit test for this case?

304NotModified · 2018-05-02T21:13:14Z

src/NLog/Internal/XmlHelper.cs

+                {
+                    if (i == 0 && (chr == 'x' || chr == 'X') && xmlElementName.Length >= 3)
+                    {
+                        if (char.ToLowerInvariant(xmlElementName[1]) == 'm' && char.ToLowerInvariant(xmlElementName[2]) == 'l')


Could we add an unit test for this?

304NotModified · 2018-05-02T21:13:20Z

src/NLog/Internal/XmlHelper.cs

+                {
+                    if (chr == '_')
+                    {
+                        sb?.Append(chr);


Could we add an unit test for this?

304NotModified · 2018-05-02T21:13:36Z

src/NLog/Internal/XmlHelper.cs

+                    sb = new StringBuilder(xmlElementName.Length);
+                    if (i > 1)
+                        sb.Append(xmlElementName, 0, i - 1);
+                    if (i == 0 && char.IsWhiteSpace(chr))


Could we add an unit test for this?

snakefoot · 2018-05-02T21:29:01Z

You can have as many unit-test that you like, just a matter of work-hours.

Prefer to have agreement on API before starting to lock things down with unit-tests. But happy you like it.

304NotModified

You can have as many unit-test that you like, just a matter of work-hours.

yes, but then no force-push please.

Prefer to have agreement on API before starting to lock things down with unit-test

I'm happy with the design 👍 but there are some small naming things i'm doubting about. Added them as comments

Maybe we should move to a separate XML serializer class in the future, although 3rd party XML serializes aren't common

src/NLog/Layouts/XmlAttribute.cs

src/NLog/Layouts/XmlLayout.cs

src/NLog/Layouts/XmlAttribute.cs

304NotModified · 2018-05-02T22:06:04Z

src/NLog/Layouts/XmlLayout.cs

+        /// Auto indent and create new lines
+        /// </summary>
+        /// <docgen category='XML Options' order='10' />
+        public bool IndentXml { get; set; }


maybe just "Indent"

src/NLog/Layouts/XmlLayout.cs

304NotModified · 2018-08-23T19:52:40Z

@snakefoot could you please resolve the merge conflict? Thanks!

…ton tag)

… test)

* XmlLayout - Render LogEventInfo.Properties as XML (Ex. SQL XML Column) * Handle self-referencing types * XmlLayout - Render LogEventInfo.Properties as XML (More test coverage) * XmlLayout - Render LogEventInfo.Properties as XML (Support for singleton tag) * XmlLayout - Render LogEventInfo.Properties as XML (Xml config in unit test) * XmlLayout - Render LogEventInfo.Properties as XML (Less flexible, easier to use) * XmlLayout - Renamed NodeName to ElementName * XmlLayout - Fixed XML comments * XmlLayout - Added support for MutableUnsafe * XmlLayout - Added support for MutableUnsafe (Refactor) * XmlLayout - Render LogEventInfo.Properties as XML (Optimize for XmlEncode = false) * XmlLayout - Render LogEventInfo.Properties as XML (No need for message template, yet) * XmlLayout - Render LogEventInfo.Properties as XML (Optimize for XmlEncode = false) * consts, c# 7 * deduplicate * deduplicate properties rendering * attrib -> attributes * small improvements * XmlLayout - Removed unnecessary allocation of yield state-machine * XmlLayout - Test proposal * Added test (fails) * Added test (succeeds) * Added test (succeeds) * Added test (fails) * Added test (fails) * Added test (succeeds) * XmlLayout - Fixed test of IncludeEmptyValue, and removed newline issues * XmlLayout - Added unit test with encoding of element-names

snakefoot · 2019-02-21T22:04:06Z

Started working on https://github.com/NLog/NLog/wiki/XmlLayout

304NotModified · 2019-02-21T22:05:44Z

@snakefoot this could be helpful:

XmlLayout.md.txt

304NotModified · 2019-02-22T20:20:24Z

updated the xml layout. Added some more sections, not sure if it's better. Feel free to restructure

304NotModified · 2019-02-22T20:22:03Z

bit stange that attribute has "name" and element has "ElementName " 👼

snakefoot · 2019-02-22T21:38:44Z

bit stange that attribute has "name" and element has "ElementName "

Think it was your decision. Don't mind changing it.

304NotModified · 2019-02-22T21:40:48Z

Then maybe its my mistake 😂

snakefoot · 2019-02-23T09:39:42Z

Created #3137 as example of possible rename.

304NotModified · 2019-03-04T22:37:09Z

@snakefoot I'm happy with the docs now, are you also? (https://github.com/NLog/NLog/wiki/XmlLayout)

One thing I'm doubting of,

the first "definition" is a bit strange:

It's now a combo of a definition (includeAllProperties="Boolean`" ) and an example. Not that I am a fan of the layout of the definitions of other targets...

Should we convert it to an example or a definition?

snakefoot · 2019-03-08T21:14:42Z

@304NotModified Made some updates to the Wiki-page for XmlLayout: https://github.com/NLog/NLog/wiki/XmlLayout (Like that it is an example)

snakefoot force-pushed the XmlLayout branch from c57f9d5 to f2badbb Compare April 22, 2018 20:04

304NotModified added feature work in progress labels Apr 23, 2018

snakefoot force-pushed the XmlLayout branch from f2badbb to ec65246 Compare April 23, 2018 19:32

snakefoot force-pushed the XmlLayout branch 5 times, most recently from 8dae8cf to d8dfb39 Compare April 24, 2018 19:28

304NotModified self-requested a review April 24, 2018 19:39

snakefoot force-pushed the XmlLayout branch 2 times, most recently from 33182b8 to 9cdfa86 Compare April 24, 2018 21:43

304NotModified reviewed Apr 29, 2018

View reviewed changes

304NotModified removed the work in progress label Apr 29, 2018

snakefoot force-pushed the XmlLayout branch from 1ae6ea4 to be0f8cb Compare April 29, 2018 16:56

304NotModified reviewed May 2, 2018

View reviewed changes

304NotModified added this to the 4.6 milestone May 2, 2018

304NotModified changed the base branch from master to dev August 14, 2018 20:26

snakefoot added 5 commits August 23, 2018 22:45

XmlLayout - Render LogEventInfo.Properties as XML (Ex. SQL XML Column)

2ec3e99

Handle self-referencing types

625e6d9

XmlLayout - Render LogEventInfo.Properties as XML (More test coverage)

6f00f48

XmlLayout - Render LogEventInfo.Properties as XML (Support for single…

b1101dc

…ton tag)

XmlLayout - Render LogEventInfo.Properties as XML (Xml config in unit…

f463a37

… test)

XmlLayout - Fixed test of IncludeEmptyValue, and removed newline issues

826ee07

snakefoot force-pushed the XmlLayout branch 4 times, most recently from d95987c to d174e55 Compare September 30, 2018 22:03

XmlLayout - Added unit test with encoding of element-names

94430fd

snakefoot force-pushed the XmlLayout branch from d174e55 to 94430fd Compare October 7, 2018 19:31

304NotModified removed the needs unittests label Nov 2, 2018

304NotModified merged commit 9a05ee9 into NLog:dev Nov 2, 2018

snakefoot mentioned this pull request Nov 2, 2018

Feature request: allow for NLog to render nested innerexceptions to JSON #1311

Closed

snakefoot mentioned this pull request Nov 12, 2018

XmlLayout - Added object reflection using ObjectReflectionCache #3014

Merged

304NotModified added the xml-layout label Nov 15, 2018

snakefoot mentioned this pull request Jan 14, 2019

NLog 4.6 RTM todo #3080

Closed

304NotModified added needs documentation on wiki needs documention on nlog-project.org/config labels Feb 8, 2019

304NotModified mentioned this pull request Feb 22, 2019

Added XML layout NLog/NLog.github.io#96

Merged

304NotModified added the documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) label Mar 5, 2019

snakefoot deleted the XmlLayout branch April 4, 2020 13:09

This was referenced Jan 15, 2021

Add support for XmlLayout #1167

Closed

Add support for XmlLayout #1096

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XmlLayout - Render LogEventInfo.Properties as XML #2670

XmlLayout - Render LogEventInfo.Properties as XML #2670

snakefoot commented Apr 22, 2018 •

edited

Loading

codecov bot commented Apr 22, 2018 •

edited

Loading

snakefoot commented Apr 23, 2018

304NotModified commented Apr 23, 2018

snakefoot commented Apr 23, 2018

304NotModified Apr 24, 2018

snakefoot Apr 29, 2018

304NotModified May 2, 2018

304NotModified Apr 24, 2018

snakefoot Apr 29, 2018 •

edited

Loading

304NotModified Apr 29, 2018

snakefoot Apr 29, 2018

304NotModified left a comment

304NotModified May 2, 2018

304NotModified May 2, 2018

304NotModified May 2, 2018

304NotModified May 2, 2018

304NotModified May 2, 2018

304NotModified May 2, 2018

304NotModified May 2, 2018

snakefoot commented May 2, 2018 •

edited

Loading

304NotModified left a comment

304NotModified May 2, 2018

304NotModified commented Aug 23, 2018

snakefoot commented Feb 21, 2019

304NotModified commented Feb 21, 2019

304NotModified commented Feb 22, 2019 •

edited

Loading

304NotModified commented Feb 22, 2019

snakefoot commented Feb 22, 2019

304NotModified commented Feb 22, 2019 •

edited

Loading

snakefoot commented Feb 23, 2019 •

edited

Loading

304NotModified commented Mar 4, 2019 •

edited

Loading

snakefoot commented Mar 8, 2019 •

edited

Loading

XmlLayout - Render LogEventInfo.Properties as XML #2670

XmlLayout - Render LogEventInfo.Properties as XML #2670

Conversation

snakefoot commented Apr 22, 2018 • edited Loading

codecov bot commented Apr 22, 2018 • edited Loading

Codecov Report

snakefoot commented Apr 23, 2018

304NotModified commented Apr 23, 2018

snakefoot commented Apr 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snakefoot Apr 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

304NotModified left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snakefoot commented May 2, 2018 • edited Loading

304NotModified left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

304NotModified commented Aug 23, 2018

snakefoot commented Feb 21, 2019

304NotModified commented Feb 21, 2019

304NotModified commented Feb 22, 2019 • edited Loading

304NotModified commented Feb 22, 2019

snakefoot commented Feb 22, 2019

304NotModified commented Feb 22, 2019 • edited Loading

snakefoot commented Feb 23, 2019 • edited Loading

304NotModified commented Mar 4, 2019 • edited Loading

snakefoot commented Mar 8, 2019 • edited Loading

snakefoot commented Apr 22, 2018 •

edited

Loading

codecov bot commented Apr 22, 2018 •

edited

Loading

snakefoot Apr 29, 2018 •

edited

Loading

snakefoot commented May 2, 2018 •

edited

Loading

304NotModified commented Feb 22, 2019 •

edited

Loading

304NotModified commented Feb 22, 2019 •

edited

Loading

snakefoot commented Feb 23, 2019 •

edited

Loading

304NotModified commented Mar 4, 2019 •

edited

Loading

snakefoot commented Mar 8, 2019 •

edited

Loading