Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: replace env-logger with tracing #3878

Merged
merged 15 commits into from
Dec 20, 2023
Merged

chore: replace env-logger with tracing #3878

merged 15 commits into from
Dec 20, 2023

Conversation

kobyhallx
Copy link
Contributor

Description

Problem*

While env-logger concerns itself with logging, tracing is drop in replacement to support logging case but also provides more features for profiling.

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

I like the look of the span feature from tracing as that simplifies how we were using a lot of our tracing previously.

If we switch over to that + clean up commented deps then I think we can merge assuming the log output looks ok.

acvm-repo/acvm/src/compiler/transformers/mod.rs Outdated Show resolved Hide resolved
acvm-repo/acvm_js/Cargo.toml Outdated Show resolved Hide resolved
compiler/wasm/Cargo.toml Outdated Show resolved Hide resolved
@kobyhallx
Copy link
Contributor Author

I like the look of the span feature from tracing as that simplifies how we were using a lot of our tracing previously.

If we switch over to that + clean up commented deps then I think we can merge assuming the log output looks ok.

Let's do spans in separate PR.

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM

@TomAFrench TomAFrench changed the title chore: replace env-logger with tracing chore: replace env-logger with tracing Dec 20, 2023
@kobyhallx kobyhallx added this pull request to the merge queue Dec 20, 2023
Merged via the queue into master with commit b594919 Dec 20, 2023
31 checks passed
@kobyhallx kobyhallx deleted the kh-drop-env-log branch December 20, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants