-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[new feature] introduce loki Coprocessor querier pre query ,And provider a golang demo XRayCoprocessor. #8568
base: main
Are you sure you want to change the base?
Conversation
…der a go demo (traceID Coprocessor 1 simple text analysis)
pkg/coprocessor/observer.go
Outdated
return false, err | ||
} | ||
|
||
if res.Pass { |
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.
nit: Lines 89 to 93 could be expressed as:
return res.Pass, nil
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.
ok,done
pkg/coprocessor/observer.go
Outdated
"github.com/grafana/loki/pkg/util/build" | ||
) | ||
|
||
var timeout = 2 * time.Minute |
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 think this should be passed as a config along the URL.
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.
thanks.done
docs/sources/configuration/_index.md
Outdated
@@ -505,6 +505,9 @@ engine: | |||
# When true, allow queries to span multiple tenants. | |||
# CLI flag: -querier.multi-tenant-queries-enabled | |||
[multi_tenant_queries_enabled: <boolean> | default = false] | |||
|
|||
pre_query_url: |
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.
If we decide to be able to configure the timeout for the preQuery call, I'd rather format this config as:
coprocessor:
pre_query:
[url: <string> | default = ""]
[timeout: <duration> | default = 2m]
Furthermore, this allows us to add new functions (e.g. post_query) easier.
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.
docs/sources/configuration/_index.md
Outdated
@@ -505,6 +505,9 @@ engine: | |||
# When true, allow queries to span multiple tenants. | |||
# CLI flag: -querier.multi-tenant-queries-enabled | |||
[multi_tenant_queries_enabled: <boolean> | default = false] | |||
|
|||
pre_query_url: |
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.
We should doccument this config.
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.
ok
@salvacorts thanks .☑️ |
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.
[Docs squad] Documentation LGTM
|
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.
Has the LID been approved? Could you reference it here?
Hi, there is no LID yet. I will try to submit a LID as soon as possible, I haven't done this kind of thing before. Need to learn how to write LIDs submitted by others. This PR is only the code that our offline loki runs, which has been written before. So this PR is done faster than LID. |
LID: #8616 |
What this PR does / why we need it:
ref issue : High cardinality labels
{log_type="service_metrics"} |= "ee74f4ee-3059-4473-8ba6-94d8bfe03272"
We have counted the source distribution of our logql, and 85% of the grafana log explore queries are traceID queries.
Generally, the log time range of traceID is about 10 minutes(trace time= start~end), but because users do not know traceID start time and end time , they usually search for 7 day log. In fact having a time range of "7d-10m" is an invalid search.
So we hope to introduce some auxiliary abilities to solve this "7d-10m" invalid search.
We have checked that in the database field, such feature have been implemented very maturely.
And our team tried to implement the
preQuery Coprocessor
, and achieved great success. Through this feature, we solved the problem of "loki + traceID search is very slow".Thanks Google’s BigTable coprocessor and HBase coprocessor.
HBase coprocessor_introduction link: https://blogs.apache.org/hbase/entry/coprocessor_introduction
The idea of HBase Coprocessors was inspired by Google’s BigTable coprocessors. Jeff Dean gave a talk at LADIS ’09 (http://www.scribd.com/doc/21631448/Dean-Keynote-Ladis2009, page 66-67)
Describe the solution you'd like
Which issue(s) this PR fixes:
Fixes ##8559
Special notes for your reviewer:
pre_query_url loki.yaml
debug success in IDEA Golang .
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updateddocs/sources/upgrading/_index.md