Skip to content
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

[v10.0.x] Alerting: Add limit query parameter to Loki-based ASH api, drop default limit from 5000 to 1000, extend visible time range for new ASH UI #70857

Merged
merged 1 commit into from Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -131,13 +131,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 @@ -194,7 +254,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