sqlalchemy integration does not track all databases

[sk-]

How do you use Sentry?

Sentry Saas (sentry.io)

Version

2.61.1

Steps to Reproduce

When using snowflake-sqlalchemy or clickhouse-sqlalchemy the recorded spans do not include the attribute span.system, which make it difficult to filter the queries.

The only span.system we get is postgresql, which in our case is also used through sqlalchemey (+asyncpg).

Is this a problem on the Sentry side, or is this something that should be fixed by the sqlalchemy extensions (snowflake and clickhouse)?

Expected Result

No span.system is recorded for snowflake or clickhouse.

Actual Result

All queries should include the right system.

[linear-code]

PY-2531

[ericapisani]

Hi @sk- ,

Thanks for opening an issue.

This looks to be a bug on the SDK side rather than the SQLAlchemy extensions. The SDK determines the value to set for the db.system attribute using this method, which is invoked a little further down in that file.

Since snowflake and clickhouse aren't checked or returned in _get_db_system, no value gets set for db.system, which explains the behaviour that you've reported.

If you're feeling up for it, we'd welcome a pull request updating the _get_db_system method to support snowflake and clickhouse - you'd need to follow the pattern already there checking for snowflake or clickhouse in the engine name and returning snowflake or clickhouse respectively.

If you're tight on time/bandwidth, no worries, we've got this issue in our backlog and will pick it up when we have a moment.

[tbigelow-nomi]

I am interested in picking up this work and factoring in #6575, as I have encountered the same problem with Snowflake. It would be fairly straight-forward to get the current list of accepted values expanded.

But, there are also the OTel DB well-known values here: https://opentelemetry.io/docs/specs/semconv/registry/attributes/db/#db-system-name

This list has some slight differences in naming, such as for Oracle; Sentry has that as oracle right now but OTel says it should be oracle.db.

Should I try to align Sentry to the OTel values in those scenarios? This would definitely change the data in new spans generated from that point.

[ericapisani]

That's a great call out regarding the OTEL values @tbigelow-nomi. I would avoid aligning existing Sentry values (such as oracle) to the OTEL ones for now as that will be a breaking change for users who depend on the db.system attribute in dashboards or alerts.

However, for new values that are being introduced - use the OTEL ones where possible. For a database like Snowflake which is not in the list - feel free to use a sensible value here.

I'll assign this issue to you, don't hesitate to give a shout if you have any follow up questions or need anything from the team.