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

mysql plugin does not propagate fields parameter in query callback wrapper #1324

Closed
shuaibiyy opened this issue Nov 19, 2020 · 1 comment · Fixed by #1412
Closed

mysql plugin does not propagate fields parameter in query callback wrapper #1324

shuaibiyy opened this issue Nov 19, 2020 · 1 comment · Fixed by #1412
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@shuaibiyy
Copy link

Environment details

  • OS: 10.15.7
  • Node.js version: v10.22.1
  • npm version: 6.14.6
  • @google-cloud/trace-agent version: 5.1.1

Steps to reproduce

In the mysql plugin's callback wrapper, the fields parameter is not accepted and forwarded.

const fn = function (err, res) {

It should be like this based on the mysql api:

function wrapCallback(api, span, done) {
    const fn = function (err, res, fields) { // <-- additional `fields` parameter
        if (api.enhancedDatabaseReportingEnabled()) {
            if (err) {
                span.addLabel('error', err);
            }
            if (res) {
                span.addLabel('result', res);
            }
        }
        span.endSpan();
        if (done) {
            done(err, res, fields); // <-- pass `fields` argument
        }
    };
    return api.wrap(fn);
}

I'm happy to create a PR to fix this. Thanks!

@product-auto-label product-auto-label bot added the api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. label Nov 19, 2020
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Nov 20, 2020
@bcoe bcoe added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Nov 24, 2020
@yoshi-automation yoshi-automation removed triage me I really want to be triaged. 🚨 This issue needs some love. labels Nov 24, 2020
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Feb 22, 2021
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label May 18, 2021
@dnoe
Copy link
Contributor

dnoe commented Oct 19, 2021

Hi, sorry for the delay in response - the team I am now on at Google has recently taken over the Cloud Trace product and we are looking through some of these old bugs! I am not super familiar with node.js, but it sounds like you have a solid fix in mind. Are you still willing to send us a PR so I can review it and get it merged ASAP? That would be extremely helpful. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants