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.

Sign up for GitHub

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

Database semantic conventions: Add db.sql.table, allow SQL keyword as db.operation #1141

Merged
merged 7 commits into from
Nov 9, 2020
Merged

Database semantic conventions: Add db.sql.table, allow SQL keyword as db.operation #1141

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bcc9ca1
Add db.table to database semantic conventions
justinfoote Oct 23, 2020
5c05f92
Fix spacing in db semantic conventions
justinfoote Oct 27, 2020
657670c
Move db.table to SQL-specific call level attributes
justinfoote Oct 30, 2020
ff2e13b
Clarify instructions for setting db.operation to a SQL keyword
justinfoote Oct 30, 2020
eb4d3cd
Specify that db.sql.table MUST NOT be set when not applicable
justinfoote Nov 3, 2020
862361f
Add db.table and db.operation to CHANGELOG
justinfoote Nov 9, 2020
f504893
Merge branch 'master' into db_table_semantics
justinfoote Nov 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions semantic_conventions/trace/database.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,27 @@ groups:
conditional: Required, if `db.statement` is not applicable.
brief: >
The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations)
such as `findAndModify`.
such as `findAndModify`, or the SQL keyword.
note: >
While it would semantically make sense to set this, e.g., to a SQL keyword like `SELECT` or `INSERT`,
it is not recommended to attempt any client-side parsing of `db.statement` just to get this property
(the back end can do that if required).
examples: ['findAndModify', 'HMSET']
It is not recommended to attempt any client-side parsing of
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
`db.statement` just to get this property, but it should be set if
the operation name is provided by the library being instrumented.
If the statement has an ambiguous operation, or performs more than
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to imply that operation is always derived from statement, which is not true. In fact, the main purpose of this attribute is to support DBs that do not use a statement/query language as their API, such as CouchDB.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I see your point. This certainly was not my intention.

In the main "brief" description of this attribute, we still list the mongo example first. We could add a few more examples there.

Or I wonder if it would be better to include something in the footnote like this:

note: >
    In the case of SQL databases, the operation should be set to the SQL keyword
    (example: `SELECT` or `INSERT`). 
    It is not recommended to attempt any client-side parsing of
    `db.statement` just to get this property, but it should be set if 
    the operation name is provided by the library being instrumented.
    If the statement has an ambiguous operation, or performs more than
    one operation, this value may be omitted.

Is this redundant with the note in the 'required' column: "Required, if db.statement is not available."

What do you think?

Copy link
Member

@Oberon00 Oberon00 Oct 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested a simpler change by emphasizing that that part only applies to SQL in #1141 (review)

Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
one operation, this value may be omitted.
examples: ['findAndModify', 'HMSET', 'SELECT']
- id: table
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I just realized that the term table is probably specific to SQL. Thus I recommend adding this to a new semantic convention db.sql in this file that extends: db and has this table attribute, so that the full name is db.sql.table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative might be to have a more generic term, but since a table likely has no 1:1 mapping to other DB concepts (there might be several concepts that match a bit), I think having this in a SQL-specific convention would be easier.

Copy link
Member Author

@justinfoote justinfoote Oct 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea. I'll leave the operation where it is, but move the table to an SQL specific section in Call-level attributes for specific technologies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 657670c

tag: call-level
type: string
required:
conditional: Recommended if available.
brief: The name of the primary table, collection, segment, etc... that the operation is acting upon, including the schema name (if applicable).
note: >
It is not recommended to attempt any client-side parsing of
`db.statement` just to get this property, but it should be set if
it is provided by the library being instrumented.
If the operation is acting upon an anonymous table, or more than one
justinfoote marked this conversation as resolved.
Show resolved Hide resolved
table, this value may be omitted.
examples: ['public.users', 'customers']
- ref: net.peer.name
tag: connection-level
required:
Expand Down
7 changes: 5 additions & 2 deletions specification/trace/semantic_conventions/database.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,18 @@ Usually only one `db.name` will be used per connection though.
|---|---|---|---|---|
| `db.name` | string | If no [tech-specific attribute](#call-level-attributes-for-specific-technologies) is defined, 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). [1] | `customers`<br>`main` | Conditional [2] |
| `db.statement` | string | The database statement being executed. [3] | `SELECT * FROM wuser_table`<br>`SET mykey "WuValue"` | Conditional<br>Required if applicable. |
| `db.operation` | string | The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations) such as `findAndModify`. [4] | `findAndModify`<br>`HMSET` | Conditional<br>Required, if `db.statement` is not applicable. |
| `db.operation` | string | The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations) such as `findAndModify`, or the SQL keyword. [4] | `findAndModify`<br>`HMSET`<br>`SELECT` | Conditional<br>Required, if `db.statement` is not applicable. |
| `db.table` | string | The name of the primary table, collection, segment, etc... that the operation is acting upon, including the schema name (if applicable). [5] | `public.users`<br>`customers` | Conditional<br>Recommended if available. |

**[1]:** In some SQL databases, the database name to be used is called "schema name".

**[2]:** Required, if applicable and no more-specific attribute is defined.

**[3]:** The value may be sanitized to exclude sensitive information.

**[4]:** While it would semantically make sense to set this, e.g., to a SQL keyword like `SELECT` or `INSERT`, it is not recommended to attempt any client-side parsing of `db.statement` just to get this property (the back end can do that if required).
**[4]:** It is not recommended to attempt any client-side parsing of `db.statement` just to get this property, but it should be set if the operation name is provided by the library being instrumented. If the statement has an ambiguous operation, or performs more than one operation, this value may be omitted.
justinfoote marked this conversation as resolved.
Show resolved Hide resolved

**[5]:** It is not recommended to attempt any client-side parsing of `db.statement` just to get this property, but it should be set if it is provided by the library being instrumented. If the operation is acting upon an anonymous table, or more than one table, this value may be omitted.
<!-- endsemconv -->

For **Redis**, the value provided for `db.statement` SHOULD correspond to the syntax of the Redis CLI.
Expand Down