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

Support aliases for function & module names (to handle refactors) #66

Open
emschwartz opened this issue Apr 14, 2023 · 6 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@emschwartz
Copy link
Contributor

If you refactor your code and change the names of functions or modules, you might want to keep some continuity with the metric names.

@akesling suggested allowing users to specify an alias for each function such that the metrics would be produced with the new and the old function/module names. That way, you could keep using the old name and then transition to the new name later and remove the alias once you've moved everything over.

I like the idea of having a parameter for the macro along the lines of function_alias="other_function_name" or function_aliases=["function_1", "function_2"] and the equivalent for module_alias and module_aliases. We could then duplicate the tracking code so it produces the metrics under all of the given names.

@emschwartz emschwartz added enhancement New feature or request good first issue Good for newcomers labels Apr 14, 2023
@gagbo
Copy link
Member

gagbo commented Apr 21, 2023

A simpler way of doing that could be to "just" duplicate the metric, so calling new_name would generate metric for both function=new_name, module=new_module and function=old_name, module=old_module (called "the alias metric").

There are 2 issues with that approach

Breaking SLOs

We need to only count one of the metrics for the SLO error budget otherwise that would break the SLO (the expectation of acceptable error levels vs. what error rate actually triggers the alert). To fix that, we can make sure that when feeding data to the alias metric, we never add the objective_name label. This - the lack of objective_name label - could also be tracked to prune the alias metric (or prompt to) after some time to save space

Tracking metric from new to old

This proposal would make it really easy to see the new stats when querying only function=old_name, module=old_module (the graph will naturally extend with the newly named function), but not really to see the old stats when querying function=new_name, module=new_module.

A solution for that would be to add new_function=new_name, new_module=new_module attributes to the alias metric, and modify slightly our current queries so that they also look for alias metrics in the past using these attributes

@Archisman-Mridha
Copy link
Contributor

Hello @emschwartz @gagbo, I am interested in contributing to Autometrics. Will it be okay if I start working on this issue ?

@mies
Copy link
Member

mies commented Jan 17, 2024

@Archisman-Mridha 100%! let us know if you run into any issues or if we can help in any way. Thank you for the support!

@Archisman-Mridha Archisman-Mridha moved this from Ready to In progress in Open Source Contributions Jan 18, 2024
@Archisman-Mridha
Copy link
Contributor

Archisman-Mridha commented Jan 18, 2024

Btw, 1 thing to note here -

Let's say we have this scenario initially -

struct Foo;

#[autometrics]
impl Foo {
    fn bar( ) { }
}

Metrics for the function will be generated with name Foo::bar.

Now, we change the struct name to Fiber and put an alias. Like this -

struct Fiber;

#[autometrics(alias = "Foo")]
impl Fiber {
    fn bar( ) { }
}

Metrics for the function will now be generated with names Fiber::bar and Foo::bar.

But then, if we also rename Fiber::bar to Fiber::plane like this -

struct Fiber;

#[autometrics(alias = "Foo")]
impl Fiber {
    fn plane( ) { }
}

Metrics for the function will now be generated with names Fiber::plane and Foo::plane. We lost Foo::bar!

@Archisman-Mridha
Copy link
Contributor

Also, someone can assign this issue to me :)

@gagbo
Copy link
Member

gagbo commented Jan 25, 2024

Metrics for the function will now be generated with names Fiber::plane and Foo::plane. We lost Foo::bar!

Yeah, to solve this, you would need to add an alias when changing the function from bar to plane:

#[autometrics(alias = "Foo")]
impl Fiber {
    #[autometrics(alias = "bar")]
    fn plane( ) { }
}

But it’s also not so important if we lose the metrics that are at least two different refactorings away. Renaming like this shouldn’t happen too often, and we do not necessarily want to generate 10 metrics if a function changed name 10 times, because it takes space.

@Archisman-Mridha Archisman-Mridha moved this from In progress to Ready in Open Source Contributions Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants