RE: REST GridCacheCommandHandler writes ERROR in log in caseofbaduser input

[Stanislav Lukyanov <st...@gmail.com>]

More food for thought - here are JCL guidelines that look close to what I’ve suggested:
http://commons.apache.org/proper/commons-logging/guide.html#JCL_Best_Practices

Big thanks to Oleg Ignatenko for the pointer.

Thanks,
Stan 

From: Stanislav Lukyanov
Sent: 20 декабря 2018 г. 17:20
To: dev@ignite.apache.org
Subject: RE: REST GridCacheCommandHandler writes ERROR in log in caseofbaduser input

I think that the error detected in the server’s SQL engine is INFO or even DEBUG – for the server it is just a normal workflow.

At the same time the error needs to be propagated to the client – and shown to the user as an exception, error code in the response, etc.

Stan

From: Ilya Kasnacheev
Sent: 20 декабря 2018 г. 16:31
To: dev@ignite.apache.org
Subject: Re: REST GridCacheCommandHandler writes ERROR in log in case ofbaduser input

Hello!

Where should user-specified SQL errors and REST call parameter errors fall
in this scheme?

Regards,
-- 
Ilya Kasnacheev


чт, 20 дек. 2018 г. в 16:12, Stanislav Lukyanov <st...@gmail.com>:

> +1.
>
> ERROR and even WARNING messages are often treated as incidents in the
> production systems.
> Ignite produces a lot of these messages though.
>
> I’d suggest the following as a rule of thumb:
> - ERROR – we’re sure there is an issue with a node/cluster that needs
> attention; cluster stability and/or data integrity are in danger
> - WARNING – we’re not sure, but something might be wrong – cluster
> stability and/or data integrity could be in danger; we can’t verify if
> that’s an error or not, so user attention is needed
> - INFO – anything that we’d like to have in the logs which doesn’t
> indicate an unrecoverable problem (or possibility of it)
> - DEBUG – anything that we’d like to have in the logs but which would
> produce too much logging; can be enabled on testing environments for
> selected packages though
> - TRACE – anything that we’d like to have for deep debugging which
> probably wouldn’t help anyone but Ignite developers; can produce enormous
> amount of logs
>
> WDYT?
>
> Stan
>
> From: Sergey Kozlov
> Sent: 20 декабря 2018 г. 15:53
> To: dev@ignite.apache.org
> Subject: Re: REST GridCacheCommandHandler writes ERROR in log in case of
> baduser input
>
> Denis
>
> I can't agree with you.
>
> Apache Ignite is the compute platform. It means that an exception on a
> server node looks like an alarm that something went wrong or even broken.
> I think if we can catch the mistake from user request and return the
> correct response (e.g. with error code) is good enough and does not require
> throw the exception on nodes.
> From the that standpoint the WARNING or even DEBUG is most suitable level
> to report such issues. Same approach may be used more widely for instance
> for wrong sql syntax.
>
>
>
>
>
> On Fri, Dec 14, 2018 at 9:35 PM Denis Magda <dm...@apache.org> wrote:
>
> > Hello Ilya,
> >
> > It's fine to use ERROR level if an operation can't be completed due to
> > missing parameters. That's, in fact, an exception/error. What needs to be
> > changed is texts of messages so that everyone understands what exactly
> > happened and how to address a failure.
> >
> > --
> > Denis
> >
> >
> > On Fri, Dec 14, 2018 at 5:45 AM Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com
> > >
> > wrote:
> >
> > > Hello!
> > >
> > > It seems that we have subj behavior since the earliest days if Apache
> > > Ignite.
> > >
> > > If you send a REST command with error in it (such as missing "keys" for
> > > getAll), you will get a nice
> > > [2018-10-30 22:22:14,021][ERROR][rest-#61061][GridCacheCommandHandler]
> > > Failed to execute cache command: GridRestCacheRequest
> > > error in your logs.
> > >
> > > Which is probably an overkill since ERROR usually means unexpected and
> > > severe errors in application code as opposed to user input validation
> > > errors.
> > >
> > > This made worse by the fact that you can have some automatic REST
> client
> > do
> > > a lot of incorrect requests, spam your logs with thousands of such
> > ERRORs.
> > > The error is returned to client but it is also tee'd to log.
> > >
> > > What we could do:
> > > - Move log level from ERROR to WARN or even INFO.
> > > - Handle REST user input validation errors differently from Ignite
> > internal
> > > errors by introducing new exception class, logging it as INFO or maybe
> > just
> > > returning to user.
> > > - Third funny option?
> > >
> > > WDYT?
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> > >
> >
>
>
> --
> Sergey Kozlov
> GridGain Systems
> www.gridgain.com
>
>