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

consider System.Logger in related hints/code-generation #8240

Open
errael opened this issue Feb 12, 2025 · 14 comments
Open

consider System.Logger in related hints/code-generation #8240

errael opened this issue Feb 12, 2025 · 14 comments
Labels
hints Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:feature A feature request

Comments

@errael
Copy link
Contributor

errael commented Feb 12, 2025

Description

This affects (the hints are under Options > Editor > Hints > Java)

  • Insert Code action/dialog, aka Generate > Logger...
  • Surround with try-catch hint, under Error Fixes hints
  • Logging hints

There might be other items that should be added to the above list.

A key question is how to specify which Logger, between System and java.util, to use. Could

  1. Add a new option somewhere; like a new tab under Tools > Options, examples
    • Tools > Options > Java > Java Logging
    • Tools > Options > Java > Java for future looking
  2. Extend list at Tools > Options > Editor > Hints > Java > Surround with try-catch
    with Use java.lang.System.Logger

Doing (2) above is the simplest. I'd be inclined to put it above java.util and have it check-marked by default. Consider that in most instances at run-time, there is little practical difference from the current behavior. These options could be used by Generate > Logger..., and in other cases as needed, to select which of the two loggers to use to initialize the logger. If this is acceptable, I volunteer.

A few additional things that could be done

A "use existing logger" option for Surround with try-catch.

For reference, here the current and proposed code.

  • Current

    • Generate>Logger...
      private static final Logger LOG = Logger.getLogger(Mavenproject5.class.getName());
    • Surround with try-catch
      Logger.getLogger(Mavenproject5.class.getName()).log(Level.SEVERE, null, ex);
  • Proposed (when option is for System.Logger)

    • Generate>Logger...
      private static final Logger LOG = System.getLogger(Mavenproject5.class.getName());

    • Surround with try-catch

      • System.getLogger(Mavenproject5.class.getName()).log(Level.ERROR, null, ex);
      • LOG.log(Level.ERROR, null, ex); // If "use existing logger" implemented

Additional Generate>Logger... options for consideration

  • specify name of generated logger, e.g. "logger" instead of "LOG"
  • expression to use to init logger, e.g. "Utils.getLogger()" vs System.getLogger(...)

With these options, a generated logger would look like

private static final Logger logger = Utils.getLogger();

Additional features

Added 2025-02-16

  • Hints that convert from JUL to System.Logger. Use with Refactor > Inspect and Transform....

Other existing NetBeans logger stuff

Added 2025-02-16

If there's missing things, LMK and I'll include them here

  • Logging hints category

    Logger declaration is not static final
    Multiple loggers
    No loggers
    String concatenation in logger
    
  • Code Templates

    log, logb, logbp, logbps, loge, logp, logr, logrb
    

Use case/motivation

Provide access to newer, as of Java-9, capability

Related issues

No response

Are you willing to submit a pull request?

Yes

@errael errael added kind:feature A feature request needs:triage Requires attention from one of the committers labels Feb 12, 2025
@mbien mbien added the hints label Feb 13, 2025
@mbien
Copy link
Member

mbien commented Feb 13, 2025

parts of this might be already customizable by templates. But I agree there could be likely out-of-the-box improvements made.

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) and removed needs:triage Requires attention from one of the committers labels Feb 13, 2025
@matthiasblaesing
Copy link
Contributor

If this is touched, other logging frameworks should also be considered. SLF4J/Log4J2/Logback/commons-logging all can be valid selections for logging. It would be good if this would be per project.

@errael
Copy link
Contributor Author

errael commented Feb 14, 2025

parts of this might be already customizable by templates. But I agree there could be likely out-of-the-box improvements made.

In addition to stuff mentioned in the OP, there is also stuff in Tools > Options > Editor > Code Templates. Is this what you're talking about?

In particular, log, logb, logbp, logbps, loge, logp, logr, logrb

These could be copied to slog, slogb, slogbps, ...

