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

Rename db.operation to db.operation.name #875

Merged
merged 10 commits into from
Apr 5, 2024

Conversation

trask
Copy link
Member

@trask trask commented Apr 3, 2024

Changes

Renames db.operation to db.operation.name, to be more future proof and allow other attributes under the db.operation.* namespace in the future.

Merge requirement checklist

Fixes #884

model/registry/db.yaml Outdated Show resolved Hide resolved
@pyohannes
Copy link
Contributor

In messaging, we have messaging.operation, and I wonder if consistency is important here.

Do you have an example of what additional attributes might be expected under db.operation?

@lmolkova
Copy link
Contributor

lmolkova commented Apr 4, 2024

In messaging, we have messaging.operation, and I wonder if consistency is important here.

Do you have an example of what additional attributes might be expected under db.operation?

I think we might entertain the idea of db.operation.duration metric (and it might still be an open question for messaging metrics since operation list is an open set).

In case of databases, we have a precedent when CosmosDB records operation name (e.g. ReadItems) along with db.cosmosdb.operation_type (e.g. Read). We might want to generalize at some point.
If we translated it to messaging, we'd do something like messaging.operation.type = settle, messaging.operation.name = ack.

@lmolkova
Copy link
Contributor

lmolkova commented Apr 4, 2024

Also, graphql has operation.name and operation.type - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/graphql/graphql-spans.md

I'm starting to understand why graphql used to be in the DB semconv - both try to describe QL

@trask trask marked this pull request as ready for review April 5, 2024 01:10
@trask trask requested review from a team as code owners April 5, 2024 01:10
model/registry/db.yaml Outdated Show resolved Hide resolved
@pyohannes
Copy link
Contributor

Discussed in the messaging workgroup: for messaging we'll take over what's adopted for databases, as we don't see any strong reasons to stick with messaging.operation as an attribute (and be inconsistent with database semantic conventions).

See the related issue #890.

@trask trask requested a review from gregkalapos April 5, 2024 15:37
@lmolkova lmolkova merged commit f812621 into open-telemetry:main Apr 5, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rename db.operation to db.operation.name for future proofing
7 participants