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

Sanitization of span attribute db.statement #1376

Open
tammy-baylis-swi opened this issue Oct 5, 2022 · 2 comments
Open

Sanitization of span attribute db.statement #1376

tammy-baylis-swi opened this issue Oct 5, 2022 · 2 comments

Comments

@tammy-baylis-swi
Copy link
Contributor

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.

@ranrib
Copy link

ranrib commented Nov 2, 2022

There's a bit similar discussion here - open-telemetry/opentelemetry-js-contrib#1237

@tammy-baylis-swi
Copy link
Contributor Author

Referencing #1543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants