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

Jump to bottom

Add more attributes for Cassandra to database semantic conventions #1217

Merged
merged 16 commits into from
Dec 9, 2020
Merged

Add more attributes for Cassandra to database semantic conventions #1217

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0cf131a
feat(cassandra): more tags
FrankSpitulski Nov 10, 2020
9edabb4
Update semantic_conventions/trace/database.yaml
FrankSpitulski Nov 10, 2020
a9842c1
Update semantic_conventions/trace/database.yaml
FrankSpitulski Nov 10, 2020
3354514
Update semantic_conventions/trace/database.yaml
FrankSpitulski Nov 10, 2020
8e3ba0d
incorporate changes
FrankSpitulski Nov 10, 2020
2ec296f
Merge branch 'master' into feat/cassandra/more-tags
FrankSpitulski Nov 10, 2020
608e40a
Update semantic_conventions/trace/database.yaml
FrankSpitulski Nov 12, 2020
e8e7013
incorporate feedback
FrankSpitulski Nov 12, 2020
ee77c46
additional feedback
FrankSpitulski Nov 12, 2020
3eedef1
revert unchanged file
FrankSpitulski Nov 12, 2020
2d5db14
Update semantic_conventions/trace/database.yaml
FrankSpitulski Nov 13, 2020
2fc1dfd
Update specification/trace/semantic_conventions/database.md
FrankSpitulski Nov 13, 2020
71e84a9
split cassandra to separate table
FrankSpitulski Dec 2, 2020
2bef33f
Merge branch 'master' into feat/cassandra/more-tags
FrankSpitulski Dec 3, 2020
0dd6b01
Update specification/trace/semantic_conventions/database.md
FrankSpitulski Dec 3, 2020
a7d4db2
Merge branch 'master' into feat/cassandra/more-tags
SergeyKanzhelev Dec 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ release.

New:

