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

disable tracing-log feature for tracing-subscriber #877

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

konradsz
Copy link
Contributor

What was changed

Disable the tracing-log feature for tracing-subscriber by turning off default features. The ansi feature was explicitly listed because it's used in the code.

Why?

I am trying to use core crate from Temporal SDK in a Rust application that already uses tracing-subscriber. However, it fails to initialize the subscriber because core depends on tracing-subscriber with tracing-log feature enabled. This behavior is documented here:

This method panics if a global default subscriber has already been set, or if a log logger has already been set (when the “tracing-log” feature is enabled).

In a perfect world, core would not depend on tracing-subscribers since it is a library, but I propose this as an immediate fix.

@konradsz konradsz requested a review from a team as a code owner February 12, 2025 12:49
@CLAassistant
Copy link

CLAassistant commented Feb 12, 2025

CLA assistant check
All committers have signed the CLA.

@Sushisource
Copy link
Member

Thanks for this @konradsz . I think this is left-over from when log was used internally too.

And yes, I agree about

In a perfect world, core would not depend on tracing-subscribers

In principle, but Core is in a bit of an odd position where most consumers are using FFI and so a lot of the time logging needs to be enabled by the library itself too. Potentially we could introduce another crate that thinly layers over core by adding the tracing subscriber dep and maybe a handful of other things meant for non-Rust consumers, but that's for another day.

Thanks!

@Sushisource Sushisource enabled auto-merge (squash) February 12, 2025 17:31
@Sushisource Sushisource merged commit 865cd49 into temporalio:master Feb 12, 2025
15 checks passed
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.

3 participants