Kuadrant / architecture

Architecture Documents
0 stars 10 forks source link

RFC: Observability API #97

Open R-Lawton opened 1 month ago

R-Lawton commented 1 month ago

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.

maleck13 commented 1 month ago

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

1) offer log and tracing options in the kuadrant CR (but they will only affect Authorino and Limitador) in phase 1 and be as simple as:

logging:
   level: debug
tracing:
  common tracing config applied to both limitador and authorino
metrics:
  enabled: false (removes service monitors) enabled by default

Where by default it is set to error level for logging and a default logging config plus , metrics defaulted to on.

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:

logging:
   authorino: debug
   limitador: debug
tracing:
  ... 

WDYT @Boomatang @R-Lawton

R-Lawton commented 1 month ago
  1. offer log and tracing options in the kuadrant CR (but they will only affect Authorino and Limitador) in phase 1 and be as simple as:
logging:
   level: debug
tracing:
  common tracing config applied to both limitador and authorino
metrics:
  enabled: false (removes service monitors) enabled by default

Where by default it is set to error level for logging and a default logging config plus , metrics defaulted to on.

@maleck13

By default the log level is info, what are your thoughts about changing it to error?

Boomatang commented 1 month ago

@maleck13

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, https://github.com/Kuadrant/architecture/discussions/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, https://github.com/Kuadrant/kuadrant-operator/pull/795