Add more attributes for Cassandra to database semantic conventions #1217

FrankSpitulski · 2020-11-10T02:40:53Z

Changes

Adds additional optional cassandra tags.

Related issues #

arminru

Hi @FrankSpitulski!
While some SIGs use PR titles as the one here, this is not common in the spec repo. Apart from that, please use the correct term "attribute" or people might get confused. What about "Add more attributes for Cassandra to database semantic conventions"?
Also please add an entry in CHANGELOG.md.
Thanks!

semantic_conventions/trace/database.yaml

arminru · 2020-11-10T09:38:29Z

semantic_conventions/trace/database.yaml

+        - id: table
+          type: string
+          tag: call-level-tech-specific
+          brief: >
+            The table being queried.
+          examples: 'mytable'


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.

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.

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

db.sql.table is already mentioned in the rendered table. Having both is confusing:

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

Bumping, will get to this eventually

@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

Cool, then it should be possible to just edit the magic HTML comments in the markdown to generate that in a separate table.

Actually, I think we have to remove it from the db.tech group below, then we can put it in a separate table.

@Oberon00 it's been split

semantic_conventions/trace/database.yaml

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

arminru

LGTM!
It would be great if someone more familiar with Cassandra than me could take a look as well 🙂

semantic_conventions/trace/database.yaml

specification/trace/semantic_conventions/database.md

yurishkuro · 2020-11-11T16:00:18Z

specification/trace/semantic_conventions/database.md

