mysql2 promise support

[vkruoso]

mysql2 promise support

[michaelgoin]

Hi @vkruoso,

Have you tried using this module directly?

It should support mysql2/promise. It is not the instrumentation included in the agent by default.

If you do npm install @newrelic/mysql this module will take over instrumentation and may give you visibility into what you want.

That being said, its obviously not awesome our instrumentation has this awkward split. Also, ORMs or other libraries (such as knex) can cause transaction state loss in which case we often will fail to be able to report data for things we do instrument. If you find 'knex' is a specific issue, I'd submit (or thumbs-up an existing) feature request for knex over on the main node-newrelic repo.

Thanks!

[vkruoso]

Thanks for the fast answer. I though this package was used internally via the recommended newrelic module. Will try to add it and see if the instrumentation starts to work with the particular setup of knex and objectionjs. Thanks.

[bizob2828]

@vkruoso were you able to get this working with the @newrelic/mysql module?

[edwinveldhuizen]

We're having the same issue actually.

This is the basic implementation we have:

import { Pool, OkPacket, RowDataPacket, FieldPacket } from "mysql2/promise"
import Cache from "./cache"

export class ErrDuplicateEntry extends Error {
    message: string = 'Duplicate entry found while inserting or modifying row.'
}

export class ErrForeignKeyConstraint extends Error {
    message: string = 'Foreign key constraint failed while inserting or modifying row.'
}

export default class Database {
    private readonly connPool: Pool
    public readonly cache: Cache

    constructor(connPool: Pool, cache: Cache) {
        this.connPool = connPool
        this.cache = cache
    }

    async ping(): Promise<void> {
        await this.select('SELECT 1', {});
    }

    async execute(query: string, args?: { [key: string]: any }): Promise<[OkPacket, FieldPacket[]]> {
        if (args) {
            args = convertArgsToStrings(args);
        }

        return this.connPool.execute<OkPacket>(query, args);
    }

    async select(query: string, args: { [key: string]: any }): Promise<[RowDataPacket[], FieldPacket[]]> {
        if (args) {
            args = convertArgsToStrings(args);
        }
        return this.connPool.execute<RowDataPacket[]>(query, args);
    }

    /*
    * @param query MySQL query to send to database
    * @param args Placeholder values for MySQL query
    * @param key Cache key for Redis
    * @param ttl Time to Live in seconds
    * @param refreshCache Set to true to ignore cached result and force refresh
    */
    async selectCache(query: string, args: { [key: string]: any }, key: string, ttl: number, refreshCache?: boolean): Promise<[RowDataPacket[], FieldPacket[]]> {
        return this.cache.select(key, ttl, this.select.bind(this), [query, args], refreshCache);
    }
}

/**
 * This function is a temporarily solution for the following issue:
 * https://github.com/sidorares/node-mysql2/issues/1239
 * @param args
 */
function convertArgsToStrings(args: { [key: string]: any }): { [key: string]: any } {
    Object.keys(args).forEach((key) => {
        if (typeof args[key] === "number") {
            args[key] = args[key] + "";
        }
    });
    return args;
}

And using node --require @newrelic/mysql
And env variables:

NEW_RELIC_RECORD_SQL=raw
NEW_RELIC_SLOW_SQL_ENABLED=true

Yet, the db section in APM is only showing redis calls, so are the traces.

[michaelgoin]

@edwinveldhuizen I was looking at this recently for a different situation and it looks like we don't have instrumentation for the pool execute function. So that is likely the cause of the issue here.

[vkruoso]

@bizob2828 sorry for the delay. We did install the package and I think we are in the same situation that @edwinveldhuizen mentioned.

[michaelgoin]

@vkruoso thank you for the info and sorry for the limitation. I added an issue specific to that issue: #59.

We have plans to consolidate the mysql instrumentation into a single point and ensure everything works. We'll be creating an issue for that and then likely closing this out in favor of that making everything straightforward and working out of the box.

[vkruoso]

That is great to hear! Thanks for the update.

[bizob2828]

It's worth noting that not much has diverged between the mysql instrumentation within agent and the external module. The > is the agent and > is external module

8c8,11
< exports.callbackInitialize = function callbackInitialize(shim, mysql) {
---
> const dbutils = require('../db/utils')
> const properties = require('../util/properties')
> 
> module.exports = function initialize(agent, mysql, moduleName, shim) {
48,55d50
< exports.promiseInitialize = function promiseInitialize(shim) {
<   const callbackAPI = shim.require('./index')
< 
<   if (callbackAPI && !shim.isWrapped(callbackAPI.createConnection)) {
<     exports.callbackInitialize(shim, callbackAPI)
<   }
< }
< 
127a123,124
> 
>   let describe
129c126
<     shim.recordQuery(proto, 'query', describePoolQuery)
---
>     describe = describePoolQuery
131c128
<     shim.recordQuery(proto, 'query', describeQuery)
---
>     describe = describeQuery
134a132,137
>   shim.recordQuery(proto, 'query', describe)
> 
>   if (queriable.execute) {
>     shim.recordQuery(proto, 'execute', describe)
>   }
> 
152,153c155,156
<   if (shim.isArray(args[1]) || (!args[1] && shim.isFunction(args[2]))) {
<     // query({opts|sql}, values, callback) - values can be undefined
---
>   if (shim.isArray(args[1])) {
>     // query({opts|sql}, values, callback)
211c214
<     if (conf.socketPath) {
---
>     if (properties.hasOwn(conf, 'socketPath') && conf.socketPath) {
229c232
<     const databaseName = shim.getDatabaseNameFromUseQuery(query)
---
>     const databaseName = dbutils.extractDatabaseChangeFromUse(query)