Skip to content

feat: expose granular trace event targets#6853

Merged
westonpace merged 3 commits into
lance-format:mainfrom
beinan:beinan/expose-event-targets
May 21, 2026
Merged

feat: expose granular trace event targets#6853
westonpace merged 3 commits into
lance-format:mainfrom
beinan:beinan/expose-event-targets

Conversation

@beinan
Copy link
Copy Markdown
Contributor

@beinan beinan commented May 20, 2026

Summary

  • Add granular tracing targets for Lance event categories under lance::events::....
  • Preserve the original tracing target in Python log passthrough so LANCE_LOG can filter individual event types directly.
  • Document the new event targets and add Python regression coverage for target-specific log filtering.

Testing

  • PATH=/Users/beinan/.rustup/toolchains/1.94.0-aarch64-apple-darwin/bin:$PATH cargo fmt --all -- --check
  • git diff --check
  • PATH=/Users/beinan/.rustup/toolchains/1.94.0-aarch64-apple-darwin/bin:$PATH cargo check -p lance-core -p lance-io -p lance-index -p lance -p lance-datafusion
  • PATH=/Users/beinan/.rustup/toolchains/1.94.0-aarch64-apple-darwin/bin:$PATH cargo check --manifest-path python/Cargo.toml
  • uv sync --python 3.12 --no-install-project
  • .venv/bin/maturin develop --uv -v
  • uv run --no-sync --python 3.12 pytest -q python/tests/test_log.py::test_lance_log_filters_trace_event_targets python/tests/test_tracing.py::test_tracing_callback
  • uv run --no-sync --python 3.12 ruff format --check python/tests/test_log.py python/tests/test_tracing.py
  • uv run --no-sync --python 3.12 ruff check python/tests/test_log.py python/tests/test_tracing.py

@github-actions github-actions Bot added enhancement New feature or request A-python Python bindings labels May 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-io/src/object_store/throttle.rs 92.30% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@beinan beinan marked this pull request as ready for review May 20, 2026 03:27
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copy link
Copy Markdown
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a fan of this change but I'm hoping we can do it without modifying existing targets (to add events::) because that change would be disruptive to us.

Also, semi-related, but I've been thinking we should move more (if not all) of our log statements to tracing events. Having both a "log stream" and a "trace event stream" is pretty confusing.

Comment thread rust/lance-core/src/utils/tracing.rs Outdated
Comment thread rust/lance-core/src/utils/tracing.rs Outdated
@beinan
Copy link
Copy Markdown
Contributor Author

beinan commented May 20, 2026

For the Azure request-id concern: the final object-store error is preserved through LanceError(IO), and I added regression coverage that an Azure-style x-ms-request-id present in the object-store error text survives; response headers that object_store::RetryError does not expose would still need an upstream object_store change.

Copy link
Copy Markdown
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!


let lance_error = lance_core::Error::from(err);
let error_message = lance_error.to_string();
assert!(error_message.contains("x-ms-request-id"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised the mock store actually includes these

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I mocked the response msg somewhere 😆

@beinan
Copy link
Copy Markdown
Contributor Author

beinan commented May 20, 2026

@westonpace can we merge?

@westonpace westonpace merged commit dbeeefb into lance-format:main May 21, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants