Skip to content

Commit

Permalink
Alerting: Add limit query parameter to Loki-based ASH api, drop defau…
Browse files Browse the repository at this point in the history
…lt limit from 5000 to 1000, extend visible time range for new ASH UI (#70769)

* Add limit query parameter

* Drop copy paste comment

* Extend history query limit to 30 days and 250 entries

* Fix history log entries ordering

* Update no history message, add empty history test

---------

Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>
  • Loading branch information
alexweav and konrad147 committed Jun 28, 2023
1 parent 00d5f7f commit f94fb76
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 30 deletions.
2 changes: 2 additions & 0 deletions pkg/services/ngalert/api/api_ruler_history.go
Expand Up @@ -27,6 +27,7 @@ const labelQueryPrefix = "labels_"
func (srv *HistorySrv) RouteQueryStateHistory(c *contextmodel.ReqContext) response.Response {
from := c.QueryInt64("from")
to := c.QueryInt64("to")
limit := c.QueryInt("limit")
ruleUID := c.Query("ruleUID")

labels := make(map[string]string)
Expand All @@ -42,6 +43,7 @@ func (srv *HistorySrv) RouteQueryStateHistory(c *contextmodel.ReqContext) respon
SignedInUser: c.SignedInUser,
From: time.Unix(from, 0),
To: time.Unix(to, 0),
Limit: limit,
Labels: labels,
}
frame, err := srv.hist.Query(c.Req.Context(), query)
Expand Down
1 change: 1 addition & 0 deletions pkg/services/ngalert/models/history.go
Expand Up @@ -13,5 +13,6 @@ type HistoryQuery struct {
Labels map[string]string
From time.Time
To time.Time
Limit int
SignedInUser *user.SignedInUser
}
4 changes: 2 additions & 2 deletions pkg/services/ngalert/state/historian/loki.go
Expand Up @@ -43,7 +43,7 @@ const defaultQueryRange = 6 * time.Hour
type remoteLokiClient interface {
ping(context.Context) error
push(context.Context, []stream) error
rangeQuery(ctx context.Context, logQL string, start, end int64) (queryRes, error)
rangeQuery(ctx context.Context, logQL string, start, end, limit int64) (queryRes, error)
}

// RemoteLokibackend is a state.Historian that records state history to an external Loki instance.
Expand Down Expand Up @@ -126,7 +126,7 @@ func (h *RemoteLokiBackend) Query(ctx context.Context, query models.HistoryQuery
}

// Timestamps are expected in RFC3339Nano.
res, err := h.client.rangeQuery(ctx, logQL, query.From.UnixNano(), query.To.UnixNano())
res, err := h.client.rangeQuery(ctx, logQL, query.From.UnixNano(), query.To.UnixNano(), int64(query.Limit))
if err != nil {
return nil, err
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/services/ngalert/state/historian/loki_http.go
Expand Up @@ -17,7 +17,8 @@ import (
"github.com/weaveworks/common/http/client"
)

const defaultPageSize = 5000
const defaultPageSize = 1000
const maximumPageSize = 5000

func NewRequester() client.Requester {
return &http.Client{}
Expand Down Expand Up @@ -225,19 +226,25 @@ func (c *httpLokiClient) setAuthAndTenantHeaders(req *http.Request) {
}
}

func (c *httpLokiClient) rangeQuery(ctx context.Context, logQL string, start, end int64) (queryRes, error) {
func (c *httpLokiClient) rangeQuery(ctx context.Context, logQL string, start, end, limit int64) (queryRes, error) {
// Run the pre-flight checks for the query.
if start > end {
return queryRes{}, fmt.Errorf("start time cannot be after end time")
}
if limit < 1 {
limit = defaultPageSize
}
if limit > maximumPageSize {
limit = maximumPageSize
}

queryURL := c.cfg.ReadPathURL.JoinPath("/loki/api/v1/query_range")

values := url.Values{}
values.Set("query", logQL)
values.Set("start", fmt.Sprintf("%d", start))
values.Set("end", fmt.Sprintf("%d", end))
values.Set("limit", fmt.Sprintf("%d", defaultPageSize))
values.Set("limit", fmt.Sprintf("%d", limit))

queryURL.RawQuery = values.Encode()

Expand Down
64 changes: 62 additions & 2 deletions pkg/services/ngalert/state/historian/loki_http_test.go
Expand Up @@ -145,13 +145,73 @@ func TestLokiHTTPClient(t *testing.T) {
now := time.Now().UTC().UnixNano()
q := `{from="state-history"}`

_, err := client.rangeQuery(context.Background(), q, now-100, now)
_, err := client.rangeQuery(context.Background(), q, now-100, now, 1100)

require.NoError(t, err)
params := req.lastRequest.URL.Query()
require.True(t, params.Has("limit"), "query params did not contain 'limit': %#v", params)
require.Equal(t, fmt.Sprint(1100), params.Get("limit"))
})

t.Run("uses default page size if limit not provided", func(t *testing.T) {
req := NewFakeRequester().WithResponse(&http.Response{
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
ContentLength: int64(0),
Header: make(http.Header, 0),
})
client := createTestLokiClient(req)
now := time.Now().UTC().UnixNano()
q := `{from="state-history"}`

_, err := client.rangeQuery(context.Background(), q, now-100, now, 0)

require.NoError(t, err)
params := req.lastRequest.URL.Query()
require.True(t, params.Has("limit"), "query params did not contain 'limit': %#v", params)
require.Equal(t, fmt.Sprint(defaultPageSize), params.Get("limit"))
})

t.Run("uses default page size if limit invalid", func(t *testing.T) {
req := NewFakeRequester().WithResponse(&http.Response{
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
ContentLength: int64(0),
Header: make(http.Header, 0),
})
client := createTestLokiClient(req)
now := time.Now().UTC().UnixNano()
q := `{from="state-history"}`

_, err := client.rangeQuery(context.Background(), q, now-100, now, -100)

require.NoError(t, err)
params := req.lastRequest.URL.Query()
require.True(t, params.Has("limit"), "query params did not contain 'limit': %#v", params)
require.Equal(t, fmt.Sprint(defaultPageSize), params.Get("limit"))
})

t.Run("uses maximum page size if limit too big", func(t *testing.T) {
req := NewFakeRequester().WithResponse(&http.Response{
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
ContentLength: int64(0),
Header: make(http.Header, 0),
})
client := createTestLokiClient(req)
now := time.Now().UTC().UnixNano()
q := `{from="state-history"}`

_, err := client.rangeQuery(context.Background(), q, now-100, now, maximumPageSize+1000)

require.NoError(t, err)
params := req.lastRequest.URL.Query()
require.True(t, params.Has("limit"), "query params did not contain 'limit': %#v", params)
require.Equal(t, fmt.Sprint(maximumPageSize), params.Get("limit"))
})
})
}

