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

Initial work to switch to tracing #2185

Merged
merged 7 commits into from
Feb 11, 2023

Conversation

CosmicHorrorDev
Copy link
Contributor

Resolves #2177

cc @taylor1791 (Sorry for the slight delay. Forgot it was Halloween weekend)

This is the initial work to switch from log to tracing

Roadbumps

  • Tracing does not support dynamic levels or targets for the event!() and enabled!() macros which led to some ugly macros
  • I couldn't find a proper way for tracing to detect the log level set in log world (e.g. RUST_LOG=debug with env_logger), so both tracing and log are checked for dynamic bits
    • Worth noting that the log feature of tracing pulls in log anyways, so it doesn't add any extra deps

Future Work

It would probably be worthwhile to have some spans setup for tracking calls throughout the library, but I'll leave that to someone else since life is getting busy again

@gitmalong
Copy link

Does your work include emitting traces that follow the OpenTelemetry db convention? https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/database/

That would just be awesome.

@abonander
Copy link
Collaborator

@CosmicHorrorDev do you mind rebasing this please?

@CosmicHorrorDev
Copy link
Contributor Author

@gitmalong I doubt it. I kept the messages as is since the focus of this PR is just to switch the logging framework from log to tracing


@abonander Finished rebasing. Should be good for a review

@abonander
Copy link
Collaborator

This looks good! I remember looking into tracing and being annoyed that the level of an event had to be a constant, I'm glad you found a workaround.

@abonander abonander merged commit e321b5c into launchbadge:0.7-dev Feb 11, 2023
grgi pushed a commit to faccts/sqlx that referenced this pull request Feb 14, 2023
* Add tracing dep

* Switch over basic events

* Switch over dynamically enabled events

* Fix missing SocketAddr formatting

* More format fixing

* refactor: Apply tracing changes to new crate structure
abonander pushed a commit that referenced this pull request Feb 18, 2023
* Add tracing dep

* Switch over basic events

* Switch over dynamically enabled events

* Fix missing SocketAddr formatting

* More format fixing

* refactor: Apply tracing changes to new crate structure
abonander pushed a commit that referenced this pull request Feb 21, 2023
* Add tracing dep

* Switch over basic events

* Switch over dynamically enabled events

* Fix missing SocketAddr formatting

* More format fixing

* refactor: Apply tracing changes to new crate structure
Aandreba pushed a commit to Aandreba/sqlx that referenced this pull request Mar 31, 2023
* Add tracing dep

* Switch over basic events

* Switch over dynamically enabled events

* Fix missing SocketAddr formatting

* More format fixing

* refactor: Apply tracing changes to new crate structure
@ShayBox
Copy link

ShayBox commented May 12, 2023

log_statements requires log::LevelFilter, maybe a tracing_statements, built-in enum, int, or re-exported enum so an additional dependency isn't required for a single type

@CosmicHorrorDev
Copy link
Contributor Author

I would leave a comment on #2177 instead since that's the pseudo tracking issue for the tracing switch

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.

4 participants