loki icon indicating copy to clipboard operation
loki copied to clipboard

[new feature] add Remote read API

Open liguozhong opened this issue 3 years ago • 25 comments

What this PR does / why we need it:

image

[new feature] add Remote read API.

remote read use /Users/fuling/go/src/github.com/grafana/loki/pkg/logcli/client/client.go, loki server do not add new http handler.

type Client interface {
	Query(queryStr string, limit int, time time.Time, direction logproto.Direction, quiet bool) (*loghttp.QueryResponse, error)
	QueryRange(queryStr string, limit int, start, end time.Time, direction logproto.Direction, step, interval time.Duration, quiet bool) (*loghttp.QueryResponse, error)
	ListLabelNames(quiet bool, start, end time.Time) (*loghttp.LabelResponse, error)
	ListLabelValues(name string, quiet bool, start, end time.Time) (*loghttp.LabelResponse, error)
	Series(matchers []string, start, end time.Time, quiet bool) (*loghttp.SeriesResponse, error)
	LiveTailQueryConn(queryStr string, delayFor time.Duration, limit int, start time.Time, quiet bool) (*websocket.Conn, error)
	GetOrgID() string
}

loki.yaml config demo

remote_read:
  - url: http://loki_us.svc:3100
    name: loki_us
    orgID: buy
  - url: http://loki_eu.svc:3100
    name: loki_eu
    orgID: carts

prometheus remote read doc:https://prometheus.io/docs/prometheus/latest/configuration/configuration/#remote_read Which issue(s) this PR fixes: Fixes #7306 And #https://github.com/grafana/loki/issues/1866

Special notes for your reviewer: This PR is already quite large, this PR is only implementation. The basic framework of remote read and the configuration of remote read and an interface read.go SelectLogs test.

Checklist

  • [x] Reviewed the CONTRIBUTING.md guide
  • [x] Documentation added
  • [x] Tests updated
  • [x] CHANGELOG.md updated
  • [ ] Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

liguozhong avatar Oct 03 '22 17:10 liguozhong

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.4%

grafanabot avatar Oct 04 '22 08:10 grafanabot

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
- querier/queryrange	-0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.4%

grafanabot avatar Oct 04 '22 09:10 grafanabot

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.4%

grafanabot avatar Oct 08 '22 06:10 grafanabot

Is there an issue or something similar where this is discussed in more detail?

jeschkies avatar Oct 10 '22 11:10 jeschkies

Is there an issue or something similar where this is discussed in more detail?

hi @jeschkies ,As far as I know, there is no detailed discussion about remote read, but I have some experience with the remote read module in the prometheus project before.

This PR is only to provide a template for the community to discuss, we can discuss in this PR.

I'm very sorry to submit this PR directly without discussing it in detail, trust me, I have no ill intentions. I just want to make loki a more mature and complete logging solution faster.

liguozhong avatar Oct 11 '22 05:10 liguozhong

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.4%

grafanabot avatar Oct 11 '22 07:10 grafanabot

I'm very sorry to submit this PR directly without discussing it in detail, trust me, I have no ill intentions. I just want to make loki a more mature and complete logging solution faster.

@liguozhong no worries. I didn't mean to criticize. I was out for some time and thought I missed some info.

I'm super happy you took this initiative. I have a few ideas that I'll add later here.

jeschkies avatar Oct 11 '22 07:10 jeschkies

When I was operating prometheus before, I had some experience with remote read.

case 1: 
promql -> prometheus federation -> remote read adapter -> prometheus node

case 2: 
promql -> prometheus federation -> nginx(SLB or any load balance)-> remote read adapter -> prometheus node

The remote read of prometheus is implemented through the http protocol, so it is easy to use nginx to form a more complex global network topology.

But. Previously, we tried to achieve a global read capability similar to globalview through the grpc protocol, but load balancing such as nginx that could not work for the grpc protocol.

so, we should pay attention to a question, whether we are http or grpc, if we use stream api, we should be careful about grpc.

liguozhong avatar Oct 11 '22 07:10 liguozhong

@liguozhong @jeschkies whats the current status here I only see last progress in october? We're also interested in getting this feature.

We're bascially are willing to use loki similar to thanos.

Moep90 avatar Dec 21 '22 09:12 Moep90

@liguozhong @jeschkies whats the current status here I only see last progress in october? We're also interested in getting this feature.

We're bascially are willing to use loki similar to thanos.

I am waiting for review

liguozhong avatar Dec 21 '22 10:12 liguozhong

ready for reveiw .

liguozhong avatar Feb 17 '23 02:02 liguozhong

I tried to implement the code for batch pulling remote storage. If batchSize==0, it will still pull all at once 2 mode provide options for loki's operator to choose.

But this batch pulling code makes the code look more difficult to read, please help review

liguozhong avatar Feb 27 '23 09:02 liguozhong

I'm worried about performance of this at scale, which often coincides with users that want this feature. Right now, no aggregation is done remotely, meaning we're going to ship potentially a lot of data back to the host cluster on each query. This can quickly become prohibitively expensive (data egress costs).

Some other thoughts

  • (mentioned above) How do we minimize bandwidth between clusters? Ideally we'd need to do more sophisticated query planning (aggregation push-down).
  • What happens when the ideal shard factor should differ on different clusters during query parallelization?