But I notice that none of these templates have/use Supplier<String>; so these templates are often a performance problem. So this probably should be revisited in general rather than copying the old ones.

Also, there's a hint Inefficient use of string concatenation in logger.
It has a fix convert string concatenation to message template.

LOG.log(Level.SEVERE, "message" + a);

is converted to

LOG.log(Level.SEVERE, "message{0}", a);

Could add an additional, probably better, fix: convert string concatenation to Supplier<String>.

LOG.log(Level.SEVERE, () -> "message" + a);

@errael
Copy link
Contributor Author

errael commented Feb 14, 2025

If this is touched, other logging frameworks should also be considered. SLF4J/Log4J2/Logback/commons-logging all can be valid selections for logging. It would be good if this would be per project.

Before Java 9 I would 100% agree. But now, doesn't System.Logger make all of these selectable at run-time rather than hard-coding?

An anecdote along these lines: There a library project I occasionally work on which used log4j2. I finally convinced the owner that System.Logger was a superior choice. Wrote some jackpot and converted the code base (with some sed assist). Nice that the original log4j config files still worked and the output looked the same.

@matthiasblaesing
Copy link
Contributor

If this is touched, other logging frameworks should also be considered. SLF4J/Log4J2/Logback/commons-logging all can be valid selections for logging. It would be good if this would be per project.

Before Java 9 I would 100% agree. But now, doesn't System.Logger make all of these selectable at run-time rather than hard-coding?

SLF4J existed for how long? Sorry, but I see https://xkcd.com/927/ here. Just as the module system of java faces an uphill battle, the same is true for logging systems.

I have projects where the framework mandates logback, a CMS that mandates SLF4J and has legacy use of log4j, local projects that use JUL. Only the latter could be changed, all others have to integrate with their corresponding ecosystem.

@mbien
Copy link
Member

mbien commented Feb 14, 2025

In addition to stuff mentioned in the OP, there is also stuff in Tools > Options > Editor > Code Templates. Is this what you're talking about?

yes, although I don't know how many are visible through that window. Things like #3458 etc are usually all solved with templates.

  1. Extend list at Tools > Options > Editor > Hints > Java > Surround with try-catch
    with Use java.lang.System.Logger

yep, updating this hint would be a good approach (e.g making it more configurable), hint settings can also optionally be per-project -> this solves multiple issues at once.

The hint can also not simply look at the classpath and try to "do the right thing", since a typical dusty enterprise project has one logging facade but logging bridges to 5+ logging impls - every second lib can use a different impl. So everything might be in the classpath. It likely has to be a user choice set somewhere.

@errael
Copy link
Contributor Author

errael commented Feb 14, 2025

My limited experience, with anything other than JUL, is with a library that was developed as part of much larger application. The app used log4j2 and it was used by the library. We can probably all agree that a library should use a facade and not a specific logger. Another selling point of using System.Logger is reducing the library's depency on 3rd party artifacts. I understand that projects have requirements for style/libraries/logging, but I don't understand:

Only the latter [JUL] could be changed, all others have to integrate with their corresponding ecosystem.

After changing the library (in the example) to System.Logger, and adding the log4j2 bridge to the main app, it integrated fine into the existing ecosystem without any changes to the log4j2 configuration files and produced identical log output.

But that's all beside the point.

A facility to add/register arbitrary 3rd party logging frameworks/facades and their code generation templates/actions and a UI to specify which one to use when NB generates logging code doesn't sound like a bad idea. The most popular logging methods would be provided as reference implementations.

It may be that extending existing capability with JDK's System.Logger is not worth it because of the possibility of re-architecting how NB generates logging.

@mbien
Copy link
Member

mbien commented Feb 15, 2025

We can probably all agree that a library should use a facade and not a specific logger.

sure but in practice it doesn't really matter. For every logging lib a bridge lib exists by now. Logging is a getter with something what holds the log level fields. This can be (luckily) bridged to any other logging impl without having to convince lib authors to use dedicated facades.

