Add request-scoped DB query tracing to metadata service - #7
Open
vagxrth wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #5.
This PR adds request-scoped DB query tracing to the metadata service using a
ContextVar-backed tracer plus anaiohttpmiddleware.The tracing is fully opt-in:
QUERY_TRACING_ENABLED=1X-Metaflow-Trace-DB: 1When enabled for a request, the metadata service logs:
query_counttotal_rowsdb_time_msrequest_time_msat the end of the request, without changing any API payloads or schemas.
What Changed
Request lifecycle tracing
aiohttpmiddleware that:finallyDB instrumentation
execute_sql()path used by metadata-service read APIs.execute(...)fetchall()db_time_ms.Failure handling
row_count=0error_typeSafe SQL logging
Why This Approach
This implementation keeps the strong parts of the linked PR direction (
ContextVar+ middleware), but tightens the behavior in a few important ways:Logging Behavior
For traced requests, the service logs one request summary like:
query_counttotal_rowsdb_time_msrequest_time_msWhen the
QueryTracinglogger is in DEBUG, it also logs one line per query with:Tests Added
Unit tests
Added coverage for:
ContextVarisolation across concurrent tasksIntegration tests
Added coverage for:
GET /flows/{flow_id}/runsGET /flows/{flow_id}/runs/{run_number}/steps/{step_name}/tasksLocal Verification
Passed locally:
python3 -m py_compile ...git diff --check./.venv/bin/pytest services/metadata_service/tests/unit_tests/query_tracing_test.py -qIntegration tests are included, but could not be completed end-to-end in this environment because the repo’s integration suite expects:
db_testgoose) available in the test environmentNotes
This PR is intentionally scoped to metadata-service read tracing through
execute_sql(). It does not expand tracing to: