public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Loki: Fix merging responses would merge null notices (#112920)

Browse Source 
* Loki: Fix merging responses would merge `null` notices

* fix tests
This commit is contained in:
Sven Grossmann
2025-10-24 12:28:39 +02:00
committed by GitHub
parent bb6d7d02c7
commit f03125279a
2 changed files with 120 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/app/plugins/datasource/loki/mergeResponses.test.ts
+97 -1
View File
@@ -1,6 +1,6 @@
import { cloneDeep } from 'lodash';
import { DataQueryResponse, Field, FieldType, QueryResultMetaStat } from '@grafana/data';
import { DataQueryResponse, Field, FieldType, QueryResultMetaNotice, QueryResultMetaStat } from '@grafana/data';
import { cloneQueryResponse, combineResponses } from './mergeResponses';
import { getMockFrames } from './mocks/frames';
@@ -76,6 +76,7 @@ describe('combineResponses', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -154,6 +155,7 @@ describe('combineResponses', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -199,6 +201,7 @@ describe('combineResponses', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -244,6 +247,7 @@ describe('combineResponses', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -424,6 +428,7 @@ describe('combineResponses', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -499,6 +504,7 @@ describe('combineResponses', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -574,6 +580,7 @@ describe('combineResponses', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -639,6 +646,84 @@ describe('combineResponses', () => {
});
});
describe('combine notices', () => {
const { metricFrameA } = getMockFrames();
const makeResponse = (notices?: QueryResultMetaNotice[]): DataQueryResponse => ({
data: [
{
...metricFrameA,
meta: {
...metricFrameA.meta,
notices,
},
},
],
});
it('combines notices from both frames', () => {
const responseA = makeResponse([{ severity: 'warning', text: 'Warning from frame A' }]);
const responseB = makeResponse([{ severity: 'info', text: 'Info from frame B' }]);
expect(combineResponses(responseA, responseB).data[0].meta?.notices).toStrictEqual([
{ severity: 'warning', text: 'Warning from frame A' },
{ severity: 'info', text: 'Info from frame B' },
]);
});
it('deduplicates identical notices', () => {
const responseA = makeResponse([{ severity: 'warning', text: 'Same warning' }]);
const responseB = makeResponse([{ severity: 'warning', text: 'Same warning' }]);
expect(combineResponses(responseA, responseB).data[0].meta?.notices).toStrictEqual([
{ severity: 'warning', text: 'Same warning' },
]);
});
it('keeps notices with same text but different severity', () => {
const responseA = makeResponse([{ severity: 'warning', text: 'Message' }]);
const responseB = makeResponse([{ severity: 'info', text: 'Message' }]);
expect(combineResponses(responseA, responseB).data[0].meta?.notices).toStrictEqual([
{ severity: 'warning', text: 'Message' },
{ severity: 'info', text: 'Message' },
]);
});
it('handles one frame with notices and one without', () => {
const responseA = makeResponse([{ severity: 'warning', text: 'Warning message' }]);
const responseB = makeResponse();
expect(combineResponses(responseA, responseB).data[0].meta?.notices).toStrictEqual([
{ severity: 'warning', text: 'Warning message' },
]);
expect(combineResponses(responseB, responseA).data[0].meta?.notices).toStrictEqual([
{ severity: 'warning', text: 'Warning message' },
]);
});
it('returns empty array when neither frame has notices', () => {
const responseA = makeResponse();
const responseB = makeResponse();
expect(combineResponses(responseA, responseB).data[0].meta?.notices).toHaveLength(0);
});
it('filters out null values from notices arrays', () => {
const responseA = makeResponse([
{ severity: 'warning', text: 'Valid warning' },
null as unknown as QueryResultMetaNotice, // Simulating the bug scenario
]);
const responseB = makeResponse([{ severity: 'info', text: 'Valid info' }]);
const result = combineResponses(responseA, responseB).data[0].meta?.notices;
expect(result).toStrictEqual([
{ severity: 'warning', text: 'Valid warning' },
{ severity: 'info', text: 'Valid info' },
]);
expect(result).not.toContainEqual(null);
});
});
it('does not combine frames with different refId', () => {
const { metricFrameA, metricFrameB } = getMockFrames();
metricFrameA.refId = 'A';
@@ -730,6 +815,7 @@ describe('combineResponses', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -796,6 +882,7 @@ describe('combineResponses', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -843,6 +930,7 @@ describe('mergeFrames', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -892,6 +980,7 @@ describe('mergeFrames', () => {
length: 3,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -937,6 +1026,7 @@ describe('mergeFrames', () => {
length: 4,
meta: {
type: 'timeseries-multi',
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -1016,6 +1106,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -1098,6 +1189,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -1129,6 +1221,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -1208,6 +1301,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
@@ -1242,6 +1336,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [{ displayName: 'Summary: total bytes processed', unit: 'decbytes', value: 22 }],
},
length: 2,
@@ -1267,6 +1362,7 @@ describe('mergeFrames', () => {
custom: {
frameType: 'LabeledTimeValues',
},
notices: [],
stats: [
{
displayName: 'Summary: total bytes processed',
public/app/plugins/datasource/loki/mergeResponses.ts
+23
View File
@@ -7,6 +7,7 @@ import {
Field,
FieldType,
LoadingState,
QueryResultMetaNotice,
QueryResultMetaStat,
shallowCompare,
} from '@grafana/data';
@@ -197,6 +198,7 @@ export function mergeFrames(dest: DataFrame, source: DataFrame) {
dest.meta = {
...dest.meta,
stats: getCombinedMetadataStats(dest.meta?.stats ?? [], source.meta?.stats ?? []),
notices: getCombinedNotices(dest.meta?.notices ?? [], source.meta?.notices ?? []),
};
}
@@ -288,6 +290,27 @@ function getCombinedMetadataStats(
return stats;
}
function getCombinedNotices(
destNotices: QueryResultMetaNotice[],
sourceNotices: QueryResultMetaNotice[]
): QueryResultMetaNotice[] {
// Combine notices from both frames and filter out null/undefined values
const allNotices = [...destNotices, ...sourceNotices].filter(
(notice): notice is QueryResultMetaNotice => notice != null
);
// Deduplicate notices based on text to avoid showing the same warning twice
const uniqueNotices = allNotices.reduce((acc: QueryResultMetaNotice[], notice) => {
const exists = acc.some((n) => n.severity === notice.severity && n.text === notice.text);
if (!exists) {
acc.push(notice);
}
return acc;
}, []);
return uniqueNotices;
}
/**
* Deep clones a DataQueryResponse
*/