[metricstore] Remove min step API from metricstore by mahadzaryab1 · Pull Request #8425 · jaegertracing/jaeger

mahadzaryab1 · 2026-04-23T01:40:01Z

Which problem is this PR solving?

Towards [clickhouse] Native Service Performance Monitoring (SPM) Support #8313
Addresses [clickhouse][spm] Implement GetMinStepDuration for ClickHouse Storage #8421 (comment)

Description of the changes

Removes the GetMinStepDuration function and all its implementations as it is not used by the UI today.

How was this change tested?

CI

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/main/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: make lint test

AI Usage in this PR (choose one)

See AI Usage Policy.

None: No AI tools were used in creating this PR
Light: AI provided minor assistance (formatting, simple suggestions)
Moderate: AI helped with code generation or debugging specific parts
Heavy: AI generated most or all of the code changes

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot

Pull request overview

Removes the GetMinStepDuration metricstore API surface and its backend implementations, since it’s no longer used by the UI.

Changes:

Dropped GetMinStepDuration from metricstore.Reader and removed MinStepDurationQueryParameters.
Removed GetMinStepDuration implementations/tests from ClickHouse v2, Prometheus, Elasticsearch, and disabled metricstores.
Removed metrics instrumentation and related mocks for GetMinStepDuration.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/storage/v1/api/metricstore/interface.go	Removes `GetMinStepDuration` from the Reader interface and deletes its query params type.
internal/storage/v1/api/metricstore/mocks/mocks.go	Updates generated mocks to match the new Reader interface (drops min-step method).
internal/storage/v1/api/metricstore/metricstoremetrics/decorator.go	Removes decorator instrumentation for the min-step operation.
internal/storage/v1/api/metricstore/metricstoremetrics/decorator_test.go	Adjusts tests for decorator behavior (but still needs updates to expected counters).
internal/storage/v2/clickhouse/metricstore/reader.go	Removes ClickHouse v2 stub implementation for `GetMinStepDuration`.
internal/storage/v2/clickhouse/metricstore/reader_test.go	Removes ClickHouse v2 tests for `GetMinStepDuration`.
internal/storage/metricstore/prometheus/metricstore/reader.go	Removes Prometheus min-step constant and method implementation.
internal/storage/metricstore/prometheus/metricstore/reader_test.go	Removes Prometheus `GetMinStepDuration` test.
internal/storage/metricstore/elasticsearch/reader.go	Removes Elasticsearch min-step constant and method implementation.
internal/storage/metricstore/elasticsearch/reader_test.go	Removes Elasticsearch `GetMinStepDuration` test.
internal/storage/metricstore/disabled/reader.go	Removes disabled-reader `GetMinStepDuration` implementation.
internal/storage/metricstore/disabled/reader_test.go	Removes disabled-reader `GetMinStepDuration` test.
cmd/jaeger/internal/extension/jaegerquery/internal/http_handler.go	Removes the min-step handler implementation (but route registration still references it).
cmd/jaeger/internal/extension/jaegerquery/internal/http_handler_test.go	Removes min-step endpoint test (but another test still calls `/api/metrics/minstep`).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T01:44:13Z

 func (aH *APIHandler) metrics(w http.ResponseWriter, r *http.Request, getMetrics func(context.Context, metricstore.BaseQueryParameters) (*metrics.MetricFamily, error)) {
 	requestParams, err := aH.queryParser.parseMetricsQueryParams(r)
 	if aH.handleError(w, err, http.StatusBadRequest) {


minStep handler was removed, but RegisterRoutes still registers /metrics/minstep via aH.minStep, which will not compile. Remove the route registration (and any related endpoint expectations) to fully drop the API, or reintroduce the handler if the endpoint should remain supported.

Copilot · 2026-04-23T01:44:14Z

 // getJSON fetches a JSON document from a server via HTTP GET
 func getJSON(url string, out any) error {
 	return getJSONCustomHeaders(url, make(map[string]string), out)


TestMetricsQueryDisabled still calls /api/metrics/minstep and expects the "metrics querying is currently disabled" error. Since the minstep endpoint is being removed, this test case should be deleted or updated to assert the new expected behavior (e.g., 404 / route not found) after the route is removed.

Copilot · 2026-04-23T01:44:14Z

 	counters, gauges := mf.Snapshot()
 	wantCounts := map[string]int64{
 		"requests|operation=get_latencies|result=ok":          1,


The expected counters in wantCounts still include operation=get_min_step_duration, but GetMinStepDuration instrumentation was removed from the decorator. Update the expected counter maps in this test (and the failing-calls variant below) to remove get_min_step_duration entries.

Copilot · 2026-04-23T01:44:14Z

 	counters, gauges := mf.Snapshot()
 	wantCounts := map[string]int64{
 		"requests|operation=get_latencies|result=ok":          0,


This test’s wantCounts map still expects operation=get_min_step_duration counters, but the decorator no longer emits them after removing GetMinStepDuration. Remove those expected entries so the assertions reflect the updated decorator.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

codecov · 2026-04-23T01:58:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.09%. Comparing base (f9333aa) to head (7c7e0d5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8425      +/-   ##
==========================================
- Coverage   96.10%   96.09%   -0.01%     
==========================================
  Files         323      323              
  Lines       20643    20619      -24     
==========================================
- Hits        19838    19814      -24     
  Misses        649      649              
  Partials      156      156

Flag	Coverage Δ
badger_direct	`9.22% <0.00%> (+<0.01%)`	⬆️
badger_e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
cassandra-4.x-direct-manual	`12.57% <0.00%> (+<0.01%)`	⬆️
cassandra-4.x-e2e-auto	`0.98% <0.00%> (+<0.01%)`	⬆️
cassandra-4.x-e2e-manual	`0.98% <0.00%> (+<0.01%)`	⬆️
cassandra-5.x-direct-manual	`12.57% <0.00%> (+<0.01%)`	⬆️
cassandra-5.x-e2e-auto	`0.98% <0.00%> (+<0.01%)`	⬆️
cassandra-5.x-e2e-manual	`0.98% <0.00%> (+<0.01%)`	⬆️
clickhouse-direct	`8.47% <0.00%> (+<0.01%)`	⬆️
clickhouse-e2e	`1.12% <0.00%> (+<0.01%)`	⬆️
elasticsearch-6.x-direct	`16.86% <0.00%> (+0.01%)`	⬆️
elasticsearch-7.x-direct	`16.91% <0.00%> (+0.01%)`	⬆️
elasticsearch-8.x-direct	`17.06% <0.00%> (+0.01%)`	⬆️
elasticsearch-8.x-e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
elasticsearch-9.x-e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
grpc_direct	`7.56% <0.00%> (+<0.01%)`	⬆️
grpc_e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
kafka-3.x-v2	`0.99% <0.00%> (-0.09%)`	⬇️
memory_v2	`0.99% <0.00%> (+<0.01%)`	⬆️
opensearch-1.x-direct	`16.95% <0.00%> (+0.01%)`	⬆️
opensearch-2.x-direct	`16.95% <0.00%> (+0.01%)`	⬆️
opensearch-2.x-e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
opensearch-3.x-e2e	`0.99% <0.00%> (+<0.01%)`	⬆️
query	`0.99% <0.00%> (+<0.01%)`	⬆️
tailsampling-processor	`0.47% <0.00%> (+<0.01%)`	⬆️
unittests	`94.54% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T02:05:43Z

@@ -41,11 +40,10 @@ func (q *queryMetrics) emit(err error, latency time.Duration) {
 // NewReadMetricsDecorator returns a new ReadMetricsDecorator.


The doc comment says NewReadMetricsDecorator but the function is named NewReaderDecorator. Please update the comment to match the actual function name (or rename the function) so GoDoc and search results are accurate.

Suggested change

// NewReadMetricsDecorator returns a new ReadMetricsDecorator.

// NewReaderDecorator returns a new ReadMetricsDecorator.

Copilot · 2026-04-23T02:05:43Z

 	aH.handleFunc(router, aH.latencies, http.MethodGet, "/metrics/latencies")
 	aH.handleFunc(router, aH.calls, http.MethodGet, "/metrics/calls")
 	aH.handleFunc(router, aH.errors, http.MethodGet, "/metrics/errors")
-	aH.handleFunc(router, aH.minStep, http.MethodGet, "/metrics/minstep")
 	aH.handleFunc(router, aH.getQualityMetrics, http.MethodGet, "/quality-metrics")


This PR removes the /api/metrics/minstep endpoint, but docker-compose/monitor/README.md still documents and demonstrates it. Please update/remove that documentation (or add a note about removal) to avoid broken examples for users.

yurishkuro

we also need to remove it from IDL

mahadzaryab1 added 3 commits April 22, 2026 18:30

Remove GetCallRates From Interface

c0e6d65

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Regenerate Mocks

f486ad9

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Update Callsites

f0ef61c

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot AI review requested due to automatic review settings April 23, 2026 01:40

mahadzaryab1 added the changelog:breaking-change Change that is breaking public APIs or established behavior label Apr 23, 2026

mahadzaryab1 requested a review from a team as a code owner April 23, 2026 01:40

dosubot Bot added the area/storage label Apr 23, 2026

Copilot started reviewing on behalf of mahadzaryab1 April 23, 2026 01:40 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

fix

c7faed8

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Fix

7c7e0d5

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot AI review requested due to automatic review settings April 23, 2026 02:02

Copilot started reviewing on behalf of mahadzaryab1 April 23, 2026 02:03 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

yurishkuro approved these changes Apr 23, 2026

View reviewed changes

mahadzaryab1 added this pull request to the merge queue Apr 23, 2026

Merged via the queue into jaegertracing:main with commit 5b8081c Apr 23, 2026
76 checks passed

lopster568 mentioned this pull request Apr 24, 2026

[Feature]: Expose SPM metrics through MCP server #8409

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[metricstore] Remove min step API from metricstore#8425

[metricstore] Remove min step API from metricstore#8425
mahadzaryab1 merged 5 commits into
jaegertracing:mainfrom
mahadzaryab1:remove-getminstepduration

mahadzaryab1 commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

codecov Bot commented Apr 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

yurishkuro left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -41,11 +40,10 @@ func (q *queryMetrics) emit(err error, latency time.Duration) {
		// NewReadMetricsDecorator returns a new ReadMetricsDecorator.

	// NewReadMetricsDecorator returns a new ReadMetricsDecorator.
	// NewReaderDecorator returns a new ReadMetricsDecorator.

Uh oh!

Conversation

mahadzaryab1 commented Apr 23, 2026

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

AI Usage in this PR (choose one)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

yurishkuro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 23, 2026 •

edited

Loading