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

Include attributes when starting sqlalchemy spans for samplers

Open
#3,563 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

In this code:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/59cc34e9f37decd0e9f00ccb279c3bb8e0345d6e/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py#L270-L321

There's no good reason to set the attributes only after checking is_recording, except:

  1. when enable_commenter and enable_attribute_commenter are both true
  2. specifically for the statement attribute
  3. assuming that it's important that the initial and final values of the statement attribute are the same.

Otherwise, it's computationally trivial to include those attributes when starting the span. This would allow samplers to exclude spans with particular attributes (especially certain trivial queries which aren't worth tracing), and SpanProcessor.on_start to make use of the attributes.

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 in instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py at the span-starting code around lines 270-321. Trace how attributes are built with enable_commenter and enable_attribute_commenter, then verify that samplers and SpanProcessor.on_start can see the intended attributes while the statement-attribute exception remains correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, sqlalchemy
Domain
observability
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.