-
Notifications
You must be signed in to change notification settings - Fork 327
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
feat(persistence): span query DSL with SQL #2911
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:XXL
This PR changes 1000+ lines, ignoring generated files.
label
Apr 17, 2024
This was
linked to
issues
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
mikeldking
approved these changes
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 17, 2024
axiomofjoy
reviewed
Apr 18, 2024
axiomofjoy
reviewed
Apr 18, 2024
axiomofjoy
approved these changes
Apr 18, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice job, Roger. Stepping through the test cases helped me understand how the filters, query, and translator works.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #2081
resolves #2780
resolves #2782
resolves #2358
Notes
.
(dot) separator for span attributes #2081, giving us consistent JSON paths for attributes.latency_ms
is changed to a calculated field in DB. Unfortunately sqlite doesn’t give the same values as python or postgres (not sure why), So sqlite values are slightly off and filtered results can be slightly different.'service' in output.value
) in sqlite is using theLIKE
operator which is case-insensitive, which gives different filtered results than python or postgres. It’s not easy to fix this. Punting for now. (Also special characters such as % are not escaped. Also punting.)str()
,float()
,int()
to change the type. I have tried to infer as much as possible, but this is not always possible.use_active_session_if_available
option onpx.Client
is more difficult to now that the db is async. So i have nixed it. Now both thread and process sessions are making network calls using px.Client.parent_span_id
is renamed asparent_id
2.0.4
in order to useinplace.expression
forhybrid_property
.openinference.span.kind
is now included in the atributes. this shows the original string value in the event that it gets overwrittend toUNKNOWN
because it doesn't fit the enum.openinference.span.kind
is also outputted in the exported spans dataframe.