- Additional Cassandra semantic attributes
([#1217](https://github.com/open-telemetry/opentelemetry-specification/pull/1217))
- Add performance benchmark specification
([#748](https://github.com/open-telemetry/opentelemetry-specification/pull/748))
- Enforce that the Baggage API must be fully functional, even without an installed SDK.
Expand Down
64 changes: 63 additions & 1 deletion semantic_conventions/trace/database.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,69 @@ groups:
brief: >
The name of the keyspace being accessed. To be used instead of the generic `db.name` attribute.
examples: 'mykeyspace'
- id: page_size
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
type: number
tag: call-level-tech-specific
brief: >
The page size of the query.
examples: [5000]
- id: consistency_level
tag: call-level-tech-specific
brief: >
The consistency level of the query. Based on consistency values from [cql](https://docs.datastax.com/en/cassandra-oss/3.0/cassandra/dml/dmlConfigConsistency.html).
FrankSpitulski marked this conversation as resolved.
Show resolved Hide resolved
type:
members:
- id: ALL
value: 'ALL'
FrankSpitulski marked this conversation as resolved.
Show resolved Hide resolved
- id: EACH_QUORUM
value: 'EACH_QUORUM'
- id: QUORUM
value: 'QUORUM'
- id: LOCAL_QUORUM
value: 'LOCAL_QUORUM'
- id: ONE
value: 'ONE'
- id: TWO
value: 'TWO'
- id: THREE
value: 'THREE'
- id: LOCAL_ONE
value: 'LOCAL_ONE'
- id: ANY
value: 'ANY'
- id: SERIAL
value: 'SERIAL'
- id: LOCAL_SERIAL
value: 'LOCAL_SERIAL'
- id: table
type: string
tag: call-level-tech-specific
brief: >
The table being queried.
examples: 'mytable'
Copy link
Member

Choose a reason for hiding this comment

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

We could consider using db.sql.table instead which has the same meaning and a more elaborate definition. CQL is close to SQL after all.

Copy link
Member

Choose a reason for hiding this comment

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

I think having a separate attribute here for cassandra is fine. Otherwise we might want to rename db.sql.table to db.table after all and describe to which technologies it applies. But I think I prefer the separate attribute a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 I agree that should either be separate or use a top level db attribute

Copy link
Member

Choose a reason for hiding this comment

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

db.sql.table is already mentioned in the rendered table. Having both is confusing:
Uploading image.png…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't much more to say @SergeyKanzhelev unless you have a new suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumping, will get to this eventually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Oberon00, I just had a closer look. It seems it already is a different convention group that extends the generic db.

groups:
    - id: db ...

    - id: db.mssql ...

    - id: db.cassandra
      prefix: db.cassandra
      extends: db

Copy link
Member

Choose a reason for hiding this comment

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

Cool, then it should be possible to just edit the magic HTML comments in the markdown to generate that in a separate 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 think we have to remove it from the db.tech group below, then we can put it in a separate table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Oberon00 it's been split

- id: idempotence
type: boolean
tag: call-level-tech-specific
brief: >
Whether or not the query is idempotent.
- id: speculative_execution_count
type: number
tag: call-level-tech-specific
brief: >
The number of times a query was speculatively executed. Not set or `0` if the query was not executed speculatively.
examples: [0, 2]
- id: host.id
type: string
tag: call-level-tech-specific
brief: >
The host id reported by the coordinating node for a query.
examples: 'be13faa2-8574-4d71-926d-27f16cf8a7af'
- id: host.dc
type: string
tag: call-level-tech-specific
brief: >
The host data center reported by the coordinating node for a query.
examples: 'us-west-2'

- id: db.hbase
prefix: db.hbase
Expand Down Expand Up @@ -317,7 +380,6 @@ groups:
value MUST NOT be set.
examples: ['public.users', 'customers']


- id: db.tech
brief: "Semantic convention group for specific technologies"
constraints:
Expand Down
7 changes: 7 additions & 0 deletions specification/trace/semantic_conventions/database.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ For example, when retrieving a document, `db.operation` would be set to (literal
| Attribute | Type | Description | Example | Required |
|---|---|---|---|---|
| `db.cassandra.keyspace` | string | The name of the keyspace being accessed. To be used instead of the generic `db.name` attribute. | `mykeyspace` | Yes |
| `db.cassandra.page_size` | number | The page size of the query. | `5000` | No |
| `db.cassandra.consistency_level` | string enum | The consistency level of the query. Based on consistency values from [cql](https://docs.datastax.com/en/cassandra-oss/3.0/cassandra/dml/dmlConfigConsistency.html). | `ALL` | No |
| `db.cassandra.table` | string | The table being queried. | `mytable` | No |
| `db.cassandra.idempotence` | boolean | Whether or not the query is idempotent. | | No |
Copy link
Member

Choose a reason for hiding this comment

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

Who can determine whether a query is idempotent? This does not sound like Cassandra specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every query is marked as idempotent or not. https://docs.datastax.com/en/devapp/doc/devapp/driversQueryIdempotence.html

| `db.cassandra.speculative_execution_count` | number | The number of times a query was speculatively executed. Not set or `0` if the query was not executed speculatively. | `0`<br>`2` | No |
| `db.cassandra.host.id` | string | The host id reported by the coordinating node for a query. | `be13faa2-8574-4d71-926d-27f16cf8a7af` | No |
FrankSpitulski marked this conversation as resolved.
Show resolved Hide resolved
| `db.cassandra.host.dc` | string | The host data center reported by the coordinating node for a query. | `us-west-2` | No |
| `db.hbase.namespace` | string | The [HBase namespace](https://hbase.apache.org/book.html#_namespace) being accessed. To be used instead of the generic `db.name` attribute. | `default` | Yes |
| `db.redis.database_index` | number | The index of the database being accessed as used in the [`SELECT` command](https://redis.io/commands/select), provided as an integer. To be used instead of the generic `db.name` attribute. | `0`<br>`1`<br>`15` | Conditional [1] |
| `db.mongodb.collection` | string | The collection being accessed within the database stated in `db.name`. | `customers`<br>`products` | Yes |
Expand Down