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

Proposal: Quick-Logging Approach #129

Closed
DawidNiezgodka opened this issue Oct 20, 2022 · 0 comments · Fixed by #131
Closed

Proposal: Quick-Logging Approach #129

DawidNiezgodka opened this issue Oct 20, 2022 · 0 comments · Fixed by #131
Assignees
Labels
area/tooling Relates to tooling around Quick, e.g. justfile type/design Design documents for enhancements

Comments

@DawidNiezgodka
Copy link
Contributor

DawidNiezgodka commented Oct 20, 2022

Synopsis

At the time of writing, there are 152 log statements in Quick. Some are ok, but some are superfluous (either not informative, lacking context, or duplicated). Furthermore, logging levels are not used consistently.

Concrete plan for changes

Log levels

INFO
I perceive log.info as a piece of info for the user. They want to know on a high level what happens, but, for example, when they make a query, they are only concerned about the answer and not that the request was rerouted because of a problem.

  • user-triggered actions: topic (CRD), gateway (CRUD), app (CD) + context, f.e. name
  • start-up of services

DEBUG
Additional context

  • Details of actions. When a user-triggered action triggers sub-actions, for example: creation of topic triggers creation of mirror
  • input needed to reproduce
  • keys, reqs, resp, etc.

TRACE
Extra info for detailed debugging

  • detailed descriptions of particular steps
  • output

Duplicated

It'd be good to decide whether we want to log twice at different service levels (see (5) in the Logging guideline above). Furthermore, determine if we log at the callee or caller level. Logging twice in the same class at both the callee and caller level doesn't broaden the understanding of what happens (see KafkaTopicService regarding the topic creation).

Missing log statements

Concerning missing log statements, I would only add info statements for now. Regarding debug and trace, I would add them on demand.

Components

Manager

  1. Creating, updating, and deleting user-triggered (i.e., via CLI) resources will be logged at the info level. Currently, some of these activities are debug, some are info, and some are not logged at all). Such log statements should contain some context info, f.e. a topic name.
  2. Subprocesses, for example, creating a mirror when a user creates a topic, might be logged at the debug level. Whether we need trace there could be discussed.

Mirror

Mirror uses quite an extensive logging, which is fine. Numerous intern things are on debug, which is also acceptable.
To discuss:

  1. If we want to log info about the request forwarding on the info level. Isn't it instead an intern thing? Moreover, if we inform about the forwarding to a specific host at the info level, shouldn't we log the info about the original host at the same level?
  2. Again, the question from the General above applies. MirrorController's getRange function logs: "Request for key {} and range from {} to {}", keyString, from, to and then calls KafkaQueryService.getRange(), where the same info might be logged again.
  3. If a piece of info like "Record value of type Avro Generic Record" has the trace level, shouldn't
    "Type Avro detected" also be logged at trace and not debug? Seems like an additional, detailed piece of info that is supposed to help with meticulous debugging.

Ingest

  1. If we log.debug info about sending the data, we might as well log info about deleting it. Again, there is a question about the callee-caller relationship.
  2. We might add some info about the sent data in the trace.

Common

I would put all routing info to the debug level.

Gateway

@DawidNiezgodka DawidNiezgodka self-assigned this Oct 20, 2022
@DawidNiezgodka DawidNiezgodka added the type/design Design documents for enhancements label Oct 20, 2022
@DawidNiezgodka DawidNiezgodka added this to the Tracing and logging milestone Oct 25, 2022
@DawidNiezgodka DawidNiezgodka added the area/tooling Relates to tooling around Quick, e.g. justfile label Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tooling Relates to tooling around Quick, e.g. justfile type/design Design documents for enhancements
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant