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

Revisit Dokka's logging messages and API #2944

Open
IgnatBeresnev opened this issue Mar 27, 2023 · 5 comments
Open

Revisit Dokka's logging messages and API #2944

IgnatBeresnev opened this issue Mar 27, 2023 · 5 comments
Assignees
Labels
tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life
Milestone

Comments

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Mar 27, 2023

There's a number of complaints related to Dokka's logging messages:

The majority of the reports can be summed up as "Dokka logs too many irrelevant messages".

At first, the problem might come off as being runner-specific. For instance, one might think the Gradle runner's mapping of logging levels is incorrect. So the obvious solution is to just re-route it like in #2915.

However, the problem itself is deeper than that.


Meeting user expectations

It'd be ideal to start with understanding what Dokka should log by default under default runner configurations (like running the Gradle tasks with no additional flags).

The decision might be runner-dependant. For example, Dokka might log nothing when run with the Gradle runner as Gradle itself reports task progress, so the users will be able to see that Dokka is working. However, it's not acceptable for the CLI runner - it reports nothing by itself, so if it completes silently the users might think it didn't work. For this reason, maybe Dokka's PROGRESS level should be used exclusively for the CLI runner, or a separate level / mechanism needs to be introduced for that.

The goal here is so that the general and broad user expectations are met when Dokka is run with different runners.

DokkaLogger API

DokkaLogger is considered to be an abstraction for logging.

At the moment, it's not very clear how to use this API. The following questions should be addressed:

  • When should each logging level be used? That is, which information should be logged under info and which under progress? What is the default logging level that the users will see messages of? Some guidance needs to be added so that developers/contributors/plugin authors know when to use each level.
  • What is the priority of Dokka's logging levels. Is PROGRESS more important than INFO? At the moment, the order of functions in DokkaLogger leads to to believe that PROGRES > INFO. However, if you look at LoggingLevel#index, you'll see that PROGRESS < INFO. Maybe PROGRESS needs to stand on its own and not be compared to other levels at all.

Logging level mapping

Once the API questions have been addressed, the mapping of Dokka's logging levels should be revisited. The implementations of DokkaLogger should be consistent for our runners.

  • Gradle plugin logging mapping. Gradle's default logging level is lifecycle, which now corresponds to Dokka's PROGRESS. However, if Dokka's PROGRESS < INFO, this mapping is incorrect, and it needs to be adjusted.
  • Maven plugin logging mapping. Maven uses the standard logging levels, so it doesn't have anything specific like PROGRESS or LIFECYCLE. However, it does have the trace leve, which other runners don't. At the moment, Dokka's PROGRESS and INFO both get mapped to Maven's INFO, which might be incorrect.

Existing logging messages

Dokka emites many redundant messages already. For example, it logs the execution time of its steps under INFO.

Chances are, after the API refactoring outlined above, it might emit even more redundant messages than before, so the existing messages need to be revisited: they can be removed/re-written or their logging level can be changed.

Note: there's a configuration property DokkaConfiguration#failOnWarning - the behaviour should be preserved, so pay attention to messages with the WARN level.

@IgnatBeresnev IgnatBeresnev added the tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life label Mar 27, 2023
@aSemy
Copy link
Contributor

aSemy commented Mar 28, 2023

A couple of lessons learned from Dokkatoo:

@IgnatBeresnev
Copy link
Member Author

Update: Denis will think it through from the user's perspective (taking into consideration the CLI runner and the default Gradle/Maven levels), and also try to categorize the existing messages that Dokka emits. Once that's done and we understand what we want to achieve in the end, it can be implemented in code, with API changes if necessary.

@IgnatBeresnev
Copy link
Member Author

IgnatBeresnev commented Oct 6, 2023

Update: Denis went through all of the existing messages and tried to group them together to see the trends. Based on it, we've worked out some guidelines for each logging level about how they should be used by developers and how they should be mapped/displayed by the runners.

We will next try to change the logging levels of the current messages to adhere to the agreed upon guidelines, which will also help validate them.

Increasing the logging coverage seems to be out of scope of this issue, and should be done separately.

@IgnatBeresnev IgnatBeresnev added this to the Dokka 1.9.20 milestone Oct 17, 2023
@d-ambatenne
Copy link
Contributor

Plans:

  • Create a PR with new logging levels requirements
  • Change the logging according to the new requirements.
  • Change all messages with the DEBUG level to a better understandable by humans
  • Design a better UX by adding more informative messages to the PROGRESS level.

d-ambatenne pushed a commit that referenced this issue Oct 25, 2023
d-ambatenne added a commit that referenced this issue Dec 18, 2023
Co-authored-by: Denis Ambatenne <denis.ambatenne@jetbrains.com>
Co-authored-by: IgnatBeresnev <ignat.beresnev@jetbrains.com>
d-ambatenne pushed a commit that referenced this issue Feb 9, 2024
d-ambatenne pushed a commit that referenced this issue Feb 9, 2024
d-ambatenne pushed a commit that referenced this issue Feb 12, 2024
vmishenev pushed a commit that referenced this issue Mar 20, 2024
Co-authored-by: Denis Ambatenne <denis.ambatenne@jetbrains.com>
Co-authored-by: IgnatBeresnev <ignat.beresnev@jetbrains.com>
@eirnym
Copy link

eirnym commented Jul 4, 2024

@d-ambatenne there was a PR for that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt A technical issue that is not observable by the users, but improves maintainers quality of life
Projects
None yet
Development

No branches or pull requests

5 participants