[GitHub] [skywalking-nodejs] tom-pytel edited a comment on pull request #29: add plugin mysql

[GitBox <gi...@apache.org>]

tom-pytel edited a comment on pull request #29:
URL: https://github.com/apache/skywalking-nodejs/pull/29#issuecomment-785787227


   Some ideas for your plugin:
   * Wrap entire section after `newExitSpan()` with try / catch so that you can catch any exception from `_query.apply()` or even your own code and `span.error()` that and then `span.stop()` cleanly.
   * Put  static init stuff like `span.component = Component.MYSQL` before the query call so that if there is an exception the span will have as much information as possible.
   * Does `mysql2` do row streaming like `mysql`? If so it should be handled.
   * And a really silly detail, you don't need `span.errored = true` before `span.error(err)`, that is set by the `.error()` function.
   
   @kezhenxu94 asked me to review this because I also recently added a `mysql` plugin, just the first one, not `mysql2`, so you can look at that for some ideas of what I am talking about, its in master.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org