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

Database: review sampling_relevant attributes #1019

Closed
lmolkova opened this issue May 9, 2024 · 3 comments · Fixed by #1166
Closed

Database: review sampling_relevant attributes #1019

lmolkova opened this issue May 9, 2024 · 3 comments · Fixed by #1166
Assignees
Labels
area:db enhancement New feature or request

Comments

@lmolkova
Copy link
Contributor

lmolkova commented May 9, 2024

Area(s)

area:db

What happened?

We should identify and document attributes that are relevant for sampling

Semantic convention version

1.25.0

Additional context

No response

@lmolkova lmolkova added bug Something isn't working triage:needs-triage labels May 9, 2024
@lmolkova lmolkova added enhancement New feature or request and removed bug Something isn't working labels May 9, 2024
@trask trask assigned trask and unassigned AlexanderWert May 29, 2024
@trask
Copy link
Member

trask commented May 31, 2024

proposed list of sampling relevant attributes:

  • db.system
  • db.namespace
  • server.address
  • server.port
  • db.query.text - (e.g. I want to filter out SELECT 1 health checks)
    • parsing cost?
      • often parsing already to capture db.collection.name and db.collection.operation
      • if you don't want to incur parsing cost on unsampled spans, then can always disable parsing
  • db.collection.name - yes
  • db.operation.name - yes
  • db.query.parameter.<key> - not sure about strong use case, but should be available at span start

@trask
Copy link
Member

trask commented Jun 7, 2024

Note: db.namespace may not be available for sampling in Elasticsearch: https://github.com/open-telemetry/semantic-conventions/pull/1002/files#r1631347169

@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 7, 2024

db.query.text - (e.g. I want to filter out SELECT 1 health checks)

sampling-relevant query would mean sanitization for every query and the perf hit. I'd prefer to avoid it.

In the future, if we introduce some form of db.query.alias that one can be sampling-relevant.

It also feels like making attribute sampling-relevant post-stability should be ok - it adds a new feature without breaking anything existing. The stability doc can be interpreted in different ways though:

Whether these attributes must be provided at span start time, due to sampling concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:db enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

4 participants