Initial query trace metrics for traces view by serena-ruan · Pull Request #19191 · mlflow/mlflow

serena-ruan · 2025-12-03T09:52:35Z

🛠 DevTools 🛠

Install mlflow from this PR

# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/19191/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/19191/merge#subdirectory=libs/skinny

For Databricks, use the following command:

%sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/19191/merge

Related Issues/PRs

#xxx

What changes are proposed in this pull request?

Initial query_trace_metrics implementation. This PR supports below:

Trace view type
Metrics: Count + Trace & AVG / Percentiles for Trace Latency
Time granularity
Group by options for above metrics

Refactored test fixtures a bit so it can be reused.
Added tests for above metrics.

How is this PR tested?

Existing unit/integration tests
New unit/integration tests
Manual tests

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

How should the PR be classified in the release notes? Choose one:

rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

Yes should be selected for bug fixes, documentation updates, and other small changes. No should be selected for new features and larger changes. If you're unsure about the release classification of this PR, leave this unchecked to let the maintainers decide.

What is a minor/patch release?

Minor release: a release that increments the second part of the version number (e.g., 1.2.0 -> 1.3.0).
Bug fixes, doc updates and new features usually go into minor releases.
Patch release: a release that increments the third part of the version number (e.g., 1.2.0 -> 1.2.1).
Bug fixes and doc updates usually go into patch releases.

Yes (this PR will be cherry-picked and included in the next patch release)
No (this PR will be included in the next minor release)

github-actions · 2025-12-03T10:01:04Z

Documentation preview for 0469093 is available at:

https://pr-19191--mlflow-docs-preview.netlify.app/docs/latest/

More info

Ignore this comment if this PR does not change the documentation.
The preview is updated when a new commit is pushed to this PR.
This comment was created by this workflow run.
The documentation was built by this workflow run.

serena-ruan · 2025-12-05T05:59:55Z

/review

🚀 Review workflow started

✅ Review completed.

Review Output

Perfect! I've completed the PR review and added comments for all the issues I found. Let me provide a summary:

PR Review Summary

I've reviewed PR #19191 "Initial query trace metrics for traces view" and found 3 code quality issues:

Issues Found:

mlflow/entities/trace_metrics.py:30 - Missing docstring on map_to_percentile() method. The mapping logic is non-obvious and would benefit from documentation.
mlflow/store/tracking/utils/sql_trace_metrics_utils.py:188 - Potential bug with walrus operator truthiness check. Using if percentile := ... would fail if a percentile value of 0.0 is ever added (since 0.0 is falsy). Should explicitly check is not None.
tests/store/tracking/conftest.py:78 - Similar truthiness issue with walrus operator. Should explicitly check is not None instead of relying on truthiness to make intent clearer and avoid potential bugs with empty strings.

All comments have been posted to the PR with specific suggestions for fixes. The review focused on:

Code quality and potential bugs
Python style guide compliance
Clear intent and maintainability

The overall implementation looks solid - these are relatively minor issues around edge cases and code clarity.

mlflow/entities/trace_metrics.py

mlflow/store/tracking/utils/sql_trace_metrics_utils.py

tests/store/tracking/conftest.py

Signed-off-by: Serena Ruan <serena.rxy@gmail.com>

daniellok-db · 2025-12-12T06:29:25Z

mlflow/store/tracking/sqlalchemy_store.py

+        if time_interval_seconds and (start_time_ms is None or end_time_ms is None):
+            raise MlflowException.invalid_parameter_value(
+                "start_time_ms and end_time_ms are required if time_interval_seconds is set"
+            )


just curious, what's the reason start / end time is required if interval is set?

I think it's good to keep control of the time frame when we want to get time series results, otherwise the result could be unpredictable based on the overall dataset. BTW it's good to align with the UI I think, there should always be a time frame. If we have requests to remove start/end_time constraint then we can remove this later

daniellok-db · 2025-12-12T06:32:24Z

mlflow/store/tracking/utils/sql_trace_metrics_utils.py

+TIME_BUCKET_LABEL = "time_bucket"
+
+
+def get_percentile_aggregation(db_type: str, percentile_value: float, column):


nit: if we have some expected types for this, should we use Literal?

Suggested change