The project I am looking at right now uses: jcl-over-slf4j, log4-to-slf4j, jul-to-slf4j and and eclipse logger bridge which maps to slf4j. The "impl" is just yet another bridge, pretending to be a slf4j impl which logs directly to JFR.

A facility to add/register arbitrary 3rd party logging frameworks/facades..

I wouldn't make this too extensible. Luckily the java community decided at some point to stop developing more logging libs. "Generate -> logger" could offer a list. The hint would also need a few more options.

@errael
Copy link
Contributor Author

errael commented Feb 16, 2025

IIUC, JUL can not be a bridge source; so currently NB does not support generating code that can be a logging bridge source. Adding System.Logger allows generating code that can bridge to the well known loggers. This is in some sense a bug fix, at least in the past NB supported new Java features as they came out. In addition to what's been discussed, a set of hints that converts from JUL to System.Logger could be useful; use with Refactor > Inspect and Transform....

If NB want to support code generation for 3rd party logging, IMHO anything less than an extensible infrastructure misses the mark. Different projects probably have different standards. I wouldn't be surprised if different projects that use the same backend logger have different standards for how the logging is used. I wonder how many projects are around that still use custom logging implementations. In NB, there is not only code generation, but additional hints that look for standards/style violations (I've added a list from NB of things I'm aware to the end of the OP). Each logger, or logger style, that is added should specify not only code generation, but also standard checks and rewrites for use with existing hints. Additional and/or custom logger hints should also be considered that are useful for enterprise projects. Might want to register a set of hints that convert from one logger framework to another. It may be that per project specification is too coarse grained (thinking about some of the previous comments in this thread); and so a set of roots should be specified for a given logger/style.

It took me a little while but I finally remember that NetBeans has layer.xml, supplemented later with annotatations. Extension points are defined for registration. I can't imagine why this wouldn't be used for specifying 3rd party loggers/styles.

@mbien
Copy link
Member

mbien commented Feb 16, 2025

IIUC, JUL can not be a bridge source;

    <dependencies>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>jul-to-slf4j</artifactId>
            <version>2.0.16</version>
        </dependency>
        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-slf4j-impl</artifactId>
            <version>2.24.3</version>
        </dependency>
    </dependencies>

If NB want to support code generation for 3rd party logging,

then it can offer more options for the code gen actions - sure.

@errael
Copy link
Contributor Author

errael commented Feb 16, 2025

IIUC, JUL can not be a bridge source;

<dependencies>
    ...

Whoa, wow, yeah, OK. That works in many situations where a "sole JUL handler" is OK.

Thanks.

then it can offer more options for the code gen actions - sure.

Making 3rd party logging a "first class" option in NB needs more that just code gen.

@neilcsmith-net
Copy link
Member

I think System.Logger should be the default option where we can use it, in preference to JUL. It has the benefit of being in java.base.

errael added a commit to errael/netbeans that referenced this issue Feb 16, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
@errael
Copy link
Contributor Author

errael commented Feb 17, 2025

Opened #8253, DRAFT.

Forgot to mention that the PR is fully functional. (at least that's the idea). It's DRAFT while looking for answers to make sure corner cases are correctly handled.

errael added a commit to errael/netbeans that referenced this issue Feb 18, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
errael added a commit to errael/netbeans that referenced this issue Feb 18, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
@errael
Copy link
Contributor Author

errael commented Feb 18, 2025

Ready for review...

errael added a commit to errael/netbeans that referenced this issue Feb 19, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
errael added a commit to errael/netbeans that referenced this issue Feb 19, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
errael added a commit to errael/netbeans that referenced this issue Feb 20, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
errael added a commit to errael/netbeans that referenced this issue Feb 20, 2025
This default is also used by `Generate > Logger...`.
Addresses some issues in raised in apache#8240.
New feature `Use existing declared Logger`.
Fix "Logging" hints to handle System.Logger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hints Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:feature A feature request
Projects
None yet
Development

No branches or pull requests

4 participants