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

HSEARCH-798 Use logging categories #4381

Merged

Conversation

marko-bekhta
Copy link
Member

https://hibernate.atlassian.net/browse/HSEARCH-798

This commit only splits the existing Log interfaces and moves messages/exceptions to more "specific" interfaces that represent logging categories.
While at it I've adjusted the usage of logging interfaces in a way that they are not used outside the module (I'll take a look at jqassistant to see if any rules can be adjusted there) as I've noticed that some were "shared" even if just for the tests.

Also, there were some string messages in the logging interfaces. Those are now moved to new/existing *Hints message bundles

It also adds a new annotation:

@CategorizedLogger(
		category = FormattingLog.CATEGORY_NAME,
		description = "Anything related to parsing/formatting."
)

which may change in near future 😄. The plan is to have all the interfaces-categories be marked with this interface and then collect the info from it to build a documentation appendix containing available logging categories and what they are for. Not all categories are used for logging (some are there only with exceptions), hence the plan is to check:

  • is there any void blabla(..) methods
  • or whether the interface-category extends the BasicLogger (and potentially uses SOME_LOGGER.infof kind of methods)

And if so, consider that category as such that can log something.
Then, since categories are shared between modules each module may contribute more to the category description.

To address the "hmmmmmmmmmmmmmmmm what's the next id to use?!!" I've added nextLoggerIdForConvenience in the interfaces combining categories. It has the next "free ID"; let's see how it goes in practice...

Next, I'll work on "something" that generates that docs appendix.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


@marko-bekhta marko-bekhta force-pushed the feat/HSEARCH-798-logging-categories branch 3 times, most recently from f656967 to 1cb18d6 Compare November 14, 2024 08:32
@marko-bekhta
Copy link
Member Author

I've looked at the reports that the logging lib produces:
image

and it doesn't match what we'd wanted to add to the reference. Hence, I ended up with an annotation processor that generates a YAML into meta-inf/hibernate-search/logging-categories.yaml file like:

# List of logging categories that this module utilizes.
org.hibernate.search.report:
  module: hibernate-search-engine
  categories:
  - name: org.hibernate.search.common.failures
    description:
    - |
      Logs unexpected problems that occur during background Hibernate Search processes.
  - name: org.hibernate.search.configuration
    description:
    - |
      Logs warnings about unused configuration properties and tracking of such properties.
      +
      Debug logs may also include information on configuration providers or other configuration-related issues.
  - name: org.hibernate.search.executor
    description:
    - |
      Logs the information on various executor operations.
      Executor in this case in particular can be the one used to batch the index work.

Then in the documentation module, there's a new exec plugin execution that collects all these yamls and generates an adoc file added as an appendix:
image

To try this out .... 🙈

mvn clean install -am -pl documentation -DskipTests

Logger descriptions would need some more attention, but I thought.. I'll see to that once the list of logging categories is "finalized".

NOTE: the annotation processor generates that yaml into meta inf so that we wouldn't have troubles with Develocity, like if we'd generate a file somewhere in the root (on compile goal...)

@yrodiere, I'll keep this PR as is for now so you can have a look at it when you have time 😃.

@marko-bekhta marko-bekhta force-pushed the feat/HSEARCH-798-logging-categories branch 4 times, most recently from 3473cba to ad73b95 Compare November 20, 2024 17:28
@marko-bekhta marko-bekhta marked this pull request as ready for review November 20, 2024 17:28
@marko-bekhta marko-bekhta force-pushed the feat/HSEARCH-798-logging-categories branch 2 times, most recently from d35f6c7 to 07842c5 Compare November 22, 2024 11:57
@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Nov 22, 2024

Thanks for your pull request!

This pull request appears to follow the contribution rules.

› This message was automatically generated.

@marko-bekhta marko-bekhta force-pushed the feat/HSEARCH-798-logging-categories branch from 07842c5 to 8d9afa4 Compare November 22, 2024 11:59
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I (obviously) didn't review everything, but here are a few comments... I trust you'll pick what makes sense, ignore the rest, and will know to extrapolate to other modules :)


Regarding the YAML, I think description shouldn't be a list just text, no? I.e.:

 # List of logging categories that this module utilizes.
 org.hibernate.search.report:
   module: hibernate-search-engine
   categories:
   - name: org.hibernate.search.common.failures
-     description:
-    - |
+     description: |
       Logs unexpected problems that occur during background Hibernate Search processes.

Depending how you transform that into asciidoc, this could also get rid of the extra bullet points under "Description" in the documentation...

@marko-bekhta
Copy link
Member Author

Thanks for taking a look! Will go through your review shortly 😃 🥳

@marko-bekhta marko-bekhta force-pushed the feat/HSEARCH-798-logging-categories branch from 8d9afa4 to 5f6079c Compare November 22, 2024 22:06
Copy link

sonarcloud bot commented Nov 22, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
56.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@marko-bekhta
Copy link
Member Author

Regarding the YAML, I think description shouldn't be a list just text, no? I.e.:

The initial idea was that since a category can be shared between multiple modules each module can produce multiple descriptions for the same category. So I thought to keep it "flexible" and allow multiple descriptions from the same module (even though it doesn't make that much sense, but it was letting make it simple on the report rendering side). But now since there are "module suffixes" if the category is shared this is no longer needed. So, the report contains the description as a single text block, and the processor should fail the build if the same category is used multiple times within the same module.

I think I've addressed most of your suggestions (except the one on the interface grouping in a single parent interface since that doesn't pass the build). I'll merge this PR as there's a few files changed in it 😄 and rebasing each time requires resolving conflicts 🙈. If there are any other ideas or suggestions, I'll address them in follow-ups.
Thanks one more time for taking a look at the PR!

@marko-bekhta marko-bekhta merged commit c9c39e3 into hibernate:main Nov 25, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants