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

Questions re ESM support for MikroOrm / knex, Node v20, & OIDC #3870

Open
er-imageline opened this issue Feb 12, 2024 · 3 comments
Open

Questions re ESM support for MikroOrm / knex, Node v20, & OIDC #3870

er-imageline opened this issue Feb 12, 2024 · 3 comments
Labels
agent-nodejs Make available for APM Agents project planning. community

Comments

@er-imageline
Copy link

Is your feature request related to a problem? Please describe.
This is more of a question thread, but relates to problems auto-instrumenting ECMAScript / ESM modules.

Describe the solution you'd like
Auto-instrumentation of MikroOrm module, and OIDC flows if possible

Describe alternatives you've considered
Considered manual instrumentation but unsure how to proceed there.

Additional context
Stacktrace provided below.

Hello Team!

My company is an Elastic Cloud customer happily running a search application in production.

I'm an SRE with one of the software teams, and we are also trying to get APM set up for observability.

We've been trying to get ECMAScript / ESM auto-instrumentation set up for a service for some time with limited success. We are running it via package.json using:

node -r dotenv/config \
-r elastic-apm-node/start.js \
--experimental-loader=elastic-apm-node/loader.mjs \
--env-file .env \
dist/server.js"

It works in that some transactions / metrics are sent to our APM server, however it doesn't know about dependencies or databases.

Our service uses MikroOrm for database calls, and out of the box APM doesn't capture spans. When I enable the ELASTIC_APM_SPAN_STACK_TRACE_MIN_DURATION env var, it seems to prevent the DB client from initialising:

[discovery] - entity discovery finished, found 5 entities, took 9 ms
TypeError: Cannot read properties of undefined (reading 'Client')
    at MariaDbConnection.createKnexClient (/Users/myuser/myproject/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:132:66)
    at MariaDbConnection.createKnex (/Users/myuser/myproject/node_modules/@mikro-orm/mariadb/MariaDbConnection.js:7:28)
    at MariaDbConnection.connect (/Users/myuser/myproject/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:21:14)
    at MariaDbDriver.connect (/Users/myuser/myproject/node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:70:31)
    at MikroORM.connect (/Users/myuser/myproject/node_modules/@mikro-orm/core/MikroORM.js:101:46)
    at MikroORM.init (/Users/myuser/myproject/node_modules/@mikro-orm/core/MikroORM.js:44:23)
    at async file:///Users/myuser/myproject/dist/server.js:38:23
Retrying connection in 5 seconds...

I initially thought this was supported as MikroOrm uses knex under the hood, but now I see in the docs that the supported versions of knex are:

knex
>=0.20.0 <3 Also, only with pg@8.

Could someone advise what pg@8 refers to here? Not PostGres v8 surely?

Knex standalone is currently at 3.1.0, and I'm not exactly sure how MikroOrm invokes it. They apparently also use some 'monkey-patching' (their words) to apply some compatibility fixes to knex.

Also, our service has just been upgraded to node v20. I can see in Supported Node Versions that >=20.2.0 is listed, however there is an draft PR open for enable ESM support for Node v20. Could someone provide some clarity on what is / isn't supported in v20+?

Assuming I can't find a way to make auto instrumentation work with our project, do you think manual instrumentation might be possible in this kind of scenario?

For reference, here are the main components of our stack:

  • Node.js v20.9.0
  • TypeScript
  • MicroOrm v6.1.0
  • knex v3.1.0
  • MariaDB 10.6

Our service is also using oidc-provider for authentication. As a side note, it would be amazing if APM could (now or at some point in the future) somehow instrument OIDC flows, but for the moment some observability on DB interactions are what we need the most.

So, to summarise my questions:

Auto-instrumentation:

  • Is ESM auto-instrumentation supported on Node >v20?
  • Can you clarify what pg@8 means in relation to knex?
  • Does that strictly mean 'standalone' knex, or is it possible to auto-instrument a package invoking knex such as MikroOrm?

Manual instrumentation:

  • Failing that, any suggestions as to how might we manually instrument MikroOrm transactions?

Bonus: OIDC

  • Any plans to support instrumenting OIDC flows?
@github-actions github-actions bot added agent-nodejs Make available for APM Agents project planning. community triage labels Feb 12, 2024
@trentm
Copy link
Member

trentm commented Feb 13, 2024

Could someone advise what pg@8 refers to here? Not PostGres v8 surely?