+| `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 |


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

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

semantic_conventions/trace/database.yaml

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

specification/performance-benchmark.md

semantic_conventions/trace/database.yaml

specification/trace/semantic_conventions/database.md

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

arminru

LGTM!
It would be great if someone more familiar with Cassandra than me could take a look as well 🙂

semantic_conventions/trace/database.yaml

SergeyKanzhelev · 2020-11-17T17:22:22Z

I am still concerned with this: #1217 (comment) two attributes with the similar semantic in the same table:

Attribute	Type	Description	Example	Required
`db.cassandra.table`	string	The table being queried.	`mytable`	No
`db.sql.table`	string	The name of the primary table that the operation is acting upon, including the schema name (if applicable). [2]	`public.users` `customers`	Conditional Recommended if available.

github-actions · 2020-11-26T03:22:30Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

specification/trace/semantic_conventions/database.md

FrankSpitulski · 2020-12-03T02:19:34Z

@SergeyKanzhelev It's been moved to a new table

SergeyKanzhelev

LGTM, but if somebody with the experience of running Cassandra can comment - would be great.

iNikem · 2020-12-09T10:50:56Z

@open-telemetry/technical-committee does anything prevent this from merging? Either we don't have "somebody with the experience of running Cassandra" or they do not object. Either way there is no point in waiting forever, is it?

Oberon00 · 2020-12-01T15:15:43Z

semantic_conventions/trace/database.yaml

+        - id: table
+          type: string
+          tag: call-level-tech-specific
+          brief: >
+            The table being queried.
+          examples: 'mytable'


Actually, I think we have to remove it from the db.tech group below, then we can put it in a separate table.

Oberon00 · 2020-12-09T11:53:41Z

specification/trace/semantic_conventions/database.md

+
+Separated for clarity.
+
+<!-- semconv db.tech(tag=call-level-tech-specific-cassandra) -->


I think this would be cleaner if you used

Suggested change





here. Then you could remove the tag: call-level-tech-specific-cassandra line everywhere.

Also, you can remove the include: for cassandra from db.tech.

Oberon00 · 2020-12-09T11:54:50Z

specification/trace/semantic_conventions/database.md

+Separated for clarity.
+


Suggested change

Separated for clarity.

Oberon00 · 2020-12-09T11:55:55Z

IMHO: Let's merge it, and address my YAML nitpicks later. We can't make the review periods arbitrarily long and still expect contributors to respond.

feat(cassandra): more tags

0cf131a

FrankSpitulski requested review from a team as code owners November 10, 2020 02:40

github-actions bot assigned SergeyKanzhelev Nov 10, 2020

FrankSpitulski mentioned this pull request Nov 10, 2020

Additional tags for cassandra 4 open-telemetry/opentelemetry-java-instrumentation#1298

Closed

arminru reviewed Nov 10, 2020

View reviewed changes

FrankSpitulski and others added 2 commits November 10, 2020 10:49

Update semantic_conventions/trace/database.yaml

9edabb4

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

Update semantic_conventions/trace/database.yaml

a9842c1

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

FrankSpitulski changed the title ~~feat(cassandra): more tags~~ Add more attributes for Cassandra to database semantic conventions Nov 10, 2020

FrankSpitulski and others added 3 commits November 10, 2020 13:35

Update semantic_conventions/trace/database.yaml

3354514

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

incorporate changes

8e3ba0d

Merge branch 'master' into feat/cassandra/more-tags

2ec296f

arminru reviewed Nov 11, 2020

View reviewed changes

semantic_conventions/trace/database.yaml Outdated Show resolved Hide resolved

yurishkuro reviewed Nov 11, 2020

View reviewed changes

specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved

yurishkuro reviewed Nov 11, 2020

View reviewed changes

semantic_conventions/trace/database.yaml Show resolved Hide resolved

FrankSpitulski and others added 3 commits November 12, 2020 14:29

Update semantic_conventions/trace/database.yaml

608e40a

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

incorporate feedback

e8e7013

additional feedback

ee77c46

yurishkuro reviewed Nov 12, 2020

View reviewed changes

specification/performance-benchmark.md Outdated Show resolved Hide resolved

revert unchanged file

3eedef1

arminru reviewed Nov 13, 2020

View reviewed changes

semantic_conventions/trace/database.yaml Outdated Show resolved Hide resolved

specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved

FrankSpitulski and others added 2 commits November 13, 2020 12:09

Update semantic_conventions/trace/database.yaml

2d5db14

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

Update specification/trace/semantic_conventions/database.md

2fc1dfd

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

arminru approved these changes Nov 16, 2020

View reviewed changes

Oberon00 reviewed Nov 16, 2020

View reviewed changes

semantic_conventions/trace/database.yaml Show resolved Hide resolved

arminru added area:semantic-conventions Related to semantic conventions release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory labels Nov 17, 2020

github-actions bot added the Stale label Nov 26, 2020

FrankSpitulski and others added 2 commits December 1, 2020 16:16

split cassandra to separate table

71e84a9

Merge branch 'master' into feat/cassandra/more-tags

2bef33f

FrankSpitulski commented Dec 3, 2020

View reviewed changes

specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved

Update specification/trace/semantic_conventions/database.md

0dd6b01

github-actions bot removed the Stale label Dec 3, 2020

SergeyKanzhelev approved these changes Dec 4, 2020

View reviewed changes

Oberon00 approved these changes Dec 9, 2020

View reviewed changes

Merge branch 'master' into feat/cassandra/more-tags

a7d4db2

SergeyKanzhelev merged commit f48457c into open-telemetry:master Dec 9, 2020

FrankSpitulski deleted the feat/cassandra/more-tags branch December 10, 2020 03:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more attributes for Cassandra to database semantic conventions #1217

Add more attributes for Cassandra to database semantic conventions #1217

FrankSpitulski commented Nov 10, 2020

arminru left a comment

arminru Nov 10, 2020

Oberon00 Nov 10, 2020

FrankSpitulski Nov 10, 2020

SergeyKanzhelev Nov 16, 2020

FrankSpitulski Nov 17, 2020

FrankSpitulski Nov 26, 2020

FrankSpitulski Nov 30, 2020

Oberon00 Dec 1, 2020

Oberon00 Dec 1, 2020

FrankSpitulski Dec 2, 2020

arminru left a comment

yurishkuro Nov 11, 2020

FrankSpitulski Nov 12, 2020

arminru left a comment

SergeyKanzhelev commented Nov 17, 2020

github-actions bot commented Nov 26, 2020

FrankSpitulski commented Dec 3, 2020

SergeyKanzhelev left a comment

iNikem commented Dec 9, 2020

Oberon00 Dec 1, 2020

Oberon00 Dec 9, 2020

Oberon00 Dec 9, 2020

Oberon00 commented Dec 9, 2020


		Separated for clarity.

		<!-- semconv db.tech(tag=call-level-tech-specific-cassandra) -->

	<!-- semconv db.tech(tag=call-level-tech-specific-cassandra) -->
	<!-- semconv db.cassandra -->

Add more attributes for Cassandra to database semantic conventions #1217

Add more attributes for Cassandra to database semantic conventions #1217

Conversation

FrankSpitulski commented Nov 10, 2020

Changes

arminru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arminru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arminru left a comment

Choose a reason for hiding this comment

SergeyKanzhelev commented Nov 17, 2020

github-actions bot commented Nov 26, 2020

FrankSpitulski commented Dec 3, 2020

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

iNikem commented Dec 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oberon00 commented Dec 9, 2020