Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mysql2 probe #541

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

realhidden
Copy link

Adds a new mysql2 probe (for package https://www.npmjs.com/package/mysql2), the mysql2 package is compatible with the original mysql one, so I used the same code and emitting 'mysql' data to be 100% compatible with the original probe and the appmetrics-dash.

Emitting mysql data to keep compatibility with the original mysql probe
@codecov-io
Copy link

codecov-io commented Sep 2, 2018

Codecov Report

Merging #541 into master will decrease coverage by 0.3%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #541      +/-   ##
==========================================
- Coverage    58.5%   58.19%   -0.31%     
==========================================
  Files          48       49       +1     
  Lines        2947     2983      +36     
==========================================
+ Hits         1724     1736      +12     
- Misses       1223     1247      +24
Impacted Files Coverage Δ
probes/mysql2-probe.js 33.33% <33.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a856a9...1c8f21e. Read the comment docs.

@sjanuary
Copy link
Contributor

@realhidden - thank you for the contribution. Please read https://github.com/RuntimeTools/appmetrics/blob/master/CONTRIBUTING.md for instructions on how to contribute to appmetrics - your first PR needs to add your name into AUTHORS.md as an indication that you agree to the Contributor Licence Agreement.

It doesn't seem like we have a test for the mysql probe at the moment, would you be interested in adding one for this new probe? Tests under https://github.com/RuntimeTools/appmetrics/tree/master/tests/probes are not run by default if they require something like an external database but they are useful when writing new probes, testing changes to a probe and fixing bugs.

* See the License for the specific language governing permissions and
* limitations under the License.
*******************************************************************************/
'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If mysql and mysql2 have identical APIs, can this file not be module.exports = require('./mysql-probe');? This might side-step test concerns, because it wouldn't introduce any new code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants