differential-datalog icon indicating copy to clipboard operation
differential-datalog copied to clipboard

Avoid logging json with exception stack trace in applyUpdate method

Open roymanish opened this issue 3 years ago • 5 comments

While trying to call applyUpdates in DDLog if it fails due to missing input relation then it logs DDLogException. The exception message concatenates json input with it. At time this json can be huge and not recommended to be logged.

Steps to reproduce : Try to send a random json to an input relation that does not existing using the Java API.

roymanish avatar May 20 '21 07:05 roymanish

The main question we have to answer is whether this should be done in Rust or in Java. If it is to be done in Rust, then I can imagine some static or dynamic configuration options for the ddlog runtime, e.g., ddlog.maxErrorLength. I could start adding a Java capability anyway to move this forward.

mihaibudiu avatar May 20 '21 16:05 mihaibudiu

@roymanish , why not just truncate the message before logging it?

ryzhyk avatar May 20 '21 21:05 ryzhyk

@roymanish so the quick solution is for you to get the exception message and truncate it to some max size before handling it. I agree that this is wasteful, but at least it does not require us to implement a policy for truncation within DDlog or the Java API. Please let me know if this solution would be adequate for you.

mihaibudiu avatar May 20 '21 21:05 mihaibudiu

I can handle this on my end but why do we not want to fix this in ddlog itself? Do we have use cases where logging the json is always required. I mean we can just separate the exception message and json data in two separate fields so that whoever needs can log the json.

roymanish avatar May 23 '21 09:05 roymanish

This has nothing to do with JSON; DDlog does not even have a concept of JSON, JSON is just an external datatype like any other, and it is not handled specially in DDlog.

Error messages produced by the DDlog runtime are not structured -- they do not have separate message and data fields. The DDlog runtime returns a Rust type of Result<T, String>. Changing this API would be a breaking change for all programs and all wrappers, including the C API and the Java API, which would need to be modified to accommodate the new structure, so it's not an easy change to make. The only way to make it without breaking all DDlog client programs would be to add parallel APIs that return Result<T, (String, String)>, or, even better, define a DDlogError struct and return Result<T, DDlogError> from all these methods. This may be a good idea, but it will take some work. Right now we assume that errors are just opaque strings. This makes interaction with C easier too. (The Java API is built on top of the C API which is built on top of the Rust API).

In Java since you can catch exceptions and handle error messages differently depending on the context (e.g., errors produced on insertion vs errors produced when starting a transaction). You have more context about where the error occurred and you can apply different policies to the error handling.

mihaibudiu avatar May 24 '21 17:05 mihaibudiu