fix: extend mlflow.sourceRun metrics filter to cover post-hoc linked OTLP traces#23591
Conversation
Install mlflow from this PR
Install mlflow from this PR# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/23591/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/23591/merge#subdirectory=libs/skinnyFor Databricks, use the following command: %sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/23591/mergePR author's recent activityIn the last 14 days, @RudraDudhat2509 opened 10 PRs across 8 repos:
|
github-actions
Bot
added
size/L
|
overall good, but please remove many lines format changes which are not related to the fix. |
|
Documentation preview for b3fe5b6 is available at: More info
|
22a66f3 to
77d9111
Compare
|
Done! Pushed a cleaned commit the formatting changes from ruff format have been removed. The diff now only contains the two new imports and the _apply_filters logic change. The test file is back to original style with just the one new test function added. |
There was a problem hiding this comment.
Pull request overview
This PR fixes trace-metrics filtering for evaluation runs by making the trace.metadata.mlflow.sourceRun filter also match OTLP-ingested traces that were linked to a run post-hoc via link_traces_to_run() (stored in SqlEntityAssociation, not trace metadata). This aligns the trace count badge and assessment score distribution queries with the trace list behavior for linked traces.
Changes:
- Extend
_apply_filters()to OR-combine the existingSqlTraceMetadatafilter with anSqlEntityAssociationEXISTS check when filtering onTraceMetadataKey.SOURCE_RUN. - Add a new SQLAlchemy store test ensuring the source-run filter includes linked OTLP-style traces in trace count metrics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
mlflow/store/tracking/utils/sql_trace_metrics_utils.py |
Expands SOURCE_RUN filtering to include traces associated to a run via SqlEntityAssociation (post-hoc OTLP linking). |
tests/store/tracking/sqlalchemy_store/test_sqlalchemy_store_query_trace_metrics.py |
Adds regression test data setup for native + linked OTLP traces and asserts the trace-count metric includes both. |
| """OTLP traces have no mlflow.sourceRun metadata; they are linked post-hoc via | ||
| link_traces_to_run(). Q2/Q3 on the Traces tab filter by mlflow.sourceRun — this test | ||
| verifies that the filter also matches entity-association-linked traces (issue #23530).""" |
There was a problem hiding this comment.
@RudraDudhat2509 would you address this comment for test code ? :)
There was a problem hiding this comment.
Added a second assertion for the ASSESSMENTS view (Q3) with a feedback logged against one of the linked OTLP traces. Also fixing the lint failure, pushing now
…inked OTLP traces Q2/Q3 on the Traces tab (trace count badge, assessment score distribution) filter by trace.metadata.mlflow.sourceRun, which OTLP traces never populate. Traces linked post-hoc via link_traces_to_run() are stored in SqlEntityAssociation, not metadata. OR-combine the metadata EXISTS with an EntityAssociation EXISTS in _apply_filters so the filter returns correct results for both native and OTLP-ingested traces. Fixes mlflow#23530 Signed-off-by: Rudra Dudhat <[email protected]>
77d9111 to
9459903
Compare
Related Issues/PRs
Closes #23530
What changes are proposed in this pull request?
When traces arrive via the OTLP endpoint and are linked to a run post-hoc via
link_traces_to_run(), the Traces tab fires three queries but Q2 (trace count badge)and Q3 (assessment score distribution) filter bytrace.metadata.mlflow.sourceRun. OTLP traces never write this metadata key; their run association lives inSqlEntityAssociation. The result is count=0 and score=NaN.Fix: in
_apply_filters(sql_trace_metrics_utils.py), when the filter key isTraceMetadataKey.SOURCE_RUN, OR-combine the existingSqlTraceMetadataEXISTS check with aSqlEntityAssociationEXISTS check. This covers both native MLflow traces (metadata path) and post-hoc linked OTLP traces (entity-association path).No API changes, no schema migrations.
How is this PR tested?
New test:
test_query_trace_metrics_source_run_filter_includes_linked_otlp_tracesin
tests/store/tracking/sqlalchemy_store/test_sqlalchemy_store_query_trace_metrics.py.Creates 1 native trace (with
mlflow.sourceRunmetadata) + 2 OTLP-style traces(no metadata, linked via
link_traces_to_run()) + 1 unrelated trace, and assertsthat the
mlflow.sourceRunfilter returns COUNT=3.Does this PR require documentation update?
Does this PR require updating the MLflow Skills repository?
Release Notes
Is this a user-facing change?
The trace count badge and assessment score distribution on the Traces tab of an
Evaluation Run now correctly include traces linked post-hoc via
client.link_traces_to_run()(e.g. OTLP-ingested traces). Previously these showed 0 / NaN.What component(s), interfaces, languages, and integrations does this PR affect?
area/tracing: MLflow Tracing features, tracing APIs, and LLM tracing functionalityHow 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" sectionrn/breaking-change- The PR will be mentioned in the "Breaking Changes" sectionrn/feature- A new user-facing feature worth mentioning in the release notesrn/bug-fix- A user-facing bug fix worth mentioning in the release notesrn/documentation- A user-facing documentation change worth mentioning in the release notesIs this PR a critical bugfix or security fix that should go into the next patch release?