-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Move components names to Ruler
, Compactor
, Querier
(-er
)
#1871
Comments
It makes sense, I agree. Also, don't forget our GH labels! 😄 |
Why not also change |
Same for |
Anyone work on this issue? I guess I can take this 😆 |
Thanks. We need to decide what to do with metrics as well as they are all over the place with |
I think we need to be careful here as this minor consistency naming change might be not worth everywhere when introducing so many breaking changes ): Things like metrics, components will make our user life harder when upgrading. |
I think updating the metrics now might not be a good option. Just update the docs first. |
👍 |
I feel like for how disruptive renaming the components would be, the gain would be too small. Yes we should be consistent in the metrics and component names, but in docs and presentations for the flow of the language we can use querier, ruler, etc. but otherwise the existing component names work just fine in my opinion and don't require changing. I don't feel super strongly about this so if the rest of the maintainers feel like this is necessary I won't stand in the way, but I feel it's going to cause more confusion and unexpected breakage than gain. |
@kakkoyun any process about this? |
Due to compatibility issues I would say we stick to docs, new places we name things like Alert names, deployment names etc. Metrics, command, and component names in code should stay for now. |
@daixiang0 This week I'll start working on this and primarily focus on docs. |
I am not sure is this still valid? |
@yeya24 Maybe we should break this into pieces. Like WDYT? |
I think we need to revisit our changes slightly here. I was under the assumption we’re only changing language in sentence form but keep technical references consistent so I think the mixin should use the command names in its structure and in let’s say alert descriptions use whatever is more fluent. |
My understanding from this issue was, we would eventually move to And it's more clear now. Just to recap. we will keep structural build blocks (code references, filenames etc.) as in commands and only rename documentation (user guideline, alert message etc). Am I right? |
Definitely, we have to find something consistent (:
What exactly this means? I assume:
There are some gray areas like logging e.g which are not really tricky to change. I think we need to just put some list and agree on this. Anything else I missed? |
That sounds perfect to me. I'll start applying that. Maybe it's worth documenting this in our |
Definitely will add - and let's make sure we keep it up to date |
Done: #2094 To close this issue we probably have to look through those items and ensure consistency. I know for sure there are metrics that are sometimes |
This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions. |
I think we’re pretty close to having all the changes down for this. Only a few metric names have to be fixed I think. |
This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions. |
It's maybe a minor but we should be consistent on naming. In the documentation, code and
thanos
command we haverule, compact, downample, query
vs in some docs, deployments and most of the presentations we are leakingQuerier
,Ruler
,Compactor
This conflicts a bit with
Store Gateway
but we already have some discussion about renaming it: #1646I would vote
-er
as it's more correct (akago
way of naming objects as well) and it avoids name collisions likerule
(recording/alerting rule) vsrule
(the component responsible for evaluating rules).AC:
< to agree>
Any thoughts?
The text was updated successfully, but these errors were encountered: