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

[Feature]: MeterProvider::meter should not borrow name argument as 'static #2347

Open
gliderkite opened this issue Nov 26, 2024 · 2 comments
Labels
enhancement New feature or request triage:todo Needs to be traiged.

Comments

@gliderkite
Copy link

Related Problems?

The trait API of MeterProvider

fn meter(&self, name: &'static str) -> Meter

requires to borrow name as 'static

Describe the solution you'd like:

It should be changed, if possible, to

fn meter(&self, name: impl Into<Cow<'static, str>>) -> Meter

so that is will be possible to construct a new Meter even when it is not possible to keep the name alive as 'static reference.

This is similar to what already happens with the TracerProvider trait in

fn tracer(&self, name: impl Into<Cow<'static, str>>) -> Self::Tracer

(I would have opened a PR myself, but currently running cargo check on main fails)

Considered Alternatives

No response

Additional Context

No response

@gliderkite gliderkite added enhancement New feature or request triage:todo Needs to be traiged. labels Nov 26, 2024
@cijothomas
Copy link
Member

It was intentionally made 'static, see #2112 (comment)

Can you elaborate more why 'static is not sufficient for meter name? Meter name represents the scope of instrumentation, usually the crate name or module name. I'd love to know more of scenarios which we may have missed.

@gliderkite
Copy link
Author

Thank you @cijothomas for providing context on this.

Use case is this: each instance of the deployed service has a name that comes as an input (via CLI args). This name is not known at compile time, it is set (by terraform) and cannot be hardcoded as unique static string for the whole crate.

While I can certainly use a workaround like String::leak, which is fine in practice given the minimal amount of strings involved, I would aim more for application code that: (1) is not ambiguous, and having to leak can certainly raise more questions than it should IMO, and (2) a situation where the behaviour of the application code is not so strongly enforced by the library APIs.

I understand the good intent and the reasons of wanting to signal the large scope of this variable by making it 'static, but I would also argue that there are use cases where this is not ideal, and that having an API that mirrors that of the other traits would be more intuitive and easier to use for the user. Good documentation can deal with the rest.

Just a suggestion, keep up with the nice work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage:todo Needs to be traiged.
Projects
None yet
Development

No branches or pull requests

2 participants