Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Emit query log in quaint:query span, rather than in its parent #431

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

miguelff
Copy link
Contributor

@miguelff miguelff commented Jan 27, 2023

Tracked in https://github.com/prisma/client-planning/issues/158

Addressed the problem expressed in https://github.com/prisma/client-planning/issues/158#issuecomment-1405530608

Before this change query log events where not attached to the quaint:query span but its parent. This had implications on log capturing. mainly, that BEGIN iTX queries where not logged until its owner span (itx_runner) was not closed at the end of the transaction.

@miguelff miguelff self-assigned this Jan 27, 2023
@miguelff miguelff marked this pull request as ready for review January 27, 2023 14:54
miguelff added a commit to prisma/prisma-engines that referenced this pull request Jan 27, 2023
@miguelff miguelff added this to the 4.10.0 milestone Jan 27, 2023
@@ -14,8 +14,16 @@ where
U: Future<Output = crate::Result<T>>,
{
let span = info_span!("quaint:query", "db.statement" = %query);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the query engine binary and node API, This span is filtered-in by target , because it is user facing but doesn't have a user_facing attribute. Should we add the attribute to prevent this additional filtering based on the target?

I know, it's coupling quiant to engine needs. But isn't that already based on all the tracing features added to it?

miguelff added a commit to prisma/prisma-engines that referenced this pull request Jan 27, 2023
@miguelff
Copy link
Contributor Author

miguelff commented Jan 27, 2023

Given the correct behavior of this patch, the simplicity of the change, and the lack of required reviewers for quaint, I will merge this on Monday, unless there's strong opposition --in a bias to action.

miguelff added a commit to prisma/prisma-engines that referenced this pull request Jan 27, 2023
miguelff added a commit to prisma/prisma-engines that referenced this pull request Jan 30, 2023
Copy link
Contributor

@Weakky Weakky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@miguelff miguelff merged commit 1550f6c into main Jan 30, 2023
@miguelff miguelff deleted the fix-traces branch January 30, 2023 14:10
miguelff added a commit to prisma/prisma-engines that referenced this pull request Jan 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants