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.

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

Propose to remove db.cassandra.keyspace and use db.name instead #1760

Closed
trask opened this issue Jun 14, 2021 · 6 comments · Fixed by #1973
Closed

Propose to remove db.cassandra.keyspace and use db.name instead #1760

trask opened this issue Jun 14, 2021 · 6 comments · Fixed by #1973
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory

Comments

@trask
Copy link
Member

trask commented Jun 14, 2021

What are you trying to achieve?

Consistency across different database client spans.

What did you expect to see?

Cassandra keyspace captured as db.name, similar to other database client spans.

Additional context:

The specification currently says that db.cassandra.keyspace is

to be used instead of the generic db.name attribute.

But I'm not sure why there is this special case for Cassandra. It seems to make sense to use db.name instead.

This attribute appears to come from #575, but I couldn't find any discussion in that PR (or in the related issue) about why db.cassandra.keyspace should be used instead of db.name.

This issue stems from discussion in open-telemetry/opentelemetry-java-instrumentation#3199

@trask trask added the spec:trace Related to the specification/trace directory label Jun 14, 2021
@SergeyKanzhelev
Copy link
Member

I'm not quite familiar with Cassandra and whether using db.name will increase consistency. Maybe people mentioned below can comment. It is already being used: https://github.com/search?q=org%3Aopen-telemetry+db.cassandra.keyspace&type=code . It's OK to change since sem conv is not locked yet, but perhaps a transition from that attribute will be needed or we can prefer to keep it as is if consistency is not very straightforward.

CC @arminru who has originally proposed this name: #575

@Oberon00 @anuraaga @carlosalberto who approved that PR in case there are objections.

@SergeyKanzhelev SergeyKanzhelev added the area:semantic-conventions Related to semantic conventions label Jun 15, 2021
@anuraaga
Copy link
Contributor

I think this issue is essentially the DB / Cassandra version of #1748, they're the same concept of overriding a common name with a specific one, which is probably not a good pattern for conventions since instrumentation becomes more complicated, and backends become less generic.

@Oberon00
Copy link
Member

I think this issue is essentially the DB / Cassandra version of #1748, they're the same concept of overriding a common name with a specific one

I agree that they go into the same direction but there is one major difference: For jsonrpc vs generic rpc, the attribute was called simply "method" in both the generic and the specific convention. So IMHO there was a strong case for reusing the generic attribute. Here, the generic name is "name" and the specific name is "keyspace". It is not clear to me whether they are 100% equivalent in all cases.

I see db.name as a bit problematic, as it might be too generic. You have to know that db.name is the keyspace for cassandra or the schema name for some SQL databases (others might call it DB name already).

@SergeyKanzhelev
Copy link
Member

So far everybody agreed generalization is great. How do we resolve this:

I see db.name as a bit problematic, as it might be too generic. You have to know that db.name is the keyspace for cassandra or the schema name for some SQL databases (others might call it DB name already).

@Oberon00
Copy link
Member

I can see two general ways:

  • Remove db.name, in favor of more special names, e.g. db.sql.name or even db.oracle.schema_name (the latter is probably too extreme)
  • Find a generally applicable criteria for what constitutes a DB name (vs a table name, group of tables name, DBMS instance name, ...) and do as the issue proposes, removing db.cassandra.keyspace in favor of using db.name for it. Of course we can still mention which concept the db.name represents per DB technology, but we should have a clear criteria.

If we go the general route: db.name should probably be something "top-level". But here the problem already begins: In MSSQL, should we use the instance name as db.name, and have the database name as a db.mssql.db_name? Obviously not, but then we don't have the actual top-level as db.name. Probably the criteria should be that we want the logical top-level, disregarding physical deployment details. But that may be difficult for some databases.

Then, what happens if some database has a concept named "database name" but has something more top level? I think in Oracle, all schemas belong to a particular user, so the user is the top-level container. Bu we would have the schema name as db.name if we want to be consistent across SQL DBs, which we probably need to be from a practical perspective, otherwise you can't do e.g. a generic JDBC instrumentation.

@trask
Copy link
Member Author

trask commented Jul 15, 2021

@Oberon00 this is really helpful, thanks.

What do you think of defining db.name as the "lowest level" of those "top-level" things where there is ambiguity? And we can create individual semantic attributes for the higher levels where they exist, e.g. db.mssql.instance.

tigrannajaryan pushed a commit that referenced this issue Oct 8, 2021
…ame (#1973)

Fixes #1760

## Changes

Removes `db.cassandra.keyspace` and `db.hbase.namespace` (in preference for using `db.name`).

Clarifies `db.name` in the case where a database product provides multiple concepts that could be mapped to `db.name`.
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Nov 30, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 1, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 1, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 3, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 8, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 15, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Dec 16, 2021
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 6, 2022
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 6, 2022
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 6, 2022
The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760
ndimiduk added a commit to apache/hbase that referenced this issue Jan 11, 2022
)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 18, 2022
…ache#4015)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 18, 2022
…ache#4015)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
ndimiduk added a commit to apache/hbase that referenced this issue Jan 19, 2022
)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this issue Jan 19, 2022
…ache#4015)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
ndimiduk added a commit to apache/hbase that referenced this issue Jan 24, 2022
)

The HBase-specific attribute `db.hbase.namespace` has been deprecated in favor of the generic
`db.name`. See also open-telemetry/opentelemetry-specification#1760

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Projects
None yet
4 participants