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

Allow debug logs into a writtable location #506

Open
jonesbusy opened this issue Dec 24, 2024 · 12 comments
Open

Allow debug logs into a writtable location #506

jonesbusy opened this issue Dec 24, 2024 · 12 comments
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted good first issue Good for newcomers

Comments

@jonesbusy
Copy link
Collaborator

What feature do you want to see added?

Right now log files are always created into the logs directory where the CLI is run

Is not convenient when running the CLI tool on a non-writtable location (like Kubernetes)

In addition when distributing the CLI (via Homebrew for example) it would be nice to write logs to a standard location (like /var/log)

Perhaps this could be configurable via an env var

Upstream changes

No response

Are you interested in contributing this feature?

No response

@jonesbusy jonesbusy added good first issue Good for newcomers enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted labels Dec 24, 2024
@CodexRaunak
Copy link

@jonesbusy how about if we use a hybrid approach like this
<file>${LOG_DIR:-/var/log}/logs/${filename}.log</file>
Use /var/log for Linux/macOS as a default writable location without configuring the env variable LOG_DIR, for home brew.

Use %LOCALAPPDATA%\logs (or AppData\Local) for Windows. We can specify it in the manifest file.

This will also work for kubernetes by setting the LOG_DIR to /tmp/logs or use a persistent Volume

@jonesbusy
Copy link
Collaborator Author

Sure, any solution is better than now (not configurable, logs in the same directory).

I think we should have something like /var/log/jenkins-plugin-modernizer-cli/${filename}.log

Similar to the default cache on ~/.cache/jenkins-plugin-modernizer-cli

I didn't know logback support environment variables inside logback.xml

@gounthar
Copy link
Collaborator

gounthar commented Jan 3, 2025

For /var/log, you must be admin/root to launch the tool.
I'm not so sure we would want that.

@jonesbusy
Copy link
Collaborator Author

For /var/log, you must be admin/root to launch the tool. I'm not so sure we would want that.

For sure no.

@CodexRaunak
Copy link

I see, so how about keep default value of LOG_DIR with the user writable directory, something like ~/jenkins-plugin-modernizer-cli/logs/${filename}.log

@gounthar
Copy link
Collaborator

gounthar commented Jan 3, 2025

I think something like:

<configuration>
    <appender name="FILE" class="ch.qos.logback.core.FileAppender">
        <file>${LOG_PATH:-${user.home}/.jenkins-plugin-modernizer-cli/logs}/jenkins-plugin-modernizer-cli.log</file>
        <encoder>
            <pattern>%date %level [%thread] %logger{10} [%file:%line] %msg%n</pattern>
        </encoder>
    </appender>

    <root level="info">
        <appender-ref ref="FILE" />
    </root>
</configuration>

could work.

@jonesbusy
Copy link
Collaborator Author

Perhaps (if we can make it work with logback) move the logs to ~/.cache/jenkins-plugin-modernizer-cli/<plugin>/logs/ by default. And the logs/modernizer.logs to ~/.cache/jenkins-plugin-modernizer-cli/modernizer.logs

Since the sources are stored on ~/.cache/jenkins-plugin-modernizer-cli/sources/ it would be great to have also logs close to the sources. What do you think ?

Would be possible by changing the PluginLoggerDiscriminator to something like

+return Plugin.build(marker.getName()).getLocalRepository().toAbsolutePath().toAbsolutePath();
-return marker.getName();

But this would not work as is because we only get the name of the plugin as "marker". And we will miss all the Config we cannot retrieve

Some ideas

public Marker getMarker() {
+   return MarkerFactory.getMarker(getLocalRepository().resolve("logs").resolve("invoker.logs").toAbsolutePath().toString());
-   return MarkerFactory.getMarker(name);
}

So feel free to explore the best and possible solution

@gounthar
Copy link
Collaborator

gounthar commented Jan 3, 2025

Yes, that's a good idea!

@CodexRaunak
Copy link

@jonesbusy can you help out a little.
I'm encountering an issue when running tests that involve Mockito mocking.

The JVM is unable to find the java.lang.instrument.Instrumentation class, which is required by Mockito's inline mock maker
I confirmed that I’m using JDK 21 (not JRE), and have verified it with the java -version and javac -version commands. Both return the correct JDK version. Also the JAVA_HOME and PATH is pointing to the correct jdk.

