[Starfish] Add db attributes to database span's span data

[AbhiPrasad]

To get an overview of this work across all SDKs, please visit:
https://github.com/orgs/getsentry/projects/135

Parent issue: getsentry/team-sdks#19

For the starfish db module it was determined that we need extra database attributes.

db.name should be set if possible, but is not required. db.system should match the list in OpenTelemetry's well known conventions.

Attribute	Type	Description	Examples	Requirement Level
db.system	string	An identifier for the database management system (DBMS) product being used.	sqlite	Required
db.name	string	This attribute is used to report the name of the database being accessed. For commands that switch the database, this should be set to the target database (even if the command fails).	customers; main	Conditionally Required: If applicable.

These values should be set on span.data.

Motivation

This is being done for the Sentry's new performance views for databases. See a WIP example of the view below:

[armcknight]

Hey @AbhiPrasad thanks for opening the issue to track this here. My main question is if we already have a comprehensive list of expected db.system values? We could report this generally as core-data, but there are several options for backing stores for a core data deployment: sqlite, file and in-memory.

For a file-backed database, depending on how this span data will be used, there may be some overlap with the current File I/O on Main Thread performance issue type. We may also want to think about how we might want to get the file path into the span.

[AbhiPrasad]

For db.system OpenTelemetry has a list of well known items here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md#notes-and-well-known-identifiers-for-dbsystem. If those aren't enough, we can add more options and and attempt to contribute to the upstream spec!

For a file-backed database, depending on how this span data will be used, there may be some overlap with the current File I/O on Main Thread performance issue type. We may also want to think about how we might want to get the file path into the span.

This is a great point! I think we need should invest some time into thinking about this considering we're looking at the mobile views right now.

I wrote this up as a task on the starfish board https://www.notion.so/sentry/Mobile-database-view-and-file-i-o-on-main-thread-performance-issue-596f63f552c140f2af91251c238f90fa cc @shruthilayaj who's doing it all mobile starfish wise.

[armcknight]

Ah, I didn't see that the list was above that link anchor 😅 Should've read the note more closely.

It does say a custom value may be used, and that that list is non-exhaustive. I still think it'd be worthwhile having our own list of known identifiers that may be sent in this field.

I think for our purposes in cocoa, we should send something like coredata:sqlite, coredata:file, coredata:in-memory. We don't support automatically instrumenting these today, but there are also raw sqlite (and many wrappers around it), realm, and probably more still although this is the extent of my knowledge.

[AbhiPrasad]

It does say a custom value may be used, and that that list is non-exhaustive. I still think it'd be worthwhile having our own list of known identifiers that may be sent in this field.

We have our own independent list at https://develop.sentry.dev/sdk/performance/span-data-conventions/ we can update - so feel free to name these however you think makes sense, we'll add it to our internal spec first.

OTEL also doesn't pay much attention to mobile things, so it is an opportunity for us to be opinionated and give some recommendations for conventions that makes sense for us and our users.

[armcknight]

Reading more there though, it seems like coredata is more of the db connection, and sqlite is the db system, based on their JDBC example.

[kahest]

For db.name we should think of reasonable fallbacks, e.g. app name. NSManagedObjectContext.name could be empty in many cases

[armcknight]

NSManagedObjectContext.name could be empty in many cases

This property is not actually the name of the underlying sqlite database name (if that's what is being used as the backing store), but rather a custom name a developer can set on the context (there can be many contexts per database; it's akin to a JDBC Connection).

What we can do for db.name is look at the list of persistent stores that will be involved (there can be multiple) and grab their URLs (for a sqlite backing store, it'd be the path/filename of the sqlite database; for an in memory store, it's a string like memory://600000015420), maybe concatenating with ;. (There may not be a URL available, I will add (null) in this case so we maintain a 1-1 relationship with multiple entires in db.system described below.)

As for db.system, I will set this as the backing store type, which can be one of SQLite, XML (not available on iOS), Binary or InMemory (these are constants defined by Cocoa, not us; we can lowercase them if desired). Again, there may be multiples, I will concatenate with ;.

[kahest]

@armcknight Good point about the context, PSC name would probably be closer to the DB name, but as you mention, there can be multiple PSs. I'm not sure how actionable for the user it is to get a list all the URLs for the stores, but it's probably closest to the truth.