From 425917cc9149de8fb12f7623900dda58a3bfbfc9 Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 10:06:51 -0500 Subject: [PATCH 01/15] fix: fix labels not updating when detected_labels is updated while viewing another tab --- .../LabelsAggregatedBreakdownScene.tsx | 187 ++++++++++++++---- src/Components/ServiceScene/ServiceScene.tsx | 8 +- 2 files changed, 156 insertions(+), 39 deletions(-) diff --git a/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx b/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx index 7c805aeac..01b7bf107 100644 --- a/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx +++ b/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx @@ -9,12 +9,18 @@ import { SceneObjectBase, SceneObjectState, SceneQueryRunner, + VariableValueOption, VizPanel, } from '@grafana/scenes'; import { LayoutSwitcher } from './LayoutSwitcher'; import { DrawStyle, LoadingPlaceholder, StackingMode } from '@grafana/ui'; import { getQueryRunner, setLevelColorOverrides } from '../../../services/panel'; -import { ALL_VARIABLE_VALUE, getFieldsVariable, getLabelGroupByVariable } from '../../../services/variables'; +import { + ALL_VARIABLE_VALUE, + getFieldsVariable, + getLabelGroupByVariable, + LEVEL_VARIABLE_VALUE, +} from '../../../services/variables'; import React from 'react'; import { LabelBreakdownScene } from './LabelBreakdownScene'; import { SelectLabelActionScene } from './SelectLabelActionScene'; @@ -23,6 +29,8 @@ import { limitMaxNumberOfSeriesForPanel, MAX_NUMBER_OF_TIME_SERIES } from './Tim import { limitFramesTransformation } from './FieldsAggregatedBreakdownScene'; import { LokiQuery } from '../../../services/query'; import { buildLabelsQuery, LABEL_BREAKDOWN_GRID_TEMPLATE_COLUMNS } from '../../../services/labels'; +import { ServiceScene } from '../ServiceScene'; +import { DataFrame, LoadingState } from '@grafana/data'; export interface LabelsAggregatedBreakdownSceneState extends SceneObjectState { body?: LayoutSwitcher; @@ -39,12 +47,30 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase { + $detectedLabels?.subscribeToState((newState, prevState) => { + if (newState.data?.state === LoadingState.Done) { + this.update(newState.data.series[0]); + } + }) + ); + + this._subs.add( + fields.subscribeToState(() => { this.updateQueriesOnFieldsVariableChange(); }) ); @@ -55,10 +81,7 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase opt.value !== ALL_VARIABLE_VALUE).map((opt) => opt.label); + + this.state.body?.state.layouts.forEach((layoutObj) => { + let existingFields = []; + const layout = layoutObj as SceneCSSGridLayout; + const newLabelsSet = new Set(newFields); + const updatedChildren = layout.state.children as SceneCSSGridItem[]; + + for (let i = 0; i < updatedChildren.length; i++) { + const { title } = this.getPanelByIndex(layout, i); + + if (newLabelsSet.has(title)) { + // If the new response has this field, delete it from the set, but leave it in the layout + newLabelsSet.delete(title); + } else { + // Otherwise if the panel doesn't exist in the response, delete it from the layout + updatedChildren.splice(i, 1); + // And make sure to update the index, or we'll skip the next one + i--; + } + existingFields.push(title); + } + + const fieldsToAdd = Array.from(newLabelsSet); + + const options = fieldsToAdd.map((fieldName) => { + return { + label: fieldName, + value: fieldName, + }; + }); + + updatedChildren.push(...this.buildChildren(options)); + + const cardinalityMap = this.calculateCardinalityMap(detectedFieldsFrame); + updatedChildren.sort(this.sortChildren(cardinalityMap)); + updatedChildren.map((child) => { + limitMaxNumberOfSeriesForPanel(child); + }); + + layout.setState({ + children: updatedChildren, + }); + }); + } + + private calculateCardinalityMap(detectedLabels?: DataFrame) { + const cardinalityMap = new Map(); + if (detectedLabels?.length) { + for (let i = 0; i < detectedLabels?.fields.length; i++) { + const name: string = detectedLabels.fields[i].name; + const cardinality: number = detectedLabels.fields[i].values[0]; + cardinalityMap.set(name, cardinality); + } + } + return cardinalityMap; + } + private build(): LayoutSwitcher { const variable = getLabelGroupByVariable(this); const labelBreakdownScene = sceneGraph.getAncestor(this, LabelBreakdownScene); labelBreakdownScene.state.search.reset(); - const children: SceneCSSGridItem[] = []; - for (const option of variable.state.options) { - const { value } = option; - const optionValue = String(value); - if (value === ALL_VARIABLE_VALUE || !value) { - continue; - } - const query = buildLabelsQuery(this, String(option.value), String(option.value)); - const dataTransformer = this.getDataTransformer(query); + const children = this.buildChildren(variable.state.options); - children.push( - new SceneCSSGridItem({ - body: PanelBuilders.timeseries() - .setTitle(optionValue) - .setData(dataTransformer) - .setHeaderActions(new SelectLabelActionScene({ labelName: optionValue, fieldType: ValueSlugs.label })) - .setCustomFieldConfig('stacking', { mode: StackingMode.Normal }) - .setCustomFieldConfig('fillOpacity', 100) - .setCustomFieldConfig('lineWidth', 0) - .setCustomFieldConfig('pointSize', 0) - .setCustomFieldConfig('drawStyle', DrawStyle.Bars) - .setOverrides(setLevelColorOverrides) - .build(), - }) - ); + const serviceScene = sceneGraph.getAncestor(this, ServiceScene); + const $detectedLabels = serviceScene.state.$detectedLabelsData; + if ($detectedLabels?.state.data?.state === LoadingState.Done) { + const cardinalityMap = this.calculateCardinalityMap($detectedLabels?.state.data.series[0]); + children.sort(this.sortChildren(cardinalityMap)); } const childrenClones = children.map((child) => child.clone()); @@ -132,12 +205,58 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase child.clone()), + children: childrenClones, }), ], }); } + private buildChildren(options: VariableValueOption[]) { + const children: SceneCSSGridItem[] = []; + for (const option of options) { + const { value } = option; + const optionValue = String(value); + if (value === ALL_VARIABLE_VALUE || !value) { + continue; + } + const query = buildLabelsQuery(this, String(option.value), String(option.value)); + const dataTransformer = this.getDataTransformer(query); + + children.push( + new SceneCSSGridItem({ + body: PanelBuilders.timeseries() + .setTitle(optionValue) + .setData(dataTransformer) + .setHeaderActions(new SelectLabelActionScene({ labelName: optionValue, fieldType: ValueSlugs.label })) + .setCustomFieldConfig('stacking', { mode: StackingMode.Normal }) + .setCustomFieldConfig('fillOpacity', 100) + .setCustomFieldConfig('lineWidth', 0) + .setCustomFieldConfig('pointSize', 0) + .setCustomFieldConfig('drawStyle', DrawStyle.Bars) + .setOverrides(setLevelColorOverrides) + .build(), + }) + ); + } + return children; + } + + private sortChildren(cardinalityMap: Map) { + return (a: SceneCSSGridItem, b: SceneCSSGridItem) => { + const aPanel = a.state.body as VizPanel; + const bPanel = b.state.body as VizPanel; + if (aPanel.state.title === LEVEL_VARIABLE_VALUE) { + return -1; + } + if (bPanel.state.title === LEVEL_VARIABLE_VALUE) { + return 1; + } + const aCardinality = cardinalityMap.get(aPanel.state.title) ?? 0; + const bCardinality = cardinalityMap.get(bPanel.state.title) ?? 0; + return bCardinality - aCardinality; + }; + } + private getDataTransformer(query: LokiQuery) { const queryRunner = getQueryRunner([query]); return new SceneDataTransformer({ diff --git a/src/Components/ServiceScene/ServiceScene.tsx b/src/Components/ServiceScene/ServiceScene.tsx index 99bce0b1a..e99f8b8c7 100644 --- a/src/Components/ServiceScene/ServiceScene.tsx +++ b/src/Components/ServiceScene/ServiceScene.tsx @@ -260,10 +260,7 @@ export class ServiceScene extends SceneObjectBase { } // If we don't have a detected labels count, or we are activating the labels scene, run the detected labels query - if ( - ((slug === PageSlugs.labels || parentSlug === ValueSlugs.label) && !this.state.$detectedLabelsData?.state.data) || - this.state.labelsCount === undefined - ) { + if (slug === PageSlugs.labels || this.state.labelsCount === undefined) { this.state.$detectedLabelsData?.runQueries(); } @@ -298,7 +295,8 @@ export class ServiceScene extends SceneObjectBase { const detectedLabelsFields = detectedLabelsResponse.series[0].fields; if (detectedLabelsResponse.series.length !== undefined && detectedLabelsFields.length !== undefined) { this.setState({ - labelsCount: detectedLabelsFields.length, + // Make sure to add one extra for the detected_level + labelsCount: detectedLabelsFields.length + 1, }); getMetadataService().setLabelsCount(detectedLabelsFields.length); } From 8989d083b5305bb48975ff7b583c34dfdcaa773f Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 11:02:56 -0500 Subject: [PATCH 02/15] fix: load detected_labels on label values --- src/Components/ServiceScene/ServiceScene.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/ServiceScene/ServiceScene.tsx b/src/Components/ServiceScene/ServiceScene.tsx index e99f8b8c7..99c858c32 100644 --- a/src/Components/ServiceScene/ServiceScene.tsx +++ b/src/Components/ServiceScene/ServiceScene.tsx @@ -260,7 +260,7 @@ export class ServiceScene extends SceneObjectBase { } // If we don't have a detected labels count, or we are activating the labels scene, run the detected labels query - if (slug === PageSlugs.labels || this.state.labelsCount === undefined) { + if (slug === PageSlugs.labels || parentSlug === ValueSlugs.label || this.state.labelsCount === undefined) { this.state.$detectedLabelsData?.runQueries(); } From cc6a247011d032f749519152469af8bc7718a43c Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 11:05:05 -0500 Subject: [PATCH 03/15] chore: rename variables --- .../LabelsAggregatedBreakdownScene.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx b/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx index 01b7bf107..67b87e771 100644 --- a/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx +++ b/src/Components/ServiceScene/Breakdowns/LabelsAggregatedBreakdownScene.tsx @@ -107,14 +107,14 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase opt.value !== ALL_VARIABLE_VALUE).map((opt) => opt.label); + const newLabels = variable.state.options.filter((opt) => opt.value !== ALL_VARIABLE_VALUE).map((opt) => opt.label); this.state.body?.state.layouts.forEach((layoutObj) => { - let existingFields = []; + let existingLabels = []; const layout = layoutObj as SceneCSSGridLayout; - const newLabelsSet = new Set(newFields); + const newLabelsSet = new Set(newLabels); const updatedChildren = layout.state.children as SceneCSSGridItem[]; for (let i = 0; i < updatedChildren.length; i++) { @@ -129,12 +129,12 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase { + const options = labelsToAdd.map((fieldName) => { return { label: fieldName, value: fieldName, @@ -143,7 +143,7 @@ export class LabelsAggregatedBreakdownScene extends SceneObjectBase { limitMaxNumberOfSeriesForPanel(child); From 6bfac1ffd65cda9d42cb52a68535ff27ac00ff12 Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 11:16:50 -0500 Subject: [PATCH 04/15] test: update assert tabs not loading --- tests/fixtures/explore.ts | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/fixtures/explore.ts b/tests/fixtures/explore.ts index c2dc74e3e..1d888ef74 100644 --- a/tests/fixtures/explore.ts +++ b/tests/fixtures/explore.ts @@ -82,12 +82,20 @@ export class ExplorePage { } async assertTabsNotLoading() { - const tabSelector = this.page.getByTestId(testIds.exploreServiceDetails.tabLogs); - const tabsLoadingSelector = tabSelector.filter({ has: this.page.locator('svg') }); - //Assert we can see the tabs - await expect(tabSelector).toHaveCount(1); - // Assert that the loading svg is not present - await expect(tabsLoadingSelector).toHaveCount(0); + const tabSelectors = [ + this.page.getByTestId(testIds.exploreServiceDetails.tabLogs), + this.page.getByTestId(testIds.exploreServiceDetails.tabPatterns), + this.page.getByTestId(testIds.exploreServiceDetails.tabLabels), + this.page.getByTestId(testIds.exploreServiceDetails.tabFields), + ]; + for (let loc of tabSelectors) { + const tabsLoadingSelector = loc.filter({ has: this.page.locator('svg') }); + + //Assert we can see the tabs + await expect(loc).toHaveCount(1); + // Assert that the loading svg is not present + await expect(tabsLoadingSelector).toHaveCount(0); + } } async click(locator: Locator) { From 694fa755c4b71ae4078d30f9f63f4556d0e0fb5b Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 15:28:34 -0500 Subject: [PATCH 05/15] test: assert label count matches # of panels, switching services updates labels --- generator/generator.go | 10 ++++---- src/Components/ServiceScene/ServiceScene.tsx | 7 ++++-- src/services/filters.ts | 1 + tests/exploreServicesBreakDown.spec.ts | 24 +++++++++++++++++++- tests/fixtures/explore.ts | 6 +++++ 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/generator/generator.go b/generator/generator.go index e4346ac03..c2bba7862 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -206,11 +206,13 @@ var mimirPod = func(ctx context.Context, logger *AppLogger) { } func startFailingMimirPod(ctx context.Context, logger Logger) { + podSequence := model.LabelValue(randSeq(5)) appLogger := NewAppLogger(model.LabelSet{ - "cluster": model.LabelValue(clusters[0]), - "namespace": model.LabelValue("mimir"), - "service_name": "mimir-ingester", - "pod": "mimir-ingester" + "-" + model.LabelValue(randSeq(5)), + "cluster": model.LabelValue(clusters[0]), + "namespace": model.LabelValue("mimir"), + "service_name": "mimir-ingester", + "pod": "mimir-ingester" + "-" + podSequence, + "pod_template_hash": podSequence, }, logger) go func() { diff --git a/src/Components/ServiceScene/ServiceScene.tsx b/src/Components/ServiceScene/ServiceScene.tsx index 99c858c32..f5d4d2c21 100644 --- a/src/Components/ServiceScene/ServiceScene.tsx +++ b/src/Components/ServiceScene/ServiceScene.tsx @@ -39,6 +39,7 @@ import { navigateToIndex } from '../../services/navigate'; import { areArraysEqual } from '../../services/comparison'; import { ActionBarScene } from './ActionBarScene'; import { breakdownViewsDefinitions, TabNames, valueBreakdownViews } from './BreakdownViews'; +import { LABELS_TO_REMOVE } from '../../services/filters'; const LOGS_PANEL_QUERY_REFID = 'logsPanelQuery'; const PATTERNS_QUERY_REFID = 'patterns'; @@ -294,9 +295,11 @@ export class ServiceScene extends SceneObjectBase { // Detected labels API call always returns a single frame, with a field for each label const detectedLabelsFields = detectedLabelsResponse.series[0].fields; if (detectedLabelsResponse.series.length !== undefined && detectedLabelsFields.length !== undefined) { + const removeSpecialFields = detectedLabelsResponse.series[0].fields.filter( + (f) => !LABELS_TO_REMOVE.includes(f.name) + ); this.setState({ - // Make sure to add one extra for the detected_level - labelsCount: detectedLabelsFields.length + 1, + labelsCount: removeSpecialFields.length + 1, // Add one for detected_level }); getMetadataService().setLabelsCount(detectedLabelsFields.length); } diff --git a/src/services/filters.ts b/src/services/filters.ts index 687e4c0d6..217756e26 100644 --- a/src/services/filters.ts +++ b/src/services/filters.ts @@ -46,6 +46,7 @@ export function getLabelOptions(labels: string[]) { return [{ label: 'All', value: ALL_VARIABLE_VALUE }, ...labelOptions]; } export const FIELDS_TO_REMOVE = ['level_extracted', LEVEL_VARIABLE_VALUE, 'level']; +export const LABELS_TO_REMOVE = [LEVEL_VARIABLE_VALUE, 'level']; export function getFieldOptions(labels: string[]) { const options = [...labels]; diff --git a/tests/exploreServicesBreakDown.spec.ts b/tests/exploreServicesBreakDown.spec.ts index da5fc11c9..f7e53d226 100644 --- a/tests/exploreServicesBreakDown.spec.ts +++ b/tests/exploreServicesBreakDown.spec.ts @@ -3,7 +3,6 @@ import { ExplorePage, PlaywrightRequest } from './fixtures/explore'; import { testIds } from '../src/services/testIds'; import { mockEmptyQueryApiResponse } from './mocks/mockEmptyQueryApiResponse'; import { LokiQuery } from '../src/services/query'; -import exp = require('node:constants'); const fieldName = 'caller'; const levelName = 'detected_level'; @@ -734,4 +733,27 @@ test.describe('explore services breakdown page', () => { // But version should exist await expect(versionPanelLocator).toHaveCount(1); }); + + test('should update label set if detected_labels is loaded in another tab', async ({ page }) => { + explorePage.blockAllQueriesExcept({}); + await explorePage.assertNotLoading(); + await explorePage.assertTabsNotLoading(); + await explorePage.goToLabelsTab(); + + const tabCountLocator = page.getByTestId(testIds.exploreServiceDetails.tabLabels).locator('> span'); + await expect(tabCountLocator).not.toBeEmpty(); + const panels = explorePage.getAllPanelsLocator(); + // Count panels, compare to tab count + await expect(panels).toHaveCount(parseInt((await tabCountLocator.textContent()) as string, 10)); + + await explorePage.assertTabsNotLoading(); + await explorePage.goToLogsTab(); + await page.getByTestId('AdHocFilter-service_name').click(); + await page.getByText('mimir-ingester').click(); + await explorePage.assertTabsNotLoading(); + await explorePage.goToLabelsTab(); + + // Count panels, compare to tab count + await expect(panels).toHaveCount(parseInt((await tabCountLocator.textContent()) as string, 10)); + }); }); diff --git a/tests/fixtures/explore.ts b/tests/fixtures/explore.ts index 1d888ef74..ac9fa3e76 100644 --- a/tests/fixtures/explore.ts +++ b/tests/fixtures/explore.ts @@ -51,6 +51,12 @@ export class ExplorePage { await main.evaluate((main) => main.scrollTo(0, main.scrollHeight)); } + async goToLogsTab() { + await this.page.getByTestId(testIds.exploreServiceDetails.tabLogs).click(); + await this.assertNotLoading(); + await this.assertTabsNotLoading(); + } + async goToFieldsTab() { await this.page.getByTestId(testIds.exploreServiceDetails.tabFields).click(); await this.assertNotLoading(); From 94ed3f67cfafa5b9a74bbe5064f009ed20613deb Mon Sep 17 00:00:00 2001 From: Galen Date: Wed, 11 Sep 2024 15:42:25 -0500 Subject: [PATCH 06/15] test: fix flake caused by using datetime which would hit max series limit --- generator/flog/log.go | 3 ++- tests/exploreServicesBreakDown.spec.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/generator/flog/log.go b/generator/flog/log.go index 4ba76605f..ff9729ae7 100644 --- a/generator/flog/log.go +++ b/generator/flog/log.go @@ -23,7 +23,7 @@ const ( // CommonLogFormat : {host} {user-identifier} {auth-user-id} [{datetime}] "{method} {request} {protocol}" {response-code} {bytes} CommonLogFormat = "%s - %s [%s] \"%s %s %s\" %d %d" // JSONLogFormat : {"host": "{host}", "user-identifier": "{user-identifier}", "datetime": "{datetime}", "method": "{method}", "request": "{request}", "protocol": "{protocol}", "status", {status}, "bytes": {bytes}, "referer": "{referer}"} - JSONLogFormat = `{"host":"%s", "user-identifier":"%s", "datetime":"%s", "method": "%s", "request": "%s", "protocol":"%s", "status":%d, "bytes":%d, "referer": "%s"}` + JSONLogFormat = `{"host":"%s", "user-identifier":"%s", "datetime":"%s", "method": "%s", "request": "%s", "protocol":"%s", "status":%d, "bytes":%d, "referer": "%s", "_25values": "%d"}` ) // NewApacheCommonLog creates a log string with apache common log format @@ -142,5 +142,6 @@ func NewJSONLogFormat(t time.Time, URI string, statusCode int) string { statusCode, gofakeit.Number(0, 30000), gofakeit.URL(), + gofakeit.Number(0, 25), ) } diff --git a/tests/exploreServicesBreakDown.spec.ts b/tests/exploreServicesBreakDown.spec.ts index f7e53d226..e4a1cdbee 100644 --- a/tests/exploreServicesBreakDown.spec.ts +++ b/tests/exploreServicesBreakDown.spec.ts @@ -671,7 +671,7 @@ test.describe('explore services breakdown page', () => { test('should see too many series button', async ({ page }) => { explorePage.blockAllQueriesExcept({ - refIds: ['logsPanelQuery', 'datetime'], + refIds: ['logsPanelQuery', '_25values'], legendFormats: [`{{${levelName}}}`], }); await page.goto( From 5c4405ecaf17c1665fddd9c98b070e9d1fbd76f5 Mon Sep 17 00:00:00 2001 From: Galen Date: Thu, 12 Sep 2024 07:54:41 -0500 Subject: [PATCH 07/15] test: update tests to work with new label, refactor --- generator/flog/log.go | 4 ++-- tests/exploreServicesBreakDown.spec.ts | 17 ----------------- tests/exploreServicesJsonBreakDown.spec.ts | 14 ++++++++++++++ tests/exploreServicesJsonMixedBreakDown.spec.ts | 8 ++++---- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/generator/flog/log.go b/generator/flog/log.go index ff9729ae7..eb1048609 100644 --- a/generator/flog/log.go +++ b/generator/flog/log.go @@ -22,8 +22,8 @@ const ( RFC5424Log = "<%d>%d %s %s %s %d ID%d %s %s" // CommonLogFormat : {host} {user-identifier} {auth-user-id} [{datetime}] "{method} {request} {protocol}" {response-code} {bytes} CommonLogFormat = "%s - %s [%s] \"%s %s %s\" %d %d" - // JSONLogFormat : {"host": "{host}", "user-identifier": "{user-identifier}", "datetime": "{datetime}", "method": "{method}", "request": "{request}", "protocol": "{protocol}", "status", {status}, "bytes": {bytes}, "referer": "{referer}"} - JSONLogFormat = `{"host":"%s", "user-identifier":"%s", "datetime":"%s", "method": "%s", "request": "%s", "protocol":"%s", "status":%d, "bytes":%d, "referer": "%s", "_25values": "%d"}` + // JSONLogFormat : {"host": "{host}", "user-identifier": "{user-identifier}", "datetime": "{datetime}", "method": "{method}", "request": "{request}", "protocol": "{protocol}", "status", {status}, "bytes": {bytes}, "referer": "{referer}", "_25values": "{_25values}" + JSONLogFormat = `{"host":"%s", "user-identifier":"%s", "datetime":"%s", "method": "%s", "request": "%s", "protocol":"%s", "status":%d, "bytes":%d, "referer": "%s", "_25values": %d}` ) // NewApacheCommonLog creates a log string with apache common log format diff --git a/tests/exploreServicesBreakDown.spec.ts b/tests/exploreServicesBreakDown.spec.ts index e4a1cdbee..12f30d47e 100644 --- a/tests/exploreServicesBreakDown.spec.ts +++ b/tests/exploreServicesBreakDown.spec.ts @@ -669,23 +669,6 @@ test.describe('explore services breakdown page', () => { await expect(explorePage.getAllPanelsLocator().first()).toBeInViewport(); }); - test('should see too many series button', async ({ page }) => { - explorePage.blockAllQueriesExcept({ - refIds: ['logsPanelQuery', '_25values'], - legendFormats: [`{{${levelName}}}`], - }); - await page.goto( - '/a/grafana-lokiexplore-app/explore/service/nginx-json/fields?var-ds=gdev-loki&from=now-5m&to=now&patterns=%5B%5D&var-fields=&var-levels=&var-patterns=&var-lineFilter=&var-filters=service_name%7C%3D%7Cnginx-json&urlColumns=%5B%5D&visualizationType=%22logs%22&displayedFields=%5B%5D&var-fieldBy=$__all' - ); - const showAllButtonLocator = page.getByText('Show all'); - await expect(showAllButtonLocator).toHaveCount(1); - await expect(showAllButtonLocator).toBeVisible(); - - await showAllButtonLocator.click(); - - await expect(showAllButtonLocator).toHaveCount(0); - }); - test('should not see maximum of series limit reached after changing filters', async ({ page }) => { explorePage.blockAllQueriesExcept({ refIds: ['logsPanelQuery', 'content', 'version'], diff --git a/tests/exploreServicesJsonBreakDown.spec.ts b/tests/exploreServicesJsonBreakDown.spec.ts index 288d7e255..beba398f2 100644 --- a/tests/exploreServicesJsonBreakDown.spec.ts +++ b/tests/exploreServicesJsonBreakDown.spec.ts @@ -56,4 +56,18 @@ test.describe('explore nginx-json breakdown pages ', () => { }); expect(requests).toHaveLength(3); }); + + test('should see too many series button', async ({ page }) => { + explorePage.blockAllQueriesExcept({ + refIds: ['logsPanelQuery', '_25values'], + }); + await explorePage.goToFieldsTab(); + const showAllButtonLocator = page.getByText('Show all'); + await expect(showAllButtonLocator).toHaveCount(1); + await expect(showAllButtonLocator).toBeVisible(); + + await showAllButtonLocator.click(); + + await expect(showAllButtonLocator).toHaveCount(0); + }); }); diff --git a/tests/exploreServicesJsonMixedBreakDown.spec.ts b/tests/exploreServicesJsonMixedBreakDown.spec.ts index bad85da58..b1fac30c2 100644 --- a/tests/exploreServicesJsonMixedBreakDown.spec.ts +++ b/tests/exploreServicesJsonMixedBreakDown.spec.ts @@ -74,8 +74,8 @@ test.describe('explore nginx-json-mixed breakdown pages ', () => { await explorePage.goToFieldsTab(); const allPanels = explorePage.getAllPanelsLocator(); - // Should be 13 fields coming back from the detected_fields, but one is detected_level - await expect(allPanels).toHaveCount(12); + // Should be 14 fields coming back from the detected_fields, but one is detected_level + await expect(allPanels).toHaveCount(13); await page .getByTestId(`data-testid Panel header ${logFmtFieldName}`) @@ -89,8 +89,8 @@ test.describe('explore nginx-json-mixed breakdown pages ', () => { // Exclude a panel await page.getByRole('button', { name: 'Exclude' }).nth(0).click(); // There is only one panel/value, so we should be redirected back to the aggregation after excluding it - // We'll have all 9 responses from detected_fields - await expect(allPanels).toHaveCount(9); + // We'll have all 10 responses from detected_fields + await expect(allPanels).toHaveCount(10); // Adhoc content filter should be added await expect( From 36059572fd4eb6a37268a0a8a4f01e73304995d3 Mon Sep 17 00:00:00 2001 From: Galen Date: Thu, 12 Sep 2024 08:46:35 -0500 Subject: [PATCH 08/15] chore: remove platform causing docker images to fail locally --- docker-compose.dev.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/docker-compose.dev.yaml b/docker-compose.dev.yaml index f2fc9468f..24cb212d3 100644 --- a/docker-compose.dev.yaml +++ b/docker-compose.dev.yaml @@ -3,7 +3,6 @@ version: '3.0' services: grafana: container_name: 'grafana-logsapp' - platform: 'linux/amd64' environment: - GF_FEATURE_TOGGLES_ENABLE=accessControlOnCall lokiLogsDataplane build: From c11dadaf56793239d6e1d4329b2ffd1f65622eb3 Mon Sep 17 00:00:00 2001 From: Galen Date: Thu, 12 Sep 2024 09:33:22 -0500 Subject: [PATCH 09/15] test: increase liklihood of logfmt line in mixed service to prevent flake --- generator/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generator/generator.go b/generator/generator.go index c2bba7862..4616ec511 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -97,7 +97,7 @@ var generators = map[model.LabelValue]map[model.LabelValue]LogGenerator{ for ctx.Err() == nil { level := randLevel() t := time.Now() - if rand.Intn(10)%2 == 0 && level == ERROR { + if level == ERROR { log := flog.NewCommonLogFormat(t, randURI(), statusFromLevel(level)) // Add a stacktrace to the logfmt log, and include a field that will conflict with stream selectors logger.Log(level, t, fmt.Sprintf("%s %s", log, `method=GET namespace=whoopsie caller=flush.go:253 stacktrace="Exception in thread \"main\" java.lang.NullPointerException\n at com.example.myproject.Book.getTitle(Book.java:16)\n at com.example.myproject.Author.getBookTitles(Author.java:25)\n at com.example.myproject.Bootstrap.main(Bootstrap.java:14)"`)) From 9e0316e49a2ecf346e56b8a78896f6ffa72236bd Mon Sep 17 00:00:00 2001 From: Galen Date: Fri, 13 Sep 2024 12:07:30 -0500 Subject: [PATCH 10/15] chore: hide service_name, allow level to show --- src/Components/ServiceScene/ServiceScene.tsx | 5 +++-- src/services/datasource.ts | 6 +++--- src/services/filters.ts | 8 ++------ 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Components/ServiceScene/ServiceScene.tsx b/src/Components/ServiceScene/ServiceScene.tsx index f5d4d2c21..28763ecdd 100644 --- a/src/Components/ServiceScene/ServiceScene.tsx +++ b/src/Components/ServiceScene/ServiceScene.tsx @@ -25,6 +25,7 @@ import { getLabelsVariable, getLevelsVariable, getServiceNameFromVariableState, + LEVEL_VARIABLE_VALUE, LOG_STREAM_SELECTOR_EXPR, SERVICE_NAME, VAR_DATASOURCE, @@ -39,7 +40,6 @@ import { navigateToIndex } from '../../services/navigate'; import { areArraysEqual } from '../../services/comparison'; import { ActionBarScene } from './ActionBarScene'; import { breakdownViewsDefinitions, TabNames, valueBreakdownViews } from './BreakdownViews'; -import { LABELS_TO_REMOVE } from '../../services/filters'; const LOGS_PANEL_QUERY_REFID = 'logsPanelQuery'; const PATTERNS_QUERY_REFID = 'patterns'; @@ -296,8 +296,9 @@ export class ServiceScene extends SceneObjectBase { const detectedLabelsFields = detectedLabelsResponse.series[0].fields; if (detectedLabelsResponse.series.length !== undefined && detectedLabelsFields.length !== undefined) { const removeSpecialFields = detectedLabelsResponse.series[0].fields.filter( - (f) => !LABELS_TO_REMOVE.includes(f.name) + (f) => LEVEL_VARIABLE_VALUE !== f.name ); + this.setState({ labelsCount: removeSpecialFields.length + 1, // Add one for detected_level }); diff --git a/src/services/datasource.ts b/src/services/datasource.ts index 7309b7b09..f23ec178c 100644 --- a/src/services/datasource.ts +++ b/src/services/datasource.ts @@ -16,8 +16,8 @@ import { getDataSource } from './scenes'; import { LokiQuery } from './query'; import { PLUGIN_ID } from './routing'; import { DetectedFieldsResponse, DetectedLabelsResponse } from './fields'; -import { FIELDS_TO_REMOVE, sortLabelsByCardinality } from './filters'; -import { LEVEL_VARIABLE_VALUE, SERVICE_NAME } from './variables'; +import { FIELDS_TO_REMOVE, LABELS_TO_REMOVE, sortLabelsByCardinality } from './filters'; +import { SERVICE_NAME } from './variables'; import { runShardSplitQuery } from './shardQuerySplitting'; import { requestSupportsSharding } from './logql'; @@ -287,7 +287,7 @@ export class WrappedLokiDatasource extends RuntimeDataSource { ); const labels = response.detectedLabels ?.sort((a, b) => sortLabelsByCardinality(a, b)) - ?.filter((label) => label.label !== LEVEL_VARIABLE_VALUE); + ?.filter((label) => !LABELS_TO_REMOVE.includes(label.label)); const detectedLabelFields: Array> = labels?.map((label) => { return { diff --git a/src/services/filters.ts b/src/services/filters.ts index 217756e26..d98f29c47 100644 --- a/src/services/filters.ts +++ b/src/services/filters.ts @@ -1,5 +1,5 @@ import { DetectedLabel, LabelType } from './fields'; -import { ALL_VARIABLE_VALUE, LEVEL_VARIABLE_VALUE } from './variables'; +import { ALL_VARIABLE_VALUE, LEVEL_VARIABLE_VALUE, SERVICE_NAME } from './variables'; import { VariableValueOption } from '@grafana/scenes'; export enum FilterOp { @@ -33,10 +33,6 @@ export function getLabelOptions(labels: string[]) { if (!labels.includes(LEVEL_VARIABLE_VALUE)) { options.unshift(LEVEL_VARIABLE_VALUE); } - const labelsIndex = options.indexOf('level'); - if (labelsIndex !== -1) { - options.splice(labelsIndex, 1); - } const labelOptions: VariableValueOption[] = options.map((label) => ({ label, @@ -46,7 +42,7 @@ export function getLabelOptions(labels: string[]) { return [{ label: 'All', value: ALL_VARIABLE_VALUE }, ...labelOptions]; } export const FIELDS_TO_REMOVE = ['level_extracted', LEVEL_VARIABLE_VALUE, 'level']; -export const LABELS_TO_REMOVE = [LEVEL_VARIABLE_VALUE, 'level']; +export const LABELS_TO_REMOVE = [SERVICE_NAME]; export function getFieldOptions(labels: string[]) { const options = [...labels]; From 7697b427d94f98ed84b818fc04db76d002ad6f16 Mon Sep 17 00:00:00 2001 From: Galen Date: Fri, 13 Sep 2024 12:36:06 -0500 Subject: [PATCH 11/15] chore: clean up --- src/services/datasource.ts | 4 ++-- src/services/filters.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/services/datasource.ts b/src/services/datasource.ts index f23ec178c..81ddb61a1 100644 --- a/src/services/datasource.ts +++ b/src/services/datasource.ts @@ -286,8 +286,8 @@ export class WrappedLokiDatasource extends RuntimeDataSource { } ); const labels = response.detectedLabels - ?.sort((a, b) => sortLabelsByCardinality(a, b)) - ?.filter((label) => !LABELS_TO_REMOVE.includes(label.label)); + ?.filter((label) => !LABELS_TO_REMOVE.includes(label.label)) + ?.sort((a, b) => sortLabelsByCardinality(a, b)); const detectedLabelFields: Array> = labels?.map((label) => { return { diff --git a/src/services/filters.ts b/src/services/filters.ts index d98f29c47..91dfadc8a 100644 --- a/src/services/filters.ts +++ b/src/services/filters.ts @@ -41,7 +41,8 @@ export function getLabelOptions(labels: string[]) { return [{ label: 'All', value: ALL_VARIABLE_VALUE }, ...labelOptions]; } -export const FIELDS_TO_REMOVE = ['level_extracted', LEVEL_VARIABLE_VALUE, 'level']; +export const LEVEL_INDEX_NAME = 'level'; +export const FIELDS_TO_REMOVE = ['level_extracted', LEVEL_VARIABLE_VALUE, LEVEL_INDEX_NAME]; export const LABELS_TO_REMOVE = [SERVICE_NAME]; export function getFieldOptions(labels: string[]) { From 3d16ee99cdd7da2d6563be540a70ff6e6cd04781 Mon Sep 17 00:00:00 2001 From: Galen Date: Fri, 13 Sep 2024 12:43:40 -0500 Subject: [PATCH 12/15] test: fix assertions --- tests/exploreServicesJsonMixedBreakDown.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/exploreServicesJsonMixedBreakDown.spec.ts b/tests/exploreServicesJsonMixedBreakDown.spec.ts index 0631de5f3..b32a28125 100644 --- a/tests/exploreServicesJsonMixedBreakDown.spec.ts +++ b/tests/exploreServicesJsonMixedBreakDown.spec.ts @@ -75,7 +75,7 @@ test.describe('explore nginx-json-mixed breakdown pages ', () => { const allPanels = explorePage.getAllPanelsLocator(); // Should be 16 fields coming back from the detected_fields, but one is detected_level - await expect(allPanels).toHaveCount(15); + await expect(allPanels).toHaveCount(16); await page .getByTestId(`data-testid Panel header ${logFmtFieldName}`) @@ -90,7 +90,7 @@ test.describe('explore nginx-json-mixed breakdown pages ', () => { await page.getByRole('button', { name: 'Exclude' }).nth(0).click(); // There is only one panel/value, so we should be redirected back to the aggregation after excluding it // We'll have all 12 responses from detected_fields - await expect(allPanels).toHaveCount(12); + await expect(allPanels).toHaveCount(13); // Adhoc content filter should be added await expect( From 3a5f965fbca3959e8cbea87e3eee69306ae7f849 Mon Sep 17 00:00:00 2001 From: Galen Kistler <109082771+gtk-grafana@users.noreply.github.com> Date: Fri, 13 Sep 2024 12:44:13 -0500 Subject: [PATCH 13/15] Update generator/flog/log.go Co-authored-by: Sven Grossmann --- generator/flog/log.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generator/flog/log.go b/generator/flog/log.go index eb1048609..8bce5e2c7 100644 --- a/generator/flog/log.go +++ b/generator/flog/log.go @@ -22,7 +22,7 @@ const ( RFC5424Log = "<%d>%d %s %s %s %d ID%d %s %s" // CommonLogFormat : {host} {user-identifier} {auth-user-id} [{datetime}] "{method} {request} {protocol}" {response-code} {bytes} CommonLogFormat = "%s - %s [%s] \"%s %s %s\" %d %d" - // JSONLogFormat : {"host": "{host}", "user-identifier": "{user-identifier}", "datetime": "{datetime}", "method": "{method}", "request": "{request}", "protocol": "{protocol}", "status", {status}, "bytes": {bytes}, "referer": "{referer}", "_25values": "{_25values}" + // JSONLogFormat : {"host": "{host}", "user-identifier": "{user-identifier}", "datetime": "{datetime}", "method": "{method}", "request": "{request}", "protocol": "{protocol}", "status": {status}, "bytes": {bytes}, "referer": "{referer}", "_25values": "{_25values}" JSONLogFormat = `{"host":"%s", "user-identifier":"%s", "datetime":"%s", "method": "%s", "request": "%s", "protocol":"%s", "status":%d, "bytes":%d, "referer": "%s", "_25values": %d}` ) From 9dcc301054e9cc424a8c14ae189e69b60b540e54 Mon Sep 17 00:00:00 2001 From: Galen Date: Fri, 13 Sep 2024 12:51:37 -0500 Subject: [PATCH 14/15] chore: remove pod --- generator/generator.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/generator/generator.go b/generator/generator.go index dec59ab2e..e18fb551c 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -219,13 +219,10 @@ var mimirPod = func(ctx context.Context, logger *AppLogger, metadata push.Labels } func startFailingMimirPod(ctx context.Context, logger Logger) { - podSequence := model.LabelValue(randSeq(5)) appLogger := NewAppLogger(model.LabelSet{ - "cluster": model.LabelValue(clusters[0]), - "namespace": model.LabelValue("mimir"), - "service_name": "mimir-ingester", - "pod": "mimir-ingester" + "-" + podSequence, - "pod_template_hash": podSequence, + "cluster": model.LabelValue(clusters[0]), + "namespace": model.LabelValue("mimir"), + "service_name": "mimir-ingester", }, logger) go func() { From f0325f4c84c957386f914d751443110a24816951 Mon Sep 17 00:00:00 2001 From: Galen Date: Fri, 13 Sep 2024 12:53:51 -0500 Subject: [PATCH 15/15] test: remove no longer applicable test --- src/services/filters.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/services/filters.test.ts b/src/services/filters.test.ts index 5d62db727..0fe4afa7a 100644 --- a/src/services/filters.test.ts +++ b/src/services/filters.test.ts @@ -81,17 +81,6 @@ describe('getLabelOptions', () => { expect(getLabelOptions(labels)).toEqual(expectedOptions); }); - it('should remove level if it is present in the list', () => { - const labels = [LEVEL_VARIABLE_VALUE, 'level', 'Label A']; - const expectedOptions: Array> = [ - { label: 'All', value: ALL_VARIABLE_VALUE }, - { label: LEVEL_VARIABLE_VALUE, value: LEVEL_VARIABLE_VALUE }, - { label: 'Label A', value: 'Label A' }, - ]; - - expect(getLabelOptions(labels)).toEqual(expectedOptions); - }); - it('should always add the All option at the beginning', () => { const labels = ['Label A', 'Label B']; const expectedOptions: Array> = [