Expand Down Expand Up @@ -208,7 +268,7 @@ func TestLokiHTTPClient_Manual(t *testing.T) {
end := time.Now().UnixNano()

// Authorized request should not fail against Grafana Cloud.
res, err := client.rangeQuery(context.Background(), logQL, start, end)
res, err := client.rangeQuery(context.Background(), logQL, start, end, defaultPageSize)
require.NoError(t, err)
require.NotNil(t, res)
})
Expand Down
8 changes: 3 additions & 5 deletions public/app/features/alerting/unified/api/stateHistoryApi.ts
@@ -1,15 +1,13 @@
import { getUnixTime } from 'date-fns';

import { DataFrameJSON } from '@grafana/data';

import { alertingApi } from './alertingApi';

export const stateHistoryApi = alertingApi.injectEndpoints({
endpoints: (build) => ({
getRuleHistory: build.query<DataFrameJSON, { ruleUid: string; from: number; to?: number }>({
query: ({ ruleUid, from, to = getUnixTime(new Date()) }) => ({
getRuleHistory: build.query<DataFrameJSON, { ruleUid: string; from?: number; to?: number; limit?: number }>({
query: ({ ruleUid, from, to, limit = 100 }) => ({
url: '/api/v1/rules/history',
params: { ruleUID: ruleUid, from, to },
params: { ruleUID: ruleUid, from, to, limit },
}),
}),
}),
Expand Down
Expand Up @@ -19,21 +19,27 @@ interface LogRecordViewerProps {
onLabelClick?: (label: string) => void;
}

function groupRecordsByTimestamp(records: LogRecord[]) {
// groupBy has been replaced by the reduce to avoid back and forth conversion of timestamp from number to string
const groupedLines = records.reduce((acc, current) => {
const tsGroup = acc.get(current.timestamp);
if (tsGroup) {
tsGroup.push(current);
} else {
acc.set(current.timestamp, [current]);
}

return acc;
}, new Map<number, LogRecord[]>());

return new Map([...groupedLines].sort((a, b) => b[0] - a[0]));
}

export const LogRecordViewerByTimestamp = React.memo(
({ records, commonLabels, onLabelClick, onRecordsRendered }: LogRecordViewerProps) => {
const styles = useStyles2(getStyles);

// groupBy has been replaced by the reduce to avoid back and forth conversion of timestamp from number to string
const groupedLines = records.reduce((acc, current) => {
const tsGroup = acc.get(current.timestamp);
if (tsGroup) {
tsGroup.push(current);
} else {
acc.set(current.timestamp, [current]);
}

return acc;
}, new Map<number, LogRecord[]>());
const groupedLines = groupRecordsByTimestamp(records);

const timestampRefs = new Map<number, HTMLElement>();
useEffect(() => {
Expand Down
Expand Up @@ -81,6 +81,7 @@ const ui = {
loadingIndicator: byText('Loading...'),
timestampViewer: byRole('list', { name: 'State history by timestamp' }),
record: byRole('listitem'),
noRecords: byText('No state transitions have occurred in the last 30 days'),
};

describe('LokiStateHistory', () => {
Expand All @@ -96,4 +97,18 @@ describe('LokiStateHistory', () => {
expect(timestampViewerElement).toHaveTextContent('/api/live/ws');
expect(timestampViewerElement).toHaveTextContent('/api/folders/:uid/');
});

it('should render no entries message when no records are returned', async () => {
server.use(
rest.get('/api/v1/rules/history', (req, res, ctx) =>
res(ctx.json<DataFrameJSON>({ data: { values: [] }, schema: { fields: [] } }))
)
);

render(<LokiStateHistory ruleUID="abcd" />, { wrapper: TestProvider });

await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument());

expect(ui.noRecords.get()).toBeInTheDocument();
});
});
Expand Up @@ -29,20 +29,28 @@ const LokiStateHistory = ({ ruleUID }: Props) => {
const { getValues, setValue, register, handleSubmit } = useForm({ defaultValues: { query: '' } });

const { useGetRuleHistoryQuery } = stateHistoryApi;
const timeRange = useMemo(() => getDefaultTimeRange(), []);

// We prefer log count-based limit rather than time-based, but the API doesn't support it yet
const queryTimeRange = useMemo(() => getDefaultTimeRange(), []);

const {
currentData: stateHistory,
isLoading,
isError,
error,
} = useGetRuleHistoryQuery({ ruleUid: ruleUID, from: timeRange.from.unix(), to: timeRange.to.unix() });
} = useGetRuleHistoryQuery({
ruleUid: ruleUID,
from: queryTimeRange.from.unix(),
to: queryTimeRange.to.unix(),
limit: 250,
});

const { dataFrames, historyRecords, commonLabels, totalRecordsCount } = useRuleHistoryRecords(
stateHistory,
instancesFilter
);

const { frameSubset, frameSubsetTimestamps } = useFrameSubset(dataFrames);
const { frameSubset, frameSubsetTimestamps, frameTimeRange } = useFrameSubset(dataFrames);

const onLogRecordLabelClick = useCallback(
(label: string) => {
Expand Down Expand Up @@ -83,7 +91,7 @@ const LokiStateHistory = ({ ruleUID }: Props) => {
const emptyStateMessage =
totalRecordsCount > 0
? `No matches were found for the given filters among the ${totalRecordsCount} instances`
: 'No state transitions have occurred in the last 60 minutes';
: 'No state transitions have occurred in the last 30 days';

return (
<div className={styles.fullSize}>
Expand Down Expand Up @@ -120,7 +128,7 @@ const LokiStateHistory = ({ ruleUID }: Props) => {
) : (
<>
<div className={styles.graphWrapper}>
<LogTimelineViewer frames={frameSubset} timeRange={timeRange} onPointerMove={onTimelinePointerMove} />
<LogTimelineViewer frames={frameSubset} timeRange={frameTimeRange} onPointerMove={onTimelinePointerMove} />
</div>
{hasMoreInstances && (
<div className={styles.moreInstancesWarning}>
Expand All @@ -147,7 +155,22 @@ function useFrameSubset(frames: DataFrame[]) {
const frameSubset = take(frames, MAX_TIMELINE_SERIES);
const frameSubsetTimestamps = sortBy(uniq(frameSubset.flatMap((frame) => frame.fields[0].values)));

return { frameSubset, frameSubsetTimestamps };
const minTs = Math.min(...frameSubsetTimestamps);
const maxTs = Math.max(...frameSubsetTimestamps);

const rangeStart = dateTime(minTs);
const rangeStop = dateTime(maxTs);

const frameTimeRange: TimeRange = {
from: rangeStart,
to: rangeStop,
raw: {
from: rangeStart,
to: rangeStop,
},
};

return { frameSubset, frameSubsetTimestamps, frameTimeRange };
}, [frames]);
}

Expand Down Expand Up @@ -199,7 +222,7 @@ const SearchFieldInput = React.forwardRef<HTMLInputElement, SearchFieldInputProp
SearchFieldInput.displayName = 'SearchFieldInput';

function getDefaultTimeRange(): TimeRange {
const fromDateTime = dateTime().subtract(1, 'h');
const fromDateTime = dateTime().subtract(30, 'days');
const toDateTime = dateTime();
return {
from: fromDateTime,
Expand Down

0 comments on commit f94fb76

Please sign in to comment.