Ran mvn clean install to ensure all dependencies are properly installed, and also tried rebuilding the project with fresh dependencies. Other test cases are working fine, but the ones with mocks still fail.

image

`java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:85)
at jdk.proxy2/jdk.proxy2.$Proxy8.isTypeMockable(Unknown Source)
at org.mockito.internal.util.MockUtil.typeMockabilityOf(MockUtil.java:78)
at org.mockito.internal.util.MockCreationValidator.validateType(MockCreationValidator.java:22)
at org.mockito.internal.creation.MockSettingsImpl.validatedSettings(MockSettingsImpl.java:275)
at org.mockito.internal.creation.MockSettingsImpl.build(MockSettingsImpl.java:236)
at org.mockito.internal.MockitoCore.mock(MockitoCore.java:82)
at org.mockito.Mockito.mock(Mockito.java:2162)
at org.mockito.Mockito.mock(Mockito.java:2077)
at io.jenkins.tools.pluginmodernizer.cli.PluginLoggerDiscriminatorTest.testGetDiscriminatingValueWithMarkers(PluginLoggerDiscriminatorTest.java:37)
at java.base/java.util.ArrayList.forEach(Unknown Source)
at java.base/java.util.ArrayList.forEach(Unknown Source)
Caused by: java.lang.IllegalStateException: Internal problem occurred, please report it. Mockito is unable to load the default implementation of class that is a part of Mockito distribution. Failed to load interface org.mockito.plugins.MockMaker
at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.create(DefaultMockitoPlugins.java:105)
at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.getDefaultPlugin(DefaultMockitoPlugins.java:79)
at org.mockito.internal.configuration.plugins.PluginLoader.loadPlugin(PluginLoader.java:75)
at org.mockito.internal.configuration.plugins.PluginLoader.loadPlugin(PluginLoader.java:49)
at org.mockito.internal.configuration.plugins.PluginRegistry.(PluginRegistry.java:29)
at org.mockito.internal.configuration.plugins.Plugins.(Plugins.java:26)
at org.mockito.internal.MockitoCore.(MockitoCore.java:71)
at org.mockito.Mockito.(Mockito.java:1741)
... 4 more
Caused by: java.lang.reflect.InvocationTargetException
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Unknown Source)
at java.base/java.lang.reflect.Constructor.newInstance(Unknown Source)
at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.create(DefaultMockitoPlugins.java:103)
... 11 more
Caused by: org.mockito.exceptions.base.MockitoInitializationException:
Could not initialize inline Byte Buddy mock maker.

It appears as if you are running an incomplete JVM installation that might not support all tooling APIs
Java : 17
JVM vendor name : Azul Systems, Inc.
JVM vendor version : 17.0.12+7-LTS
JVM name : OpenJDK 64-Bit Server VM
JVM version : 17.0.12+7-LTS
JVM info : mixed mode
OS name : Windows 10
OS version : 10.0

at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.(InlineDelegateByteBuddyMockMaker.java:254)
at org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.(InlineByteBuddyMockMaker.java:23)
... 17 more
Caused by: java.lang.NoClassDefFoundError: java/lang/instrument/Instrumentation
at java.base/java.lang.Class.getDeclaredMethods0(Native Method)
at java.base/java.lang.Class.privateGetDeclaredMethods(Unknown Source)
at java.base/java.lang.Class.getMethodsRecursive(Unknown Source)
at java.base/java.lang.Class.getMethod0(Unknown Source)
at java.base/java.lang.Class.getMethod(Unknown Source)
at org.mockito.internal.PremainAttachAccess.doGetInstrumentation(PremainAttachAccess.java:66)
at org.mockito.internal.PremainAttachAccess.getInstrumentation(PremainAttachAccess.java:24)
at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.(InlineDelegateByteBuddyMockMaker.java:138)
... 18 more
Caused by: java.lang.ClassNotFoundException: java.lang.instrument.Instrumentation
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(Unknown Source)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(Unknown Source)
at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
... 26 more`

@jonesbusy
Copy link
Collaborator Author

This look specific to your VS code ? mvn clean install works right ? If yes it might be an issue with your IDE configuration.

I'm not using VSCode for Java development so not sure what would be the issue.

@CodexRaunak
Copy link

yes mvn clean install is working fine, are you using Intellij Idea?

@jonesbusy
Copy link
Collaborator Author

Yes the community edition. Test work fine running them from IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants