no pool.name in mysql (javascript)

[haddasbronfman]

What are you trying to achieve?
I wanted to implement this metric https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/database-metrics.md#connection-pools and add the attribute pool.name for MySQL DB in javascript.

What did you expect to see?
I expected that the MySQL library will have a field that identifies the pool. something like a name/id that distinguishes it from other pools.

Since there is no such field I wonder what should I do - should I create such an identifier myself? It will cause a weird situation where the user sees an attribute like pool.name=4 that he didn't declare by himself...

the code of the pool in Mysql is here: https://github.com/mysqljs/mysql/blob/master/index.js#L22
You can see the it is possible to give a ConnectionConfig when creating the pool, but I can’t find any ‘ID’ or 'name' field in this object: https://github.com/mysqljs/mysql/blob/dc9c152a87ec51a1f647447268917243d2eab1fd/lib/ConnectionConfig.js#L12

[mateuszrzeszutek]

Ideally the pool.name should be something that makes it easy to connect metrics to the code that emits them - in case you have multiple pools and one of them is misbehaving, you want to be able to quickly find the code that configures it. For the same reason, it should also be unique - until now, all the connection pool implementations that I saw had implemented this sort of concept.

The PoolConfig that you linked object accepts a connection string (or a config object; but you can stringify it probably) - WDYT about using it as a name? I mean, it's not really the "name", but it should be both possible to identify the code that sets it up, and should be somewhat unique (I agree that this is a somewhat weak assumption, but on the other hand why would you want to configure 2 separate pools to the same data source in one application?)

[haddasbronfman]

@mateuszrzeszutek sound like a good idea. I'll implement this. thanks.

[haddasbronfman]

@mateuszrzeszutek on another thinking, the 'config' contains all the details to connect to the db - including the password. Any idea what to do with this?

[mateuszrzeszutek]

How about only using selected fields to construct the name? E.g. host, port, database, user (an allowlist would probably be much safer than a list of exclusions)

[haddasbronfman]

@mateuszrzeszutek I think it's a good idea. But if I'm not mistaken, the createPool() function can get either a URL (string) or config with host, port, database, etc.. So in case this is a URL I think I should omit the password, right?

[mateuszrzeszutek]

So in case this is a URL I think I should omit the password, right?

Yeah, that'd make the most sense. Alternatively (not sure if this is possible), you could inject instrumentation after the URL is parsed into a config object.

[haddasbronfman]

I think we can close the issue.

1. Pool: We agreed that the pool.name will look like this: host: localhost port: 3306 database: db_test user: root
   if one of the fields is missing, it won't appear.

2. PoolCluster: We will use the id of the PoolCluster if it is given. If not, use the format from test PR for CLA #1.

@mateuszrzeszutek agree?

[mateuszrzeszutek]

Sounds good to me 👍