def get_percentile_aggregation(db_type: str, percentile_value: float, column):

def get_percentile_aggregation(db_type: Literal["postgresql", "mssql", "mysql", "sqlite"], percentile_value: float, column):

This is internal function so I think it's fine to keep as string, e.g. if we want to drop mssql in the future then we don't need to update this :)

daniellok-db

lgtm

github-actions bot added the rn/none List under Small Changes in Changelogs. label Dec 3, 2025

serena-ruan force-pushed the trace_metrics_entities branch from 18a9982 to e12e73a Compare December 5, 2025 03:12

serena-ruan force-pushed the query_metrics_store branch from 899e657 to e853752 Compare December 5, 2025 03:19

github-actions bot assigned serena-ruan Dec 5, 2025

github-actions bot reviewed Dec 5, 2025

View reviewed changes

mlflow/entities/trace_metrics.py Outdated Show resolved Hide resolved

github-actions bot reviewed Dec 5, 2025

View reviewed changes

mlflow/store/tracking/utils/sql_trace_metrics_utils.py Outdated Show resolved Hide resolved

github-actions bot reviewed Dec 5, 2025

View reviewed changes

tests/store/tracking/conftest.py Show resolved Hide resolved

serena-ruan removed their assignment Dec 5, 2025

serena-ruan force-pushed the trace_metrics_entities branch 2 times, most recently from 0614e63 to b6b9f84 Compare December 8, 2025 07:57

serena-ruan force-pushed the query_metrics_store branch from e853752 to 3dee3ba Compare December 8, 2025 08:34

Base automatically changed from trace_metrics_entities to trace-metrics December 8, 2025 11:43

serena-ruan force-pushed the query_metrics_store branch from 8530dd4 to c6da702 Compare December 8, 2025 11:44

serena-ruan added the team-review Trigger a team review request label Dec 9, 2025

github-actions bot requested review from B-Step62, BenWilson2, TomeHirata, WeichenXu123, daniellok-db, harupy, kevin-lyn and xq-yin December 9, 2025 01:49

serena-ruan added 3 commits December 9, 2025 13:16

initial query trace metrics

cc70733

Signed-off-by: Serena Ruan <serena.rxy@gmail.com>

refine

159d820

Signed-off-by: Serena Ruan <serena.rxy@gmail.com>

naming

0469093

Signed-off-by: Serena Ruan <serena.rxy@gmail.com>

serena-ruan force-pushed the query_metrics_store branch from c6da702 to 0469093 Compare December 9, 2025 05:17

daniellok-db reviewed Dec 12, 2025

View reviewed changes

github-actions bot assigned daniellok-db Dec 12, 2025

daniellok-db reviewed Dec 12, 2025

View reviewed changes

daniellok-db approved these changes Dec 12, 2025

View reviewed changes

serena-ruan merged commit 0eafb78 into trace-metrics Dec 12, 2025
56 of 58 checks passed

serena-ruan deleted the query_metrics_store branch December 12, 2025 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial query trace metrics for traces view#19191

Initial query trace metrics for traces view#19191
serena-ruan merged 3 commits intotrace-metricsfrom
query_metrics_store

serena-ruan commented Dec 3, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

serena-ruan commented Dec 5, 2025 •

edited by github-actions bot

Loading

PR Review Summary

Issues Found:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniellok-db Dec 12, 2025

Uh oh!

serena-ruan Dec 12, 2025

Uh oh!

daniellok-db Dec 12, 2025

Uh oh!

serena-ruan Dec 12, 2025

Uh oh!

daniellok-db left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		TIME_BUCKET_LABEL = "time_bucket"


		def get_percentile_aggregation(db_type: str, percentile_value: float, column):

Conversation

serena-ruan commented Dec 3, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Install mlflow from this PR

Related Issues/PRs

What changes are proposed in this pull request?

How is this PR tested?

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Should this PR be included in the next patch release?

Uh oh!

github-actions bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serena-ruan commented Dec 5, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Summary

Issues Found:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniellok-db Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

serena-ruan Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

daniellok-db Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

serena-ruan Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

daniellok-db left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

serena-ruan commented Dec 3, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Dec 3, 2025 •

edited

Loading

serena-ruan commented Dec 5, 2025 •

edited by github-actions bot

Loading