-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add raw_data query support to backend #203
Conversation
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.
A couple questions/nits, but nice work!
We should probably add this in the description of either the epic ticket or the tasks in it |
dc6bbbc
to
f3ac9ea
Compare
Thanks @iwysiu and @idastambuk for having a look! I think I've addressed the feedback so far. Let me know if you have any other suggestions. |
7636439
to
c4f765c
Compare
f8353aa
to
739f780
Compare
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.
Nice work! One last little comment nit that was just me being unclear.
Was there a reason you needed to force push? If you can, once people have started reviewing a pr you should avoid force pushing, it messes up the links for comments. (If you need to fix merge conflicts you can do a rebase force push after its been approved)
Thanks for having a look @iwysiu and good point, sorry about that! I will avoid that in the future. |
What this PR does / why we need it:
This recreates the raw_data query processing that exists in the frontend in the backend. It redirects any queries with only raw_data to the backend.
This is based on equivalent work in Elasticsearch (grafana/grafana#63208 and grafana/grafana#59741) and on the existing query logic in OpenSearch's frontend.
hits
response, we create an object:_source
-- these are flattened to a maximum depth of 10_id
,_type
,_index
fields
if possible and falling back to the timestamp_source
if possibleFor testing, one way is to use the sample data and create a query > Metric > Raw Data.
Compare a version without these changes, e.g. v2.6.2.
In our squad, check the data source set up here https://clouddatasources.grafana.net/goto/BTjFTmuVR?orgId=1 under the Lucene Raw Data section.
This should be running the frontend logic.
Build locally this PR and make the same query.
The same data results should be observed. There is a difference because the data frame types are now interpreted (previously values were all strings), so some default format options round the numbers.
Here are some examples of a visual comparison:
Black is the frontend flow
White is the locally-built backend flow
Which issue(s) this PR fixes:
Fixes #196
Special notes for your reviewer:
When possible, tests were pulled from the Elasticsearch PR and adapted for our code.
We also included this change grafana/grafana#67767 as we ran into the issue. We'll likely include it for other types of queries as well.
If there are too many changes, let me know if I should try to break it up. I'm also happy to go over it together.