Loki: Remove alpha feature toggle lokiDataframeApi #65918
Conversation
gabor
requested review from
tskarhed,
ashharrison90,
andresmgot,
papagian,
zserge and
mildwonkey
and removed request for
a team
gabor
added
no-backport
|
Backend code coverage report for PR #65918
|
|
Frontend code coverage report for PR #65918 |
|
You have successfully added a new CodeQL configuration |
|
(Open the links below in a new tab to go to the correct steps)
|
grafanabot
added
the
levitate breaking change
|
@slim-bean are you still using this to debug our clusters? If yes, it seems worthwhile to keep it around -- and also still feels like a good direction to explore: have loki produce the format we use in the UI rather than a complex process in between. |
gabor
changed the title
gabor
added
add to changelog
and removed
no-changelog
gabor
changed the title
| @@ -13,8 +13,6 @@ import ( | |||
| "github.com/gorilla/websocket" | |||
| "github.com/grafana/grafana-plugin-sdk-go/backend" | |||
| "github.com/grafana/grafana-plugin-sdk-go/data" | |||
|
|
|||
| "github.com/grafana/grafana/pkg/services/featuremgmt" | |||
| ) | |||
|
|
|||
| func (s *Service) SubscribeStream(_ context.Context, req *backend.SubscribeStreamRequest) (*backend.SubscribeStreamResponse, error) { | |||
There was a problem hiding this comment.
Is this ONLY called when lokiLive feature toggle is enabled? If so this is fine, but I thought we were trying to do both?
There was a problem hiding this comment.
hi @ryantxu ... i'll try to summarize the situation here to be sure there are no misunderstandings:
(note: this functionality has nothing to do with the "tail" functionality in explore-mode)
- the
lokiLivefeature flag adds themode=streamingoption into the loki query editor. when that mode is chosen, the request comes here, toloki/streaming.go - in
loki/streaming.gothere are two ways to communicate with Loki: the "stable"/loki/api/v1/tailendpoint, and the experimental/loki/api/v2alpha/tailendpoint. the system chooses between them based on thelokiDataframeApifeature-flag.
i talked with @slim-bean about how to move forward with these experimental feature-flags, and got the info that the combination of lokiLive=true & lokiDataframeApi=false combination does not make sense (meaning, we stream, but go to the old endpoint). based on this info, i removed the feature-flag lokiDataframeApi, and everywhere i assumed it is set to true. this is the PR we are looking at.
is this acceptable?
(in general, we can always add code back, i'm just trying to move somehow forward with these experimental feature-flags. if you have a different idea, i'm fine with discussing it 👍 )
| @@ -84,15 +82,9 @@ func (s *Service) RunStream(ctx context.Context, req *backend.RunStreamRequest, | |||
| params := url.Values{} | |||
| params.Add("query", query.Expr) | |||
|
|
|||
| lokiDataframeApi := s.features.IsEnabled(featuremgmt.FlagLokiDataframeApi) | |||
There was a problem hiding this comment.
should this just check the lokiLive flag instead? so the stream endpoint still works?
IIRC, this got split into two so we could explore using the direct format in other places as well
There was a problem hiding this comment.
hi @ryantxu , this PR is basically:
"replace lokiDataFrameApi feature-flag with true, and remove the feature-flag".
currently it's not possible to use the direct format (i assume you mean the loki-emits-a-dataframe mode) anywhere except the mode=streaming option, and this PR is not regressing on that aspect... but maybe i misunderstood something?
loki: remove experimental feature-flag
(fixes #65783)
after some investigation it seems the only useful way to use the
lokiLiveis to use it together withlokiDataframeApi,so we are removing
lokiDataframeApi, and assume it is alwaystrue.this is all behind the
lokiLivefeature-flag, so will not affect normal grafana usage.