feat: Add tags dropdown to SPM page for filtering metrics #3065

wololowarrior · 2025-09-07T06:02:45Z

This commit adds functionality to filter metrics by tags in the Service Performance Monitoring (SPM) page.

Which problem is this PR solving?

[SPM] Filter metrics on page by tag jaeger#7433

Description of the changes

How was this change tested?

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: npm run lint and npm run test

graphite-app · 2025-09-07T06:02:52Z

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

yurishkuro

Make sure to stress test how it looks when all tag drop downs don't fit in one line

yurishkuro · 2025-09-07T15:52:48Z

packages/jaeger-ui/src/components/Monitor/MultiTagSelector/MultiTagSelector.css

@@ -0,0 +1,31 @@
+/*
+Copyright (c) 2025 The Jaeger Authors.


Use spdx id instead of full header

yurishkuro · 2025-09-07T15:53:20Z

packages/jaeger-ui/src/components/Monitor/MultiTagSelector/MultiTagSelector.jsx

+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0


Use spdx id

yurishkuro · 2025-09-07T15:55:59Z

packages/jaeger-ui/src/constants/default-config.tsx

@@ -105,6 +105,12 @@ const defaultConfig: Config = {
      },
    },
    docsLink: 'https://www.jaegertracing.io/docs/latest/spm/',
+    tagFilters: [],
+    tagAttributes: [
+      { name: 'environment', label: 'Environment' },


Non standard, cannot be default

Yes, I commited some of the testing values. Will remove

yurishkuro · 2025-09-07T15:57:27Z

packages/jaeger-ui/src/constants/default-config.tsx

+    tagAttributes: [
+      { name: 'environment', label: 'Environment' },
+      { name: 'http.status_code', label: 'HTTP Status' },
+      { name: 'http.method', label: 'HTTP Method' },


Default can only be something that will work out if the box. Including http route by default is nice, but how will it work with Prometheus if the label is not exposed?

By default, i can see these; the traces are generated using microsim. http.Method isnt there by default in prometheus, it'll require spanmetrics to be configured.

harshil.gupta@harshil jaeger % curl "http://localhost:9090/api/v1/labels" | jq % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 148 100 148 0 0 18642 0 --:--:-- --:--:-- --:--:-- 21142 { "status": "success", "data": [ "__name__", "exported_job", "instance", "job", "le", "otel_scope_name", "service_name", "span_kind", "span_name", "status_code" ] }

I can see there is a operation in prometheus by default. Maybe we can include that as default.

labels": [ { "name": "operation", "value": "/GetDriver" }, { "name": "service_name", "value": "redis" } ],

yurishkuro · 2025-09-07T15:58:33Z

packages/jaeger-ui/src/types/config.tsx

@@ -71,6 +71,18 @@ export type MonitorConfig = {
  menuEnabled?: boolean;
  emptyState?: MonitorEmptyStateConfig;
  docsLink?: string;
+  tagFilters?: readonly TagFilter[];


Add documentation to new fields and types

yurishkuro · 2025-09-07T15:59:57Z

packages/jaeger-ui/src/types/metrics.tsx

@@ -31,6 +31,7 @@ export type MetricsAPIQueryParams = {
  step: number;
  ratePer: number;
  spanKind: spanKinds;
+  tag?: string;


Add documentation

mdwyer6 · 2025-09-07T21:55:21Z

packages/jaeger-ui/src/components/Monitor/MultiTagSelector/MultiTagSelector.jsx

+  }, [valueAsObject]);
+
+  // Handle value selection for a specific attribute
+  const handleValueChange = useCallback((attributeName, selectedValue) => {


why useCallback here? the only place it is used is passing it to an unmemoized component in a prop that creates a new function every render anyway.

multiple things would need to change for there to be any perf impact

mdwyer6 · 2025-09-07T21:59:28Z

packages/jaeger-ui/src/components/Monitor/MultiTagSelector/MultiTagSelector.jsx

+        .map(([key, val]) => `${key}=${val}`)
+        .join(' ');
+
+      onChange(tagString);


react requires state updaters to be pure: see docs "Updater functions run during rendering, so updater functions must be pure and only return the result. Don’t try to set state from inside of them or run other side effects"

mdwyer6 · 2025-09-07T22:10:36Z

packages/jaeger-ui/src/components/Monitor/MultiTagSelector/MultiTagSelector.jsx

+  }, [service, tagAttributes]);
+
+  // Update selected values when value prop changes
+  useEffect(() => {


but this will clobber any local state changes you make every time the prop changes. you need to either lift all of this state up to the parent or keep state only for changes made at this level and merge local state with valueAsObject in render e..g.,

// somewhere in render const [localObjChanges, setLocalObjChanges] = useState() // don't attempt to sync state this with parent prop // takes parent prop and patches local state changes on top in pure way const selectedValues = mergeFn(valueAsObject, localObjChanges)

wololowarrior · 2025-09-08T02:40:42Z

Hey @yurishkuro @mdwyer6 , I truly appreciate you reviewing.
I've used Copilot for most of UI, since I've not worked with UI languages at all, you can let me know what changes are needed. Will try to address other points
I'll add a way to run this feature E2E.

wololowarrior · 2025-09-09T06:44:16Z

Make sure to stress test how it looks when all tag drop downs don't fit in one line

@yurishkuro Here's what the stress test looks like

This commit adds functionality to filter metrics by tags in the Service Performance Monitoring (SPM) page. Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

wololowarrior force-pushed the metrics-filtering-using-tags branch from 047cbc8 to 556d48b Compare September 7, 2025 07:08

yurishkuro reviewed Sep 7, 2025

View reviewed changes

mdwyer6 reviewed Sep 7, 2025

View reviewed changes

wololowarrior added 5 commits September 9, 2025 12:47

feat: Add tags dropdown to SPM page for filtering metrics

88b7620

This commit adds functionality to filter metrics by tags in the Service Performance Monitoring (SPM) page. Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

Update width of selectors

5741e37

Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

Fix multi tags

c12cf7b

Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

Show data not available

68e838a

Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

Address reviews

4d21f20

Signed-off-by: Harshil Gupta <harshilgupta1808@gmail.com>

wololowarrior force-pushed the metrics-filtering-using-tags branch from 92deeca to 4d21f20 Compare September 9, 2025 07:17

This was referenced Sep 9, 2025

[SPM] Filter metrics on page by tag jaegertracing/jaeger#7504

Open

[SPM] Filter metrics on page by tag jaegertracing/jaeger#7433

Open

wololowarrior marked this pull request as ready for review September 11, 2025 09:25

wololowarrior requested a review from a team as a code owner September 11, 2025 09:25

wololowarrior requested review from jkowall and removed request for a team September 11, 2025 09:25

feat: Add tags dropdown to SPM page for filtering metrics #3065

Are you sure you want to change the base?

feat: Add tags dropdown to SPM page for filtering metrics #3065

Uh oh!

Conversation

wololowarrior commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

graphite-app bot commented Sep 7, 2025

How to use the Graphite Merge Queue

Uh oh!

yurishkuro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wololowarrior Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wololowarrior Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wololowarrior commented Sep 8, 2025

Uh oh!

wololowarrior commented Sep 9, 2025

Uh oh!

Uh oh!

wololowarrior commented Sep 7, 2025 •

edited

Loading

wololowarrior Sep 8, 2025 •

edited

Loading

wololowarrior Sep 8, 2025 •

edited

Loading