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

Consistent naming: db.system to db.system.name, namespace constants, remove db from system-specific names #1734

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Jan 10, 2025

Fix #1581

Addresses DB-related portion of #608, replaces db-related things from #1711 and #1613.

  1. Rename db.system to db.system.name. It gives us a good opportunity to clean up constants, batching multiple breaking changes together (see below)
  2. Removes deprecated constants from new enum and updates existing constants to follow consistent naming
  3. Removes db prefix from system-specific attributes following Add system-specific naming guidance #1708:
    • some systems don't fit into one domain (e.g. redis is a cache and database and also a messaging system)
    • not all operations that are done on the database are related to DB (control plane, auth, scaling, node communication)
    • we're quite inconsistent in naming
  4. Adjusts _ -> . where it makes sense

Merge requirement checklist

@lmolkova lmolkova changed the title Align with naming guidance: db.system to db.system.name, namespace constants, remove db from system-specific names Consistent naming: db.system to db.system.name, namespace constants, remove db from system-specific names Jan 10, 2025
@lmolkova lmolkova marked this pull request as ready for review January 10, 2025 02:52
@lmolkova lmolkova requested review from a team as code owners January 10, 2025 02:52
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

I think I'm coming around to your proposal in #608 (comment)

model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Show resolved Hide resolved
@lmolkova lmolkova force-pushed the db-rename-system-and-attributes branch from bdcb0a2 to 6f9d66a Compare January 12, 2025 18:58
model/database/registry.yaml Show resolved Hide resolved
model/database/spans.yaml Show resolved Hide resolved
schema-next.yaml Outdated Show resolved Hide resolved
model/cassandra/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
model/database/registry.yaml Outdated Show resolved Hide resolved
…e constants, remove db prefix from system-specific attributes
@lmolkova lmolkova force-pushed the db-rename-system-and-attributes branch from 7574011 to 4509a9f Compare January 17, 2025 00:16
@lmolkova lmolkova requested a review from a team as a code owner January 17, 2025 02:45
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

👍

docs/attributes-registry/azure.md Outdated Show resolved Hide resolved
docs/attributes-registry/azure.md Outdated Show resolved Hide resolved
model/azure/registry.yaml Outdated Show resolved Hide resolved
model/cassandra/registry.yaml Outdated Show resolved Hide resolved
docs/database/cosmosdb.md Outdated Show resolved Hide resolved
docs/database/cosmosdb.md Outdated Show resolved Hide resolved
docs/database/elasticsearch.md Outdated Show resolved Hide resolved
Copy link
Member

@gregkalapos gregkalapos left a comment

Choose a reason for hiding this comment

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

Elasticsearch part look ok to me 👍

docs/database/mssql.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Status: Needs More Approval
Development

Successfully merging this pull request may close these issues.

Should db.system be db.system.name?
4 participants