Skip to content

Commit

Permalink
fix: empty label values missing labelValues (#1111)
Browse files Browse the repository at this point in the history
* fix: empty label values missing labelValues
  • Loading branch information
gtk-grafana authored Feb 27, 2025
1 parent b02dce9 commit bfbf715
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 9 deletions.
43 changes: 37 additions & 6 deletions src/services/extensions/links.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
ValidByteUnitValues,
validDurationValues,
} from '../../Components/ServiceScene/Breakdowns/NumericFilterPopoverScene';
import { addAdHocFilterUserInputPrefix } from '../variables';
import { addAdHocFilterUserInputPrefix, EMPTY_VARIABLE_VALUE } from '../variables';
import { addCustomInputPrefixAndValueLabels, encodeFilter, getPath } from './utils';

function getTestConfig(
Expand Down Expand Up @@ -249,7 +249,9 @@ describe('contextToLink', () => {
)},downstream`
)}` +
`&var-fields=${encodeFilter(
`error|!=|${addAdHocFilterUserInputPrefix('{"value":""__gfc__"parser":"logfmt"}')},`
`error|!=|${addAdHocFilterUserInputPrefix(
`{"value":${EMPTY_VARIABLE_VALUE}__gfc__"parser":"logfmt"}`
)},${EMPTY_VARIABLE_VALUE}`
)}` +
`&var-fields=${encodeFilter(
`endpoint|=|${addAdHocFilterUserInputPrefix('{"value":"queryData"__gfc__"parser":"logfmt"}')},queryData`
Expand Down Expand Up @@ -293,7 +295,9 @@ describe('contextToLink', () => {
)},downstream`
)}` +
`&var-fields=${encodeFilter(
`error|!=|${addAdHocFilterUserInputPrefix('{"value":""__gfc__"parser":"logfmt"}')},`
`error|!=|${addAdHocFilterUserInputPrefix(
`{"value":${EMPTY_VARIABLE_VALUE}__gfc__"parser":"logfmt"}`
)},${EMPTY_VARIABLE_VALUE}`
)}` +
`&var-fields=${encodeFilter(
`endpoint|=|${addAdHocFilterUserInputPrefix('{"value":"queryData"__gfc__"parser":"logfmt"}')},queryData`
Expand Down Expand Up @@ -403,6 +407,33 @@ describe('contextToLink', () => {
}),
});
});
it('should add label for empty variable value', () => {
const target = getTestTarget({
expr: `{cluster="C:\\Grafana\\logs\\log.txt"} | pod!=\`mimir-ingester-xjntw\` | logfmt | msg=${EMPTY_VARIABLE_VALUE}`,
});
const config = getTestConfig(linkConfigs, target);

const expectedLabelFiltersUrlString = `&var-filters=${encodeFilter(
`cluster|=|${addCustomInputPrefixAndValueLabels('C:\\Grafana\\logs\\log.txt')}`
)}`;
const expectedMetadataString = `&var-metadata=${encodeFilter(
`pod|!=|${addCustomInputPrefixAndValueLabels('mimir-ingester-xjntw')}`
)}`;
const expectedFieldsUrlString = `&var-fields=${encodeFilter(
`msg|=|${addAdHocFilterUserInputPrefix(
`{"value":${EMPTY_VARIABLE_VALUE}__gfc__"parser":"logfmt"}`
)},${EMPTY_VARIABLE_VALUE}`
)}`;

expect(config).toEqual({
path: getPath({
slug: 'cluster/C:-Grafana-logs-log.txt',
expectedLabelFiltersUrlString,
expectedMetadataString,
expectedFieldsUrlString,
}),
});
});
it('should parse label with escape chars, escape chars should get replaced in url', () => {
const target = getTestTarget({
expr: `{cluster="C:\\Grafana\\logs\\log.txt"} | pod!=\`mimir-ingester-xjntw\` `,
Expand Down Expand Up @@ -446,13 +477,13 @@ describe('contextToLink', () => {
});
});
it('should parse field with logfmt parser', () => {
const target = getTestTarget({ expr: `{cluster="eu-west-1"} | logfmt | pod=\`mimir-ingester-xjntw\` ` });
const target = getTestTarget({ expr: `{cluster="eu-west-1"} | logfmt | pod=\`mimir-ingester-xjntw\`` });
const config = getTestConfig(linkConfigs, target);

const expectedLabelFiltersUrlString = `&var-filters=${encodeFilter(
`cluster|=|${addCustomInputPrefixAndValueLabels('eu-west-1')}`
)}`;
const expectedLineFiltersUrlString = `&var-fields=${encodeFilter(
const expectedFieldsUrlString = `&var-fields=${encodeFilter(
`pod|=|${addAdHocFilterUserInputPrefix(
'{"value":"mimir-ingester-xjntw"__gfc__"parser":"logfmt"}'
)},mimir-ingester-xjntw`
Expand All @@ -462,7 +493,7 @@ describe('contextToLink', () => {
path: getPath({
slug: 'cluster/eu-west-1',
expectedLabelFiltersUrlString,
expectedLineFiltersUrlString,
expectedFieldsUrlString,
}),
});
});
Expand Down
15 changes: 12 additions & 3 deletions src/services/extensions/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
addAdHocFilterUserInputPrefix,
AdHocFieldValue,
AppliedPattern,
EMPTY_VARIABLE_VALUE,
LEVEL_VARIABLE_VALUE,
SERVICE_NAME,
stripAdHocFilterUserInputPrefix,
Expand Down Expand Up @@ -64,7 +65,7 @@ export const linkConfigs: LinkConfigs = [

function stringifyValues(value?: string): string {
if (!value) {
return '""';
return EMPTY_VARIABLE_VALUE;
}
return value;
}
Expand All @@ -76,13 +77,21 @@ export function replaceEscapeChars(value?: string): string | undefined {

export function stringifyAdHocValues(value?: string): string {
if (!value) {
return '""';
return EMPTY_VARIABLE_VALUE;
}

// All label values from explore are already escaped, so we mark them as custom values to prevent them from getting escaped again when rendering the LogQL
return addAdHocFilterUserInputPrefix(replaceEscapeChars(value));
}

export function stringifyAdHocValueLabels(value?: string): string {
if (!value) {
return EMPTY_VARIABLE_VALUE;
}

return escapeURLDelimiters(replaceEscapeChars(value));
}

function contextToLink<T extends PluginExtensionPanelContext>(context?: T) {
if (!context) {
return undefined;
Expand Down Expand Up @@ -162,7 +171,7 @@ function contextToLink<T extends PluginExtensionPanelContext>(context?: T) {

const adHocFilterURLString = `${field.key}|${field.operator}|${escapeURLDelimiters(
stringifyAdHocValues(JSON.stringify(fieldValue))
)},${escapeURLDelimiters(replaceEscapeChars(fieldValue.value))}`;
)},${stringifyAdHocValueLabels(fieldValue.value)}`;

params = appendUrlParameter(UrlParameters.Fields, adHocFilterURLString, params);
}
Expand Down

0 comments on commit bfbf715

Please sign in to comment.