feat(weather-service): add MLflow autolog observability support#223
Conversation
602a686 to
b5879ef
Compare
pdettori
left a comment
There was a problem hiding this comment.
Review Summary
Clean, well-structured change that adds MLflow autolog support with good env-var-based auto-detection and graceful degradation. The main concern is the silent removal of the default OTLP endpoint, which is a breaking change for existing deployments that relied on it.
Areas reviewed: Python, Dependencies, Security (Trivy)
Commits: 1 commit, signed-off: ✅
CI status: Trivy failing (transitive dep CVEs in aiohttp, marshmallow, cryptography, requests — not directly from this PR's code), all other checks pass
Missing tests: No tests added for the new MLflow autolog path or dual-export logic. Even a unit test mocking mlflow.langchain.autolog to verify it's called when MLFLOW_TRACKING_URI is set would add confidence.
Verdict: COMMENT — Item below (breaking change on default endpoint) should be addressed or acknowledged.
| _root_span_var: ContextVar = ContextVar("root_span", default=None) | ||
|
|
||
| # Module-level flag set by setup_observability() to indicate active backend | ||
| _use_otel = False |
There was a problem hiding this comment.
nit — Module-level mutable global is fine for init-once, but if you ever want to test the different code paths in isolation, a simple namespace (e.g., _state.use_otel) would make mocking cleaner.
b5879ef to
a598cd0
Compare
Auto-detect tracing backend from environment variables: MLflow LangChain autolog when MLFLOW_TRACKING_URI is set, OpenTelemetry with OpenInference when OTEL_EXPORTER_OTLP_ENDPOINT is set. When both are configured, both backends are active simultaneously and MLFLOW_TRACE_ENABLE_OTLP_DUAL_EXPORT is enabled automatically so traces reach both MLflow Tracking Server and OTEL Collector. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>
a598cd0 to
6b1d81e
Compare
Trivy CI Failure AnalysisThe Trivy check failure is not caused by this PR. All 16 CVEs are in transitive dependencies that already exist on
Because the PR regenerated Bumping these deps (especially aiohttp → 3.13.4) would be good housekeeping but is independent of this PR. |
Summary
Auto-detect tracing backend from environment variables: MLflow
LangChain autolog when MLFLOW_TRACKING_URI is set, OpenTelemetry with
OpenInference when OTEL_EXPORTER_OTLP_ENDPOINT is set. When both are
configured, both backends are active simultaneously and
MLFLOW_TRACE_ENABLE_OTLP_DUAL_EXPORT is enabled automatically so
traces reach both MLflow Tracking Server and OTEL Collector.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Related issue(s)
(Optional) Testing Instructions
Fixes #