That is referring to the pg npm package (https://www.npmjs.com/package/pg). Version 8.x is the latest version of pg -- though it of course supports versions of PostgreSQL later than postgres v8.

Auto-instrumentation:

* Is ESM auto-instrumentation supported on Node >v20?

Yes, ESM instrumentation is supported versions of Node.js v20 greater than or equal to v20.2.0 -- but only as of the latest v4.4.1 release:
https://www.elastic.co/guide/en/apm/agent/nodejs/current/release-notes-4.x.html#release-notes-4.4.1

Node v20.0.0 and v20.1.0 are not supported because there was a Node.js core bug.

* Can you clarify what  pg@8 means in relation to knex?

A few things here:

  1. The supported knex version range mentioned in https://www.elastic.co/guide/en/apm/agent/nodejs/current/esm.html#esm-compat-modules should say >=0.20.0 <4 -- i.e. that it supports knex versions 3.x. I will get that updated.
  2. The "knex"-specific instrumentation in the elastic-apm-node APM agent is very limited. It only adds functionality to improve stacktrace information on spans when the feature to enable span stacktraces is enabled. See https://www.elastic.co/guide/en/apm/agent/nodejs/current/supported-technologies.html#compatibility-better-stack-traces In addition enabling that feature is not recommended because collecting stacktraces in Node.js is slow and doing so for every collected APM span is too costly in practice (see https://www.elastic.co/guide/en/apm/agent/nodejs/current/performance-tuning.html#performance-span-stack-traces).
  3. On the ECMAScript module support doc page (https://www.elastic.co/guide/en/apm/agent/nodejs/current/esm.html#esm-compat-modules) the knex | >=0.20.0 <3 | Also, only with pg@8. table row is trying to say: "Yes, our knex instrumentation does work with ESM code. However, regularly our knex instrumentation only does its thing when using knex with postgres (pg module) or MySQL (mysql module) and, at this time, ESM support has not yet been added for the mysql module.
* Does that strictly mean 'standalone' knex, or is it possible to auto-instrument a package invoking knex such as MikroOrm?

Typically if the APM agent instruments a particular package A, and some package B uses package A, then that usage would also be instrumented. A good example is the mongoose package that uses the mongodb package under the hood. The APM agent instruments mongodb, so users of mongoose will see tracing data for MongoDB client requests.

Regarding your case:

  • I don't know how MikroOrm is using knex specifically, but I suspect that won't be the limiting issue for you.
  • The Knex docs (https://knexjs.org/guide/#node-js) say that its MariaDB support uses the mysql package. The APM agent (elastic-apm-node) does instrument mysql, however its current experimental ESM support does not yet support mysql.
  • I could be wrong about the mysql package being used, because I see that the MikroOrm docs (https://mikro-orm.io/docs/quick-start) mention installing @mikro-orm/mariadb, which looks like it uses mariadb as its database driver package.

So, IIUC, you are effectively requesting an enhancement that this APM agent support instrumenting mariadb. There is an older issue for this: #1759

If mysql (or perhaps mysql2) is the database driver package being used, then you need ESM support for this APM agent's instrumentation of mysql. Currently the best issue to track for that is #3445

Manual instrumentation:

* Failing that, any suggestions as to how might we manually instrument MikroOrm transactions?

Perhaps configure your usage of MikroOrm to register handlers for Lifecycle Hooks or subscribe to its events -- see https://mikro-orm.io/docs/events -- and then add manual span-creation tracing (e.g. https://www.elastic.co/guide/en/apm/agent/nodejs/current/agent-api.html#apm-start-span) there?

Bonus: OIDC

* Any plans to support instrumenting OIDC flows?

Nothing specific to OIDC, currently, no.

@trentm trentm removed the triage label Feb 13, 2024
@er-imageline
Copy link
Author

Hi Trent,

Thanks for the quick & detailed reply on this! It's much appreciated.

Yes, ESM instrumentation is supported versions of Node.js v20 greater than or equal to v20.2.0 -- but only as of the latest v4.4.1 release:
https://www.elastic.co/guide/en/apm/agent/nodejs/current/release-notes-4.x.html#release-notes-4.4.1

That's great - I had missed that update.

 it supports knex versions 3.x.

Also glad to hear this!

Regarding knex instrumentation & behaviour, I understand that it is only enabled when spanStackTraceMinDuration is set, & that it's resource-heavy; we intend to enable it only for debugging purposes, not in Production.

Still, I'm surprised it crashes in this way. I'll check if there's a resource spike but I don't think so - it's possible this behaviour is due to some implementation detail in the code - if I have time I'll try to repro this in a shareable way.

So, IIUC, you are effectively requesting an enhancement that this APM agent support instrumenting mariadb. There is an older issue for this: #1759

As it happens, it now turns out we have to migrate to MySQL as Azure is dropping managed MariaDB. So we can un-request MariaDB support, but consider this a +1 for ESM/MySQL support :) - I took a look at the Project page and I guess there's no ETA there?

Perhaps configure your usage of MikroOrm to register handlers for Lifecycle Hooks or subscribe to its events -- see https://mikro-orm.io/docs/events -- and then add manual span-creation tracing (e.g. https://www.elastic.co/guide/en/apm/agent/nodejs/current/agent-api.html#apm-start-span) there?

Thanks for this tip! I guess the big thing I wanted to verify is that fact that manual instrumentation is still possible with TypeScript/ESM apps, and if I understand correctly it is.

For now I will go modify things to use MySQL and try to add some manual instrumentation.

Typically if the APM agent instruments a particular package A, and some package B uses package A, then that usage would also be instrumented. A good example is the mongoose package that uses the mongodb package under the hood. The APM agent instruments mongodb, so users of mongoose will see tracing data for MongoDB client requests.

This is great practical info! Maybe it's basic/assumed stuff for seasoned JS devs, but I think this paragraph would be helpful in the docs & give users a better intuition of how APM works.

Finally, thanks for the answer re OIDC. I presume we could manually instrument these calls also, right? As they tend to be simple enough, along the lines of try{oidcProvider.dosomething()} and wait for a response.

@trentm
Copy link
Member

trentm commented Mar 6, 2024

but consider this a +1 for ESM/MySQL support :) - I took a look at the Project page and I guess there's no ETA there?

Correct. Unfortunately I don't have an ETA.

Finally, thanks for the answer re OIDC. I presume we could manually instrument these calls also, right?

Yes, I believe so. I haven't worked with OIDC libraries myself, so I can't be sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. community
Projects
None yet
Development

No branches or pull requests

2 participants