Generally speaking I think this is a nice idea. A few questions based on your prototype:
1. Is there any reason you wouldn't want to prefer the whole RequestMessage over just the request id as the first argument to all of those methods? 2. For onQueryError() I'd wonder if it isn't better to use the actual Throwable rather than just the error message 3. For onQueryTimeout() does this get called in addition to onQueryError() - I suppose it doesn't? I sense you're mapping to the GremlinExecutor lifecycle, but I wonder if that's right? Maybe for a Graph Providers' purposes it's better to get a single call for an error (where a timeout is a form of an error) and the GraphManager implementation figures out what to do with it? Overall, what you have seems like a good start and proof that this approach holds water. Thanks! On Tue, Oct 4, 2022 at 1:54 PM Thomas Kolanko <[email protected]> wrote: > I created https://issues.apache.org/jira/browse/TINKERPOP-2806 to enhance > the GraphManager interface to allow providers to opt into > receiving notifications when scripts/queries are processed. > > Ideally what we would like as a provider is to be able to tell when a query > starts, completes, errors or encounters a timeout so we can provide metrics > showing success / error rates by graph. Currently as a provider the > gremlin-server is a black box where you have no insight to what is being > processed. TINKERPOP-2806 proposes adding some default methods to the > GraphManager interface which will allow providers to opt into handling the > lifecycle of script/query execution. I created a prototype of what this > could look like at > > https://github.com/apache/tinkerpop/compare/3.6-dev...tkolanko:tinkerpop:3.6-dev?expand=1 > > Inside AbstractEvalOpProcessor and TraversalOpProcessor I am getting the > GraphManager from context and calling the appropriate method depending > where in the lifecycle the script/query processing is >
