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

Project Request: Query Source #40

Closed
2 tasks done
gggritso opened this issue Nov 17, 2023 · 6 comments
Closed
2 tasks done

Project Request: Query Source #40

gggritso opened this issue Nov 17, 2023 · 6 comments

Comments

@gggritso
Copy link
Member

gggritso commented Nov 17, 2023

Description

People love the Starfish "Database" module, but everyone struggles to find the source of their SQL queries in their own codebase. We're hoping to attach the line of code that caused the query (AKA the "query source") to database spans. Laravel already does this, and it's invaluable! We'd love to see this in the Python and Ruby SDKs as well as a start.

The project document describes our idea in detail and addresses some likely objections. As a summary, here's what we're hoping for: For every query that takes longer than a threshold, attach the most recent in-app frame as a string property.

For example, with a threshold of 100ms, if a query takes 450ms, attach a property called code_source (name TBD) that shows where the query happened (e.g., /app/Http/Controllers/UserController.php:21)

We are open to suggestions and happy to contribute to the project in any way that is valuable!

Tasks

RFC

No response

Slack-Channel

#discuss-starfish

Notion Document(s)

https://www.notion.so/sentry/Query-Source-c771873b972f4aac9504950f2f77741a?pvs=4

Stakeholder(s)

https://github.com/orgs/getsentry/teams/team-starfish

Team(s)

Web Backend

Tasks

No tasks being tracked yet.
@AbhiPrasad
Copy link
Member

The attributes should be named after the otel semantic conventions for code: https://opentelemetry.io/docs/specs/semconv/attributes-registry/code/

@cleptric
Copy link
Member

@AbhiPrasad all of them? This would require some parsing logic, could we live with one combined attribute?

@stephanie-anderson
Copy link
Collaborator

@gggritso @sentrivana can have a look at this starting with next week - please reach out to her directly if you want to discuss anything sooner

@AbhiPrasad
Copy link
Member

@AbhiPrasad all of them? This would require some parsing logic, could we live with one combined attribute?

Yeah a combined attribute is fine if we can't do the OTEL way in certain sdks/frameworks, we can even propose this upstream long term.

@cleptric
Copy link
Member

We'll stick with Otel, WIP for Laravel at getsentry/sentry-laravel#803

@antonpirker antonpirker removed their assignment Nov 21, 2023
sthagen pushed a commit to sthagen/getsentry-sentry-python that referenced this issue Nov 24, 2023
Adding OTel compatible information to database spans that show the code location of the query.
Refs getsentry/team-sdks#40
---------

Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
@AbhiPrasad
Copy link
Member

It's shipped!

We'll open follow up issues for other SDKs in their respective repos - I'll also update develop to add query source as an expected feature of the sdks.

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

No branches or pull requests

6 participants