-
Notifications
You must be signed in to change notification settings - Fork 69
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
need a discussion about v5.0 breaking changes at targets implementation #275
Comments
@haf do you know any other target maintainers, want to make a discussion and help wanted. can you help @ them to notice here? about the json , i think there maybe two situation:
|
We have some contributors and target maintainers But noone dedicated; the targets mostly get written once and then they last a couple of years in my experience—without changes. |
-1 on my Newtonsoft library. I don't think reflection-based serialisation is very good. Better to let the user register serialisers for known types and cast them to the known type if we need JSON serialisation; and then documenting that. Newtonsoft has a tendency to creep in everywhere, but we really only need it for a select-few targets; plus, we could provide default serialisers using Chiron for the DOM — even though the DOM is not any longer the default thing to log — that way users of the library who need to log structured data to JSON targets, can ensure they log using the known structure; we can even make JSON from Chiron the known structure; straight off the bat. |
We don't need to support deserialisation beyond the Logary-js service, which already has a good enough API. |
do you mean something like this : use
|
Yes exactly. In the third case, we can check if it's formattable and/or do a ToString on the object instead of casting it. |
i think when fsharp/fslang-design#170 is implemented, we can support custom like this : https://github.com/serilog/serilog/wiki/Structured-Data#customizing-the-stored-data . not very familiar with quotation, but seems this can work : let exnProjection =
<@@ A.only<Exception>(fun e ->
[|
e.Message;
e.StackTrace;
e.Data;
e.InnerException.Message;
e.InnerException.StackTrace
|]) @@>
let dateExceptCycleReference = <@@ A.except<DateTime>(fun date -> [| date.Date; |]) @@>
getNames dateExceptCycleReference
getNames exnProjection
// output
> getNames dateExceptCycleReference
- ;;
val it : Projection = Projection ("DateTime",Except [["Date"]])
> getNames exnProjection
- ;;
val it : Projection =
Projection
("Exception",
Only
[["Message"]; ["StackTrace"]; ["Data"]; ["InnerException"; "Message"];
["InnerException"; "StackTrace"]]) when user want to except or only include the focused properties on one type, can be used as a convenient way for user custom destructure one type to |
@haf done with json format. see JsonFormatting.fs since user may have its own chiron encoder, so make chiron explicit dependency. The effect can be seen here : |
Now, only Can we consider only maintaining those targets that are being used (by us) in v5, if anyone complain or submit a issue, we can give information to help them build what they need. i think beyond |
I can port these. Especially since they are primarily the ones I use myself ;) |
So for Suave reporter, we need to deserialise and log it; just by referencing e.g. Chiron explicitly and doing it. |
The JSON formatting you linked looks really nice. |
no the idea here is : user can config formatting strategy for in Global.fs : let internal customJsonEncoderRegistry = lazy (
configJsonEncoder<PointName>(fun _ name -> E.string (name.ToString()))
configJsonEncoder<Gauge>(fun _ (Gauge(v,u)) ->
let (vs, us) = Units.scale u v
E.string (sprintf "%s %s" (vs.ToString()) us))
configJsonEncoder<Message>(fun resolver msg ->
JsonObject.empty
|> EI.required "name" (string msg.name)
|> EI.required "value" msg.value
|> EI.required "level" (string msg.level)
|> EI.required "timestamp" msg.timestamp
|> E.required resolver "context" msg.context
|> JsonObject.toJson)
...
let internal customDestructureRegistry = lazy (
configDestructure<Gauge>(fun _ req ->
let (Gauge (value, units)) = req.Value
let (scaledValue, unitsFormat) = Units.scale units value
if String.IsNullOrEmpty unitsFormat then ScalarValue scaledValue
else ScalarValue (sprintf "%s %s" (string scaledValue) unitsFormat))
configDestructure<Exception>(fun resolver req ->
let ex = req.Value
let refCount = req.IdManager
match refCount.TryShowAsRefId req with
| _, Some pv -> pv
| refId, None ->
let typeTag = ex.GetType().FullName
let nvs = [
yield { Name = "Message"; Value = ScalarValue ex.Message }
if not <| isNull ex.Data && ex.Data.Count > 0 then
yield { Name = "Data"; Value = req.WithNewValue(ex.Data) |> resolver }
if not <| isNull ex.StackTrace then
yield { Name = "StackTrace"; Value = ScalarValue (string ex.StackTrace) }
if not <| isNull ex.TargetSite then
yield { Name = "TargetSite"; Value = req.WithNewValue(ex.TargetSite) |> resolver }
if not <| isNull ex.Source then
yield { Name = "Source"; Value = ScalarValue (ex.Source) }
if not <| isNull ex.HelpLink then
yield { Name = "HelpLink"; Value = ScalarValue (ex.HelpLink) }
if ex.HResult <> 0 then
yield { Name = "HResult"; Value = ScalarValue ex.HResult }
if not <| isNull ex.InnerException then
yield { Name = "InnerException"; Value = req.WithNewValue(ex.InnerException) |> resolver }
]
StructureValue (refId, typeTag, nvs))
... for message : Message.eventFormat (Info, "this is bad, with {1} and {0} reverse.", "the first value", "the second value")
|> Message.setName (PointName.ofList ["a"; "b"; "c"; "d"])
|> Message.setNanoEpoch 3123456700L we can get json formatting like this: {
"name": "a.b.c.d",
"value": "this is bad, with {1} and {0} reverse.",
"level": "info",
"timestamp": 3123456700,
"context": {
"_fields.0": "the first value",
"_fields.1": "the second value"
}
} for message: let inner = new Exception("inner exception")
let e = new Exception("Gremlings in the machinery", inner)
Message.eventFormat (Info, "this is bad, with {1} and {0} reverse.", "the first value", "the second value")
|> Message.setName (PointName.ofList ["a"; "b"; "c"; "d"])
|> Message.setNanoEpoch 3123456700L
|> Message.addExn e we can get levelDatetimeMessagePathNewLine formatting like this: I 1970-01-01T00:00:03.1234567+00:00: this is bad, with "the second value" and "the first value" reverse. [a.b.c.d]
fields:
0 => "the first value"
1 => "the second value"
others:
_logary.errors =>
-
System.Exception {
Message => "Gremlings in the machinery"
HResult => -2146233088
InnerException =>
System.Exception {
Message => "inner exception"
HResult => -2146233088}} |
type User =
{
id : int
name : string
created : DateTime
}
type ProjectionTestOnly =
{
ex: exn
user: User
} let only = <@@ Destructure.only<ProjectionTestOnly>(fun foo ->
[|
foo.user.created.Day;
foo.ex.Message;
foo.ex.StackTrace;
foo.ex.Data.Count;
foo.ex.InnerException.Message
|]) @@>
Logary.Configuration.Config.configProjection only
let inner = exn "inner exception"
let e = new Exception("top", inner)
e.Data.Add(1,2)
e.Data.Add(3,4)
let foo = { id = 999; name = "whatever"; created = date20171111}
sampleMessage
|> Message.setContext "only" {ex = e; user= foo}
|> levelDatetimeMessagePathNewLine.format
|> fun actual ->
let expect = """
I 1970-01-01T00:00:03.1234567+00:00: this is bad, with "the second value" and "the first value" reverse. [a.b.c.d]
fields:
0 => "the first value"
1 => "the second value"
others:
only =>
ProjectionTestOnly {
user =>
User {
created =>
DateTime {
Day => 11}}
ex =>
Exception {
StackTrace => null
Message => "top"
InnerException =>
Exception {
Message => "inner exception"}
Data =>
ListDictionaryInternal {
Count => 2}}}
"""
Expect.equal actual (expect.TrimStart([|'\n'|]))
"formatting the message LevelDatetimePathMessageNl with projection" and for some complex type, we can support projection only/except some properties, like example above. maybe can provider some convenient. these cases can be found at Logary.Tests.Formatting.fs |
Ok, but in this case I need to deserialise JSON in a very specific format, so I guess we use the Chrion dependency that Logary has then? |
i think when deserialise in if we can control the json string format which post to what do you think? |
Yes, sure, makes sense. What about the underscore-prefixed fields in context now? Could you document those? We'd need to update https://github.com/logary/logary-js to match the new structure (moving all to context) We need to hide Chiron from any consumers of SuaveReporter, so it's probably better to make it an explicit dependency, since it also has Aether and FParsec as dependencies. (Need to hide it because it exports public symbols with name "Chiron.*") |
will document those in the new readme in next, right now can find them in Constants.fs , the idea is try to prevent conflict with user's context name or fields name.
|
in pr #219 ,
since try to use
obj
instead ofValue
andField
, the main problem here is in the old version, most target implementation depend onValue
union type, to do some transform, or useChiron Json.format
to format message in a json like style.but right now, things changed, all we have is the origin object boxed with
obj
, so i want to make a discussion with the targets maintainers about how to handle this situation.The text was updated successfully, but these errors were encountered: