open-telemetry / open-telemetry/opentelemetry-python-contrib

Sanitization of span attribute `db.statement`

Open
#1,376 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature-request
Dominant language
Python
Stars
1.1k
Forks
1.1k
Avg merge
4d 15h
Merged PRs (30d)
16

Description

Is your feature request related to a problem?
Yes, there is a security concern. Currently, if an instrumented service executes a database query that is not a prepared statement (i.e. hardcoded or simple string interpolation), then the corresponding span in the resulting trace will have attribute db.statement whose value contains the entire expression. For example, db.statement would be "SELECT * FROM city WHERE city.id = 1818" from this query done by a Django app:

  conn = psycopg2.connect(
      database="world-db",
      host="postgres-world-db",
      user="my-user",
      password="my-password",
      port=5432
  )
  cursor = conn.cursor()
  query = "SELECT * FROM city WHERE city.id = 1818"
  cursor.execute(query)

Describe the solution you'd like
It would be great if the database interface OTel instrumentation libraries (e.g. dbapi, sqlalchemy, asyncpg, pymongo) would sanitize db.statement so that query arguments are not in span attributes. Any numbers ([0..9]+) would be replaced with 0, or perhaps a configurable replacement character. Any 1+ characters would be replaced with ?. For the example above, db.statement could be sanitized to "SELECT * FROM city WHERE city.id = 0".

This feature would be enabled by default and configurable. It could be an optional param at instrumentation setup like enabled_sanitizer=True (default) or enabled_sanitizer=False. This would be like the enabled_commenter param for dbapi instrumentation.

Describe alternatives you've considered
This doesn't seem to happen when a service executes prepared query statements. While a best practice, it could still be that an instrumented service somewhere has some hardcoded query.

Additional context
One way could be a new, shared util that would be used by existing database interface OTel instrumentation libraries, similar to how opentelemetry-util-http is used by http instrumentation libraries. There exists a shared util for adding sql comments, but it does not edit the original statement.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading opentelemetry-instrumentation/sqlcommenter_utils.py and the dbapi instrumentation documentation, then compare how dbapi, sqlalchemy, asyncpg, and pymongo record db.statement. Define the shared sanitizer and its default/configuration behavior before applying it across the named database instrumentations; done means query arguments are removed by default without affecting prepared statements.

Written by the indexing model from the issue text.

Assessment

Tech stack
postgresql, python, sql, sqlalchemy
Domain
databases, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.