Handling methods annotated within an impl block

[gagbo]

The issue

There is a small inconsistency that might need to be resolved when annotating a complete impl block vs. annotating a method within the block:

// in src/my_mod.rs
#[autometrics]
impl Fitz {
    fn chivalry() {}
}

will produce metrics with

{
    "module": "my_mod",
    "function": "Fitz::chivalry"
}

whereas

// in src/my_mod.rs
impl Fitz {
    #[autometrics]
    fn chivalry() {}
}

will produce metrics with

{
    "module": "my_mod",
    "function": "chivalry"
}

In the second case we should also have the Fitz:: prefix but it doesn’t appear.

It cannot be solved without user input

The issue in the second case here is that the proc-macro cannot access code outside of its scope to find the parent impl Fitz clause. I don’t think there is a way to still just have the bare #[autometrics] annotation and work out the parent class name in the macro code.

A proposal

Besides documenting this limitation and advising splitting impl blocks so that all autometrics functions live within an annotated impl block, we could also maybe add an extra argument to the macro like

// in src/my_mod.rs
impl Fitz {
    #[autometrics(class = "Fitz")]
    fn chivalry() {}
}

to restore the labelling for these cases. What do you think?

[mellowagain]

as we cannot access the outer impl in the second case, maybe we should just strip the outer namespace for the first example? it would at least be consistent then, even thought probably not fully what we want

[gagbo]

The risk with removing the information in all cases is that it might create conflicts with metric names which would lead to wrong reporting, if you have multiple methods with the same name in the same module (but scoped for different structs), like for example in opentelemtry which has multiple collect method definitions in the same file for different structs:

https://github.com/open-telemetry/opentelemetry-rust/blob/08c3d48dc8661773d96b3ab31994db624ac4a8eb/opentelemetry-prometheus/src/lib.rs#L279

https://github.com/open-telemetry/opentelemetry-rust/blob/08c3d48dc8661773d96b3ab31994db624ac4a8eb/opentelemetry-prometheus/src/lib.rs#L169

This actually also highlight the possible issues with multiple traits definining methods with the same name, but we can also choose to solve that later (Should the function name actually be <Fitz as Foo>::chivalry for all trait implementation blocks ?)

[Archisman-Mridha]

Hello @gagbo, I am new to open-sourcing and this is my first time contributing to Autometrics. I have opened a PR which solves this issue. Please let me know if I need to make any further changes or it's ready to be merged 😊.