metrics-rs / metrics

A metrics ecosystem for Rust.
MIT License
1.13k stars 158 forks source link

Cannot add expression labels on Rust 2021 #270

Closed nrxus closed 2 years ago

nrxus commented 2 years ago

repro repo: https://github.com/nrxus/metrics-playground

I am getting the following error when I try to compile any of the metrics macros with existing labels:

error: expected labels expression, but expression not found
 --> src/main.rs:3:34
  |
3 |     metrics::gauge!("some", 5.0, &labels);
  |                                  ^

error: could not compile `metrics-playground` due to previous error

This only happens on Rust 2021 however. If I change my edition to 2018 AND I add the syn crate manually with full feature then it works. If I keep the feature and go back to Rust 2021 it fails. If I remove the feature and I am on Rust 2018 it fails.

I wasn't sure if this is a bug here or in syn.

tobz commented 2 years ago

Thanks for reporting this!

I don't think I've pinned any of my projects to 2021 yet so I haven't personally encountered this. I'll see if I can take a look at your reproduction example in the next few days to figure out what's going on.

nrxus commented 2 years ago

this is the smallest example I could think of: https://github.com/nrxus/metrics-playground/blob/master/src/main.rs

tobz commented 2 years ago

Sorry, yeah.. I just somehow mentally blocked out the fact you posted a reproduction link when typing out my original message. That's on me. :)

nrxus commented 2 years ago

No worries, it happens (:

nrxus commented 2 years ago

I think I figured out why the edition matters.

In Rust 2021 it uses a new feature resolver which will not pollute build dependency features with normal dependency features. So in Rust 2021 if I add syn as a build dependency with the feature full it compiles fine. In Rust2018 adding it as a normal dependency would do the trick since the features would pollute across targets.

The bug from the metrics side is that it doesn't include the full feature of syn even though it is needed for the simple labels as expressions case.

tobz commented 2 years ago

Ah, yes, feature resolver changes. 😅

Yeah, we can fix up how the features are applied and get out a patch release. Thanks for sussing this out. 👍🏻

tobz commented 2 years ago

Released metrics-macros@v0.5.1 which moved the full feature flag up to the normal syn dependency.

Thanks again for the report, as well as researching the fix. Let me know if anything else crops up around breakage under the 2021 edition.