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

Can't disambiguate module paths and custom targets #390

Closed
acfoltzer opened this issue Apr 4, 2020 · 6 comments
Closed

Can't disambiguate module paths and custom targets #390

acfoltzer opened this issue Apr 4, 2020 · 6 comments

Comments

@acfoltzer
Copy link

I'm writing a log implementation and am hoping to use the target: "custom_target" syntax to direct logging output to various endpoints. If the target is omitted when calling the macros, though, the target is filled in with the module path of the call site. This leads to ambiguity between the special-purpose targets and the module paths.

There are a couple of workarounds I'm considering, but neither is totally satisfactory:

  1. Compare record.target() against record.module_path(), and ignore the target if they are equal. This approach has false negatives, and does the wrong thing if the target happens to share the name of the module path. This is somewhat common because the module path of a main.rs is the name of the crate.

  2. Initialize the logger with the expected custom targets, and compare record.target() for membership in that set, treating it as a custom target if there's a match. This approach incurs the performance cost of that lookup, plus false positives in the case where there's a name collision.

It would be great if the data structures just indicated whether or not a target was explicitly set by the macro caller. If I were starting from scratch, I would propose making record.target() return Option<&str>, but that's probably too disruptive a change at this point. Perhaps we could add record.is_custom_target() -> bool, and make sure that flag is set appropriately based on which pattern is matched in log!?

I'm happy to work on the code for this, but would like to have some validation of the design before digging in.

@sfackler
Copy link
Member

sfackler commented Apr 4, 2020

2. This approach incurs the performance cost of that lookup, plus false positives in the case where there's a name collision.

You can easily make targets that can't collide with module names.

@acfoltzer
Copy link
Author

acfoltzer commented Apr 4, 2020

Hmm, I don't think I follow. The target names are arbitrarily chosen by the users of our library. I can't think of a way to prevent collisions without our library having to define wrappers around the log macros.

@KodrAus
Copy link
Contributor

KodrAus commented Dec 21, 2020

The target method doesn't seem like a great fit for this, but at the moment is really the only option you've got for the log! macros.

I wouldn't be against some kind of custom_target method that returned an Option<&str>, making target effectively module_path.or(custom_target). We'd just have to balance that against generating more code at the log! callsite, but could do that only for cases where a target is actually specified.

@KodrAus
Copy link
Contributor

KodrAus commented Nov 15, 2021

Just coming in through some triage. It's been a while here, and with an impending structured logging hammer turning all problems into nails, I think we can close this one in favor of preferring other properties of the log events for filtering and forwarding.

@KodrAus KodrAus closed this as completed Nov 15, 2021
@sweihub
Copy link

sweihub commented Jun 12, 2023

I am bothered by this issue too, log::set_logger(...) affects all crates using log facade, then all logs flush into my logger. Any solution for now?

@sweihub
Copy link

sweihub commented Jun 12, 2023

I found a cheap solution, just ignore the absolute files, those are from other crates.

impl log::Log for Log2 {
    fn enabled(&self, metadata: &Metadata) -> bool {
        // this seems no effect at all
        metadata.level() <= Level::Trace
    }

    fn log(&self, record: &Record) {
        // cheap way to ignore other crate with absolute file (UNIX)
        let file = record.file().unwrap_or("unknown");
        if file.starts_with("/") {
            return;
        }
        ...
    }
}

check out the log2

EFanZh pushed a commit to EFanZh/log that referenced this issue Jul 23, 2023
…se build (rust-lang#390)

* Add new feature `log_release_max_level_debug`
* Fix calculation of `log_level` in `logging`
* Enable feature log_release_max_level_debug on CI release build

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
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

No branches or pull requests

4 participants