search

datalust / nlog-targets-seq

An NLog target that writes events to Seq. Built for NLog 4.5+.
https://getseq.net
Apache License 2.0
21 stars 11 forks source link

SeqPropertyItem - Added AsString that matches NLog.Layouts.JsonAttribute.Encode #62

Closed snakefoot closed 2 years ago

snakefoot commented 2 years ago

Think this makes more sense:

<property name="scopenested" value="${scopenested:format=@}" asString="false" />

Than this:

<property name="scopenested" value="${scopenested:format=@}" as="number" />

Maybe consider the property-name "ValueToString" instead of "AsString".

nblumhardt commented 2 years ago

Ah I see - so NLog will set IsNumber for properties that have numeric values? I think as="number" was working around properties from config always being strings (or the mistaken expectation that they might be) in an earlier version.

LGTM if so, but we'll also need to remove a couple of as="number" examples from the README.

snakefoot commented 2 years ago

The SeqPropertyItem.As = "Number" maps to JsonAttribute.Encode, that controls whether output-value should be encoded into a Json-String-Property (or output-value should should be treated as valid json).

The current PullRequest is backward-compatible, and since this source-repository is the wiki, then I guess the wiki should be updated after the actual release?

nblumhardt commented 2 years ago

Thanks for the notes!

The README shown in the root of the repo is usually updated for the dev-* versions of the package so I think we can update it; since the PR is backwards-compatible there's no rush though, as you say 👍