-
Notifications
You must be signed in to change notification settings - Fork 10
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
RFC: Observability API #97
base: main
Are you sure you want to change the base?
Conversation
34cb471
to
3b91376
Compare
afbb0c4
to
2864c28
Compare
4b9fc27
to
19b1fdb
Compare
So discussed with @R-Lawton in person, but there seems to be some redundancy in the configuration and needs a user to understand the names and configuration options for underlying components. Also a separate CRD doesn't seem needed IMO if we condense the config I think we can simplify this down somewhat into a two phase approach
Where by default it is set to We can then expand beyond this in a second phase add comming configmap for expressing observability values that can be loaded as env vars (logging for example) so that it changes all components including the operators . Add in new options around alerts and dashboards as needed. The status of the kuadrant resource can reflect what has been configured:
WDYT @Boomatang @R-Lawton |
By default the log level is info, what are your thoughts about changing it to error? |
To me this makes sense, I do think we need the focus more on the action being preformed over the component being configured. How I see this now is we should remove the current ability to configure Limitador from the kuadrant CR in favor of the more action based configuration. If you have not seen the discussion, #102 that suggest removing the work done for RFC0006 I will take your comment as the answer for that discussion. After talking with @R-Lawton, we can not see any iteration of this RFC that would not require the removal of the past configuration implantation. So I have opened a PR to remove that work, Kuadrant/kuadrant-operator#795 |
c5dbdbc
to
d0ff168
Compare
Signed-off-by: R-Lawton <[email protected]> Signed-off-by: R-Lawton <[email protected]>
478cd8e
to
81b6211
Compare
ee05ec6
to
f4e97e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks generally good, I've left some minor suggestions 👍
…e location to the Kuadrant CR Signed-off-by: R-Lawton <[email protected]> Signed-off-by: R-Lawton <[email protected]>
2dd0563
to
ed868d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
What are people's thoughts on the original suggestion of the observability CR? After a discussion i had with Jim, I think the observability policy is good but I would like others' opinions too.