Add attribute db.values to database semantic conventions

[haddasbronfman]

What are you trying to achieve?
In some DBs, the query can contain a placeholder that should be filled with given values:
in MySQL: connection.query('SELECT * FROM 'books' WHERE 'author' = ?', ['David'])
in Postgres: client.query('SELECT $1::text as message', ['Hello world!']

These values are not part of db.statement attribute and are added to the span when the user gives the flag: enhancedDatabaseReporting:true in the instrumentation configuration. However, there is no suitable parameter for it in the semantic conventions.

What did you expect to see?
I expected that each relevant DB will have a suitable attribute in the semantic convention. something like: MYSQL_VALUES or PG_VALUES.

[haddasbronfman]

@tigrannajaryan Hi, what do you think about this suggestion? Can I add it to spec?

[tigrannajaryan]

The proposal sounds reasonable.

@open-telemetry/specs-approvers I will triage this as accepted but if you think otherwise please comment.

[trask]

@open-telemetry/technical-committee can you move this to https://github.com/open-telemetry/semantic-conventions?

[trask]

These values are not part of db.statement attribute and are added to the span when the user gives the flag: enhancedDatabaseReporting:true in the instrumentation configuration. However, there is no suitable parameter for it in the semantic conventions.

just a note for others, enhancedDatabaseReporting is an option provided in JS instrumentation

[trask]

there has been similar request to capture parameter values in Java instrumentation: open-telemetry/opentelemetry-java-instrumentation#7413

[trask]

I think this is a good idea. Only not sure about naming.

Gathering options:

• *.values
• *.parameter_values
• *.bind_values
• *.statement_values

I thought maybe namespace could be db.statement.*, but then we'd need to change db.statement since we have a restriction on attributes not also being a namespace.

[Flarna]

Is it just about the values or also about the keys? Some databases use named parameters others indexed.

So maybe a record named db.bind.parameters?
Or if records are not possible db.bind.parameter.<key>=<value> similar as for HTTP headers?

[jack-berg]

Or if records are not possible db.bind.parameter.= similar as for HTTP headers?

Anonymous parameters throw a bit of a wrench into this. Some systems use generic placeholders like ?, and rely on the values to be positional. Others use indexed placeholders like $1, $2. Consider the following variations of essentially the same statement:

• INSERT INTO user VALUES (?, ?, ?, ?)
• INSERT INTO user VALUES ($1, $2, $3, $4)
• INSERT INTO user VALUES (:id, :first, :last, :middle)

If we split up keys and values and ensure they use the same index, then we can accommodate all of these:

• INSERT INTO user VALUES (?, ?, ?, ?)
  • db.bind.parameter_keys: omitted because we don't have anything useful to include
  • db.bind.parameter_values: ["123", "John", "Doe", "Thomas"]
• INSERT INTO user VALUES ($1, $2, $3, $4)
  • db.bind.parameter_keys: ["$1", "$2", "$3", "$4], or omitted because it including doesn't provide much value
  • db.bind.parameter_values: ["123", "John", "Doe", "Thomas"]
• INSERT INTO user VALUES (:id, :first, :last, :middle)
  • db.bind.parameter_keys: [":id", ":first", ":last", ":middle"]
  • db.bind.parameter_values: ["123", "John", "Doe", "Thomas"]

[Flarna]

If two arrays are used I vote for omit in case of anonymous/indexed cases as there is no extra value.
In case of using a map/record I would use the index as key (but without the $, always start with 0) to keep an unique structure.

[trask]

We are thinking to rename db.statement to db.query.text. In which case:

Option A:

• db.query.text
• db.query.parameter_keys
• db.query.parameter_values

An open question is how this relates to batching, e.g. inserting multiple rows into a table via a batch query, in which case the parameter values is an array of arrays. But maybe that can be added later as something like

• db.query.batch.parameter_keys - not reported in the case of anonymous parameters
• db.query.batch.parameter_values - flattened array of all the values
• db.query.batch.count - required for understanding the flattened parameter values array

Option B:

• db.query.text
• db.query.parameter.<key>

where anonymous parameters could be reported as positional, e.g.

• db.query.parameter.0
• db.query.parameter.1
• db.query.parameter.2

Batching could maybe be added later as something like

• db.query.batch.parameter.<key> - array of values for individual parameter
• db.query.batch.count - no longer required with this modeling