-
Notifications
You must be signed in to change notification settings - Fork 18
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
Trace Analytics: Add support for querying traces by lucene queries #129
Conversation
Levitate is-compatible report: 🔍 Resolving @grafana/data@latest... 🔬 Checking compatibility between ./src/module.ts and @grafana/data@9.4.7... 🔬 Checking compatibility between ./src/module.ts and @grafana/ui@9.4.7... 🔬 Checking compatibility between ./src/module.ts and @grafana/runtime@9.4.7... 🔬 Checking compatibility between ./src/module.ts and @grafana/e2e-selectors@9.4.7... ✔️ ./src/module.ts appears to be compatible with @grafana/data,@grafana/ui,@grafana/runtime,@grafana/e2e-selectors |
Backend code coverage report for PR #129 |
Frontend code coverage report for PR #129
|
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.
I really like the improvements, great job!
Based off of: #122
What this PR does / why we need it:
While working on #122 we did not hook up the lucene query string input field. This pr adds the ability for a user to continue to filter traces results with the lucene query field. While doing so, I realized it might make more sense to combine this field and the traceid field as I'm not sure there's a use case for users wanting to filter their spans with a lucene query, nor am I confident there's a usecase to show a single traceId in the trace list view.
While I was working on this, I kind of realized it might not make sense to store the entire lucene query object that we send to the backend within grafana. Instead we simply store that this is a traces query.
Then in datasource.ts's query function we check if the query is a traces query and if so return back the query formatted in DSL. Then when we map responses to dataframes, we do a similar check to see if it's a trace query and whether it is a single trace query and format the response.
In doing this I was able to delete the TracesQueryEditor and I ended up moving some of the traces files out of components and more top level as they are no longer involved in react. I apologies for the big git diff, but we could probably use another review anyway.
Which issue(s) this PR fixes:
Fixes #124
Fixes #123
Special notes for your reviewer:
Screen.Recording.2023-04-07.at.4.59.59.PM.mov
Also quickly added the ability to make clickable links!:
Screen.Recording.2023-04-10.at.8.42.06.AM.mov