Skip to content

Commit 3922b57

Browse files
committed
[feature/betterlogs] Improved logs in general
1 parent 24df9e4 commit 3922b57

File tree

3 files changed

+35
-24
lines changed

3 files changed

+35
-24
lines changed

JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
package org.togetherjava.jshellapi.service;
22

33
import org.apache.tomcat.util.http.fileupload.util.Closeable;
4+
import org.slf4j.Logger;
5+
import org.slf4j.LoggerFactory;
46
import org.togetherjava.jshellapi.dto.*;
57
import org.togetherjava.jshellapi.exceptions.DockerException;
68

79
import java.io.*;
8-
import java.nio.file.Files;
9-
import java.nio.file.Path;
1010
import java.time.Duration;
1111
import java.time.Instant;
1212
import java.util.ArrayList;
1313
import java.util.List;
1414
import java.util.Optional;
1515

1616
public class JShellService implements Closeable {
17+
private static final Logger LOGGER = LoggerFactory.getLogger(JShellService.class);
1718
private final JShellSessionService sessionService;
1819
private final String id;
1920
private final BufferedWriter writer;
@@ -37,11 +38,6 @@ public JShellService(DockerService dockerService, JShellSessionService sessionSe
3738
this.evalTimeoutValidationLeeway = evalTimeoutValidationLeeway;
3839
this.lastTimeoutUpdate = Instant.now();
3940
try {
40-
Path errorLogs = Path.of("logs", "container", containerName() + ".log");
41-
if(!Files.isRegularFile(errorLogs)) {
42-
Files.createDirectories(errorLogs.getParent());
43-
Files.createFile(errorLogs);
44-
}
4541
String containerId = dockerService.spawnContainer(
4642
maxMemory,
4743
(long) Math.ceil(cpus),
@@ -198,7 +194,7 @@ public void close() {
198194
reader.close();
199195
}
200196
} catch(IOException ex) {
201-
throw new RuntimeException(ex);
197+
LOGGER.error("Unexpected error while closing.", ex);
202198
} finally {
203199
sessionService.notifyDeath(id);
204200
}

JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.togetherjava.jshellapi.service;
22

3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
35
import org.springframework.beans.factory.annotation.Autowired;
46
import org.springframework.http.HttpStatus;
57
import org.springframework.lang.Nullable;
@@ -15,6 +17,7 @@
1517

1618
@Service
1719
public class JShellSessionService {
20+
private static final Logger LOGGER = LoggerFactory.getLogger(JShellSessionService.class);
1821
private Config config;
1922
private StartupScriptsService startupScriptsService;
2023
private ScheduledExecutorService scheduler;
@@ -32,7 +35,7 @@ private void initScheduler() {
3235
try {
3336
deleteSession(id);
3437
} catch (DockerException ex) {
35-
ex.printStackTrace();
38+
LOGGER.error("Unexpected error when deleting session.", ex);
3639
}
3740
}
3841
}, config.schedulerSessionKillScanRateSeconds(), config.schedulerSessionKillScanRateSeconds(), TimeUnit.SECONDS);
@@ -41,8 +44,9 @@ void notifyDeath(String id) {
4144
JShellService shellService = jshellSessions.get(id);
4245
if(shellService == null) return;
4346
if(!shellService.isClosed()) {
44-
throw new RuntimeException("JShell Service isn't dead when it should for id " + id);
47+
throw new IllegalStateException("JShell Service isn't dead when it should for id " + id);
4548
}
49+
LOGGER.info("Session {} died.", id);
4650
jshellSessions.remove(id);
4751
}
4852

@@ -52,15 +56,15 @@ public boolean hasSession(String id) {
5256

5357
public JShellService session(String id, @Nullable StartupScriptId startupScriptId) throws DockerException {
5458
if(!hasSession(id)) {
55-
return createSession(id, config.regularSessionTimeoutSeconds(), true, config.evalTimeoutSeconds(), config.evalTimeoutValidationLeeway(), config.sysOutCharLimit(), startupScriptId);
59+
return createSession(new SessionInfo(id, true, startupScriptId, config));
5660
}
5761
return jshellSessions.get(id);
5862
}
5963
public JShellService session(@Nullable StartupScriptId startupScriptId) throws DockerException {
60-
return createSession(UUID.randomUUID().toString(), config.regularSessionTimeoutSeconds(), false, config.evalTimeoutSeconds(), config.evalTimeoutValidationLeeway(), config.sysOutCharLimit(), startupScriptId);
64+
return createSession(new SessionInfo(UUID.randomUUID().toString(), false, startupScriptId, config));
6165
}
6266
public JShellService oneTimeSession(@Nullable StartupScriptId startupScriptId) throws DockerException {
63-
return createSession(UUID.randomUUID().toString(), config.oneTimeSessionTimeoutSeconds(), false, config.evalTimeoutSeconds(), config.evalTimeoutValidationLeeway(), config.sysOutCharLimit(), startupScriptId);
67+
return createSession(new SessionInfo(UUID.randomUUID().toString(), false, startupScriptId, config));
6468
}
6569

6670
public void deleteSession(String id) throws DockerException {
@@ -69,26 +73,27 @@ public void deleteSession(String id) throws DockerException {
6973
scheduler.schedule(service::close, 500, TimeUnit.MILLISECONDS);
7074
}
7175

72-
private synchronized JShellService createSession(String id, long sessionTimeout, boolean renewable, long evalTimeout, long evalTimeoutValidationLeeway, int sysOutCharLimit, @Nullable StartupScriptId startupScriptId) throws DockerException {
73-
if(hasSession(id)) { //Just in case race condition happens just before createSession
74-
return jshellSessions.get(id);
76+
private synchronized JShellService createSession(SessionInfo sessionInfo) throws DockerException {
77+
if(hasSession(sessionInfo.id())) { //Just in case race condition happens just before createSession
78+
return jshellSessions.get(sessionInfo.id());
7579
}
80+
LOGGER.info("Creating session : {}.", sessionInfo);
7681
if(jshellSessions.size() >= config.maxAliveSessions()) {
7782
throw new ResponseStatusException(HttpStatus.TOO_MANY_REQUESTS, "Too many sessions, try again later :(.");
7883
}
7984
JShellService service = new JShellService(
8085
dockerService,
8186
this,
82-
id,
83-
sessionTimeout,
84-
renewable,
85-
evalTimeout,
86-
evalTimeoutValidationLeeway,
87-
sysOutCharLimit,
87+
sessionInfo.id(),
88+
sessionInfo.sessionTimeout(),
89+
sessionInfo.renewable(),
90+
sessionInfo.evalTimeout(),
91+
sessionInfo.evalTimeoutValidationLeeway(),
92+
sessionInfo.sysOutCharLimit(),
8893
config.dockerMaxRamMegaBytes(),
8994
config.dockerCPUsUsage(),
90-
startupScriptsService.get(startupScriptId));
91-
jshellSessions.put(id, service);
95+
startupScriptsService.get(sessionInfo.startupScriptId()));
96+
jshellSessions.put(sessionInfo.id(), service);
9297
return service;
9398
}
9499

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package org.togetherjava.jshellapi.service;
2+
3+
import org.springframework.lang.Nullable;
4+
import org.togetherjava.jshellapi.Config;
5+
6+
public record SessionInfo(String id, long sessionTimeout, boolean renewable, long evalTimeout, long evalTimeoutValidationLeeway, int sysOutCharLimit, @Nullable StartupScriptId startupScriptId) {
7+
public SessionInfo(String id, boolean renewable, StartupScriptId startupScriptId, Config config) {
8+
this(id, config.regularSessionTimeoutSeconds(), renewable, config.evalTimeoutSeconds(), config.evalTimeoutValidationLeeway(), config.sysOutCharLimit(), startupScriptId);
9+
}
10+
}

0 commit comments

Comments
 (0)