I'm inclined to say we don't want this feature because at scale, this will be both costly and slow :( .

Agreed, introducing this feature will cause a lot of complaints

liguozhong avatar Mar 06 '23 01:03 liguozhong

@owen-d / @liguozhong This kind of remote-read federation is very much needed.

For example, if have multiple compute/cloud service providers in multiple geo regions. I would rather ship data per query over, than ship all logs over our regional links. Most log output goes un-queried, so keeping the logs inside a regional or provider boundary is desired.

We would also rather have the ability to circuit-breaker pattern such that if a single geo region/provider is unavailable we can continue to have access to the regions/providers.

Without remote read / distributed query, Loki would a SPoF in our observability architecture.

Having a remote read / federated model is why we went with Thanos over Mimir.

SuperQ avatar Mar 28 '23 08:03 SuperQ

@owen-d / @liguozhong This kind of remote-read federation is very much needed.

For example, if have multiple compute/cloud service providers in multiple geo regions. I would rather ship data per query over, than ship all logs over our regional links. Most log output goes un-queried, so keeping the logs inside a regional or provider boundary is desired.

We would also rather have the ability to circuit-breaker pattern such that if a single geo region/provider is unavailable we can continue to have access to the regions/providers.

Without remote read / distributed query, Loki would a SPoF in our observability architecture.

Having a remote read / federated model is why we went with Thanos over Mimir.

Hi, I understand your situation very well. At present, I also operate 22 loki clusters. When I need to query a traceID log, I need to click 22 times each time. This is very uncomfortable. I am willing to complete this PR to free up this unproductive duplication of work ,

needs to be discussed by more people.

liguozhong avatar Mar 28 '23 08:03 liguozhong

Having this as an option, and then improving on things like query pushdown is probably the best incremental approach. Documenting the down-sides like fan-out and lack of push-down is fine IMO.

For example in Thanos, we had lots of issues with fanout (we have 2000+ Prometheus instances), so we introduced a label enforcement proxy to make sure users selected which promehteus instances they needed per query. This is slightly less intuitive, but a lot less cumbersome to maintaining 2000+ datasources. :)

SuperQ avatar Mar 28 '23 09:03 SuperQ

I am very looking forward to launching an alpha version so that more developers can participate in optimizing the code. If there are 2000 loki, each loki is configured with a label, and then filtering different datasource based on the label can provide a very cool federation query. great idea.

wait more eyes

liguozhong avatar Mar 28 '23 09:03 liguozhong

What do you think about blocking queries that do not perform well without remote aggregation?

jeschkies avatar Mar 29 '23 15:03 jeschkies

What do you think about blocking queries that do not perform well without remote aggregation?

sorry, I missed your message. Today our dev team asked me for this feature again. I found your message in the PR today.

sum(count_over_time{app="buy2"}[1m])

As far as I know, when the querier queries the ingester, it will execute this statement(count_over_time{app="buy2"}[1m]) in the ingester instead of pulling all the original data back to the querier for execution. What the ingester returns to the queier is a time series result . And this amount of data is very small, there will be no performance problems.

The Remote read API is the same, the performance problem is mainly concentrated in s3 file download, | json calculation. instead of happening in sum

Maybe my understanding is wrong.

liguozhong avatar Apr 27 '23 07:04 liguozhong

What the ingester returns to the querier is a time series result . And this amount of data is very small, there will be no performance problems.

@liguozhong, that's because the ingester covers a small time range. If you query over days it's an issue. If I'm not mistaken we want to use something like the shard_summer here that fans out to each remote. Say you have one thousand pods and use their ids a labels. You'll have over a thousand streams returning. However, if you sum by say the namespace, the streams are reduced.

jeschkies avatar Apr 28 '23 16:04 jeschkies

What the ingester returns to the querier is a time series result . And this amount of data is very small, there will be no performance problems.

@liguozhong, that's because the ingester covers a small time range. If you query over days it's an issue. If I'm not mistaken we want to use something like the shard_summer here that fans out to each remote. Say you have one thousand pods and use their ids a labels. You'll have over a thousand streams returning. However, if you sum by say the namespace, the streams are reduced.

sorry ,I miss this review tips . I will try to make it right. thanks ,"over a thousand streams returning" this is really danger .

liguozhong avatar Jul 26 '23 06:07 liguozhong

@liguozhong This PR is over a year old. Can we close it? Or are you still hoping to get this merged?

JStickler avatar Jan 08 '24 22:01 JStickler

Any updates on this PR?

paulojmdias avatar Mar 28 '24 00:03 paulojmdias

What would be needed to finalize this PR?

icelynjennings avatar Jul 15 '24 10:07 icelynjennings

I believe this is a feature that Loki users desire and need.

With this in mind, I decided to build Lokxy, a log aggregator for Loki, designed to collect and unify log streams from multiple sources into a single, queryable endpoint. This allows the users to aggregate data from multiple Loki deployments into a single Grafana data source for example.

I'd love for you to check it out, contribute, or share your thoughts. Every feedback helps move this project forward. Let's make it better together! 💪

paulojmdias avatar Oct 15 '24 10:10 paulojmdias