"format is not a function" error when using mysql2 instrumentation with kysely
#1511
Comments
pichlermarc
added
bug
pichlermarc
changed the title
kysely
|
Hi @jballentine-sharpen thanks for reporting this. Looks like this is not an instrumentation-request but a bug report. An instrumentation should not interfere with the library's functionality. If possible, please provide a minimal reproducer. Before we start working on an issue we need to know exactly what's causing the behavior. Issues usually get fixed way quicker if a reproducer repository is at hand. 🙂 FYI @haddasbronfman |
pichlermarc
added
the
needs:reproducer
|
Thanks for the response. I unfortunately no longer have time to play around with this anymore and I got rid of the code I had due to me pivoting to using drizzle. However, it should be very easy to reproduce. It happened to be after simply setting up a really basic select query using the mysql2 information on their docs (I ran into this pretty much right away when trying to use this package). https://kysely.dev/docs/getting-started?dialect=mysql#dialects It's really not worth me writing up code since you'd need to modify it to fit your database connection and schema. If writing up a minimal reproducer is absolutely required, I will try to find some time to do so, but it is not really my top priority at the moment. Thanks. |
|
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
I am seeing this too, Not in kysely but with a straightforward node app writing mysql.
As compiled it looks like: and errors when I try to do an As this is preventing my app from writing data while instrumented it's a critical failure |
|
Which of your examples is testing this? with format being undefined, it gives the TypeError |
|
Seeing this too, Knex uses mysql2 under the hood. I've narrowed it down the the Inside _patchQuery it tries to use the This should be a pretty simple patch no? |
|
This issue reproduces in ESM mode on Node.JS 20.x. I stopped inside instrumentation using debugger and what I'm seeing is that This seems to me like a |
|
So I tried to reproduce this with When I'm hooking |
|
I was going through the same problem today, a solution I found was to analyze what type of mysql2 I am using, whether it is the normal one or mysql2/promise, I did not get to check completely but I assume that this instrumentation is on top of mysql2/promise, try using mysql2/promise and your problem will be solved and openTelemetry will do the tracing of your application, mysql2 in version ^3.11.0 worked I do not know the others |
|
I did a "little hack" and it work fine with Kysely, Knex should work also:
// File: loader.cjs
const { registerInstrumentations } = require("@opentelemetry/instrumentation");
const { HttpInstrumentation } = require("@opentelemetry/instrumentation-http");
const {
SocketIoInstrumentation,
} = require("@opentelemetry/instrumentation-socket.io");
const { PinoInstrumentation } = require("@opentelemetry/instrumentation-pino");
const {
MySQL2Instrumentation,
} = require("@opentelemetry/instrumentation-mysql2");
const {
FastifyInstrumentation,
} = require("@opentelemetry/instrumentation-fastify");
registerInstrumentations({
instrumentations: [
new FastifyInstrumentation(),
new MySQL2Instrumentation(),
new HttpInstrumentation(),
new SocketIoInstrumentation(),
new PinoInstrumentation(),
],
});
// load after instrumentation
const mysql = require("mysql2/promise");
const mysqlCommon = require("mysql2");
// Verify if the format function is missing and add it if necessary
if (!mysql.format) {
mysql.format = mysqlCommon.format;
}And I required it in my node command (TSX in my case): tsx watch --require 'dotenv/config' --require './loader.cjs' --require '@opentelemetry/auto-instrumentations-node/register' src/main.ts |
|
I set up a simple repo that demonstrates the error: https://github.com/tpraxl/test-esm-mysql-otel It is an esm node.js application that inserts a value into a table. Maybe this helps tracking the error down. |
|
@tpraxl as my project is also esm and the format is missing into mysql2 (without promise). Maybe the responsabilty of format should be handled by @opentelemetry/instrumentation-mysql2 as the output needed is for telemetry. Because this work in commonjs and esm ( I tested it yesterday) |
|
@clifinger you could argue that format should be the responsibility of @opentelemetry/instrumentation-mysql2 – in that case, it should depend on sqlstring and use its However, I am wondering what the point would be, because in order to instrument mysql2, we already heavily depend on its correctness and completeness, as far as I get it. Having no By removing the circular dependencies in mysql2, this bug vanishes. |
The solution is not working with |
@luiz-rissardi we use Kysely in our project and this solution work fine for us |
daria pra testar algumas outras alternativas viáveis sobre esse problema, mas acho que o @clifinger já conseguiu algo semelhante ao que você gostaria, e pra falar a verdade tenho que tirar um tempo extra pra tentar resolver esse "Bug" do open telemetry , thanks for all bros sksksk |
|
@smhmayboudi See my last answer, it work for us with Kysely ;) |
|
@pichlermarc I think I found some kind of reproduction for this bug. It's not entirely an error, but it does produce some similar looking warnings. package.json: {
"name": "otel-mysql2",
"version": "0.0.1",
"main": "index.js",
"type": "module",
"scripts": {
"start": "cross-env OTEL_PROPAGATORS=tracecontext OTEL_TRACES_EXPORTER=console OTEL_NODE_RESOURCE_DETECTORS=none node --import ./register.js ./index.js"
},
"author": "Konstantin Bryzgalin <[email protected]>",
"license": "UNLICENSED",
"description": "",
"dependencies": {
"@opentelemetry/instrumentation": "0.53.0",
"@opentelemetry/instrumentation-mysql2": "0.41.0",
"@opentelemetry/sdk-node": "0.53.0",
"cross-env": "7.0.3",
"mysql2": "3.11.3"
},
"volta": {
"node": "18.20.4"
}
}register.js: import { register } from 'node:module';
import { MySQL2Instrumentation } from '@opentelemetry/instrumentation-mysql2';
import { node, NodeSDK } from '@opentelemetry/sdk-node';
const OPENTELEMETRY_SDK = Symbol('openTelemetrySdk');
register('@opentelemetry/instrumentation/hook.mjs', import.meta.url);
globalThis[OPENTELEMETRY_SDK] = new NodeSDK({
serviceName: 'test',
instrumentations: [new MySQL2Instrumentation({ enabled: true })],
});
globalThis[OPENTELEMETRY_SDK].start();
const shutdownFn = async () => globalThis[OPENTELEMETRY_SDK].shutdown().then(() => console.log('shutdown')).catch((error) => console.error(error));
let shutdownStarted = false;
const beforeExitHandler = () => {
if (shutdownStarted) return;
shutdownStarted = true;
void shutdownFn().then(() => process.removeListener('beforeExit', beforeExitHandler));
};
process.on('beforeExit', beforeExitHandler);
const signals = ['SIGHUP', 'SIGINT', 'SIGQUIT', 'SIGILL', 'SIGTRAP', 'SIGABRT', 'SIGBUS', 'SIGFPE', 'SIGSEGV', 'SIGUSR2', 'SIGTERM'];
const signalHandler = (signal) => {
if (shutdownStarted) return;
shutdownStarted = true;
shutdownFn().then(() => {
signals.forEach((s) => process.removeListener(s, signalHandler));
process.kill(process.pid, signal);
});
};
signals.forEach((s) => process.on(s, signalHandler));index.js: import { format } from 'mysql2';
console.log(format('select ?', [1]));produced output: As you can see the The FIX for the problem is even weirder! Let's replace first line in And second, lets add right after SDK start() is called: And the output is: Somehow, requiring CommonJS-style a 'mysql2' package after the instrumentation is installed (but before it's ESM-way imported!), completely fixes the problem! This is the second time I'm seeing that sort of error, actually. Take a look at open-telemetry/opentelemetry-js#5024 and there seems to be a pattern. |
|
I am currently attempting to merge fixes to mysql2. It will fix the missing format function error by reducing the amount of circular dependencies: sidorares/node-mysql2#3081 I also studied the documentation of opentelemetry once more and found out that I should have started the instrumentation with the additional argument These two fixes allow us to have instrumentation for the callback based variant of mysql2 (promise variant worked before). I provided a test repo showing all cases and instructions on how to test it with the (not yet merged) fixes to mysql2: https://github.com/tpraxl/test-esm-mysql-otel |
Is your instrumentation request related to a problem? Please describe
Kysely currently breaks when using OpenTelemetry. When I attempt to execute a query, I get an error of
TypeError: format is not a functionas well as some warnings ofWarning: Accessing non-existent property 'format' of module exports inside circular dependency, with the error coming from@opentelemetry/instrumentation-mysql2/src/utils.ts:105:21. and caused bykysely/dist/esm/dialect/mysql/mysql-driver.js:117:33It appears the mysql instrumentation breaks Kysely functionality. It would be great to be able to use both Kysely and otel without this conflict.
Is it applicable for Node or Browser or both?
Node
Do you expect this instrumentation to be commonly used?
Weekly Downloads: 59,839
What version of instrumentation are you interested in using?
Versions: Latest?
Additional context
I think instrumentation would just work as it uses mysql/pg under the hood, it just needs this conflict to be resolved.
The text was updated successfully, but these errors were encountered: