CA-285213 add error logging to memory interface #209
Conversation
|
The Travis failure is unrelated to this PR, something gone wrong with all the preprocessors we have: #206 (comment) |
edwintorok
left a comment
edwintorok
left a comment
There was a problem hiding this comment.
Good start, might need to log some more details on the location and arguments.
| { def = errors | ||
| ; raiser = (fun e -> | ||
| let exn = MemoryError e in | ||
| error "%s (%s)" (Printexc.to_string exn) __LOC__; |
There was a problem hiding this comment.
Miight want to add D.log_backtrace () too.
| raise exn) | ||
| ; matcher = (function | ||
| | MemoryError e as exn -> | ||
| error "%s (%s)" (Printexc.to_string exn) __LOC__; |
There was a problem hiding this comment.
I think you need to register a pretty printer for this to work:
#require "xcp.memory";;
let e = Memory_interface.MemoryError (Memory_interface.Cannot_free_this_much_memory (42L, 42L));;
val e : exn =
Memory_interface.MemoryError
(Memory_interface.Cannot_free_this_much_memory (42L, 42L))
Printexc.to_string e;;
- : string = "Memory_interface.MemoryError(_)"
let string_of_error e = Rpcmarshal.marshal Memory_interface.errors.Rpc.Types.ty e |> Rpc.to_string;;
val string_of_error : Memory_interface.errors -> string = <fun>
let register_memory_error () =
Printexc.register_printer (function
| Memory_interface.MemoryError e -> Some (Printf.sprintf "Memory_interface.Memory_error(%s)" (string_of_error e)) | _ -> None);;
val register_memory_error : unit -> unit = <fun>
register_memory_error ();;
- : unit = ()
Printexc.to_string e;;
- : string =
"Memory_interface.Memory_error([S(Cannot_free_this_much_memory);[I(42);I(42)]])"
There was a problem hiding this comment.
(very bare bones pretty printer, maybe using Jsonrpc.to_string would make it look nicer?)
There was a problem hiding this comment.
Interesting. I always assumed that Printexc.to_string e would be able to inspect any exception.
There was a problem hiding this comment.
Yeah I don't see why the compiler couldn't autogenerate these pretty printers, it seems it only has trouble when the argument of the exception is an exception with a variant (otherwise it is able to at least print the constructor as a number)
There was a problem hiding this comment.
Maybe we could autogenerate these in ppx_deriving_rpc though?
There was a problem hiding this comment.
That's fairly annoying. We can investigate what to do, it may be worth opening an issue on ocaml-rpc (and maybe also in Jira)
There was a problem hiding this comment.
Can you please do the same also for the other ported modules, I think v6, networkd and maybe rrd.
There was a problem hiding this comment.
I am going to port this change to the Error functor in ocaml-rpc
|
I have added logging for the other interfaces as well. |
mseri
left a comment
mseri
left a comment
There was a problem hiding this comment.
Thanks. There is a fixup commit that needs squashing
This logs all errors flowing across the interface to squeezed. It adds code to install a custom printer for MemoryError exceptions such that we actually will get meaningful log messages. Signed-off-by: Christian Lindig <[email protected]>
This adds logging for all errors passing the network interface. Signed-off-by: Christian Lindig <[email protected]>
All errors passed through the V6 interface are now logged. Signed-off-by: Christian Lindig <[email protected]>
This adds logging for all errors passing through the Gpumon_interface. Signed-off-by: Christian Lindig <[email protected]>
3 participants
This logs all errors flowing across the interface to squeezed.
Signed-off-by: Christian Lindig [email protected]