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

TS/31571 Send log to Teamscale via a custom Appender #518

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agent/src/dist/logging/logback.debug.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration>
<define name="defaultLogDir" class="com.teamscale.jacoco.agent.util.LogDirectoryPropertyDefiner" />
<define name="defaultLogDir" class="com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner" />

<appender name="CONSOLE" class="shadow.ch.qos.logback.core.ConsoleAppender">
<encoder>
Expand Down
2 changes: 1 addition & 1 deletion agent/src/dist/logging/logback.rolling-file.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration>
<define name="defaultLogDir" class="com.teamscale.jacoco.agent.util.LogDirectoryPropertyDefiner"/>
<define name="defaultLogDir" class="com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner"/>

<appender name="RollingFile" class="shadow.ch.qos.logback.core.rolling.RollingFileAppender">
<!-- TODO: adjust the path where the log file should be written -->
Expand Down
2 changes: 1 addition & 1 deletion agent/src/main/java/com/teamscale/jacoco/agent/Agent.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import java.util.stream.Stream;

import static com.teamscale.jacoco.agent.upload.teamscale.TeamscaleUploader.RETRY_UPLOAD_FILE_SUFFIX;
import static com.teamscale.jacoco.agent.util.LoggingUtils.wrap;
import static com.teamscale.jacoco.agent.logging.LoggingUtils.wrap;

/**
* A wrapper around the JaCoCo Java agent that automatically triggers a dump and XML conversion based on a time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.teamscale.client.ProxySystemProperties;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.conqat.lib.commons.filesystem.FileSystemUtils;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
Expand Down
2 changes: 1 addition & 1 deletion agent/src/main/java/com/teamscale/jacoco/agent/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.teamscale.jacoco.agent.commandline.Validator;
import com.teamscale.jacoco.agent.convert.ConvertCommand;
import com.teamscale.jacoco.agent.util.AgentUtils;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.conqat.lib.commons.string.StringUtils;
import org.jacoco.core.JaCoCo;
import org.slf4j.Logger;
Expand Down
15 changes: 12 additions & 3 deletions agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.teamscale.jacoco.agent;

import com.teamscale.client.HttpUtils;
import com.teamscale.client.TeamscaleServer;

Check warning on line 4 in agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java#L4

Unused import: `com.teamscale.client.TeamscaleServer` https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=C48DC6E5D1ACFE39B84A16AD31440951
Copy link

Choose a reason for hiding this comment

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

Remove unused import.

The import com.teamscale.client.TeamscaleServer is not used in the file.

-	import com.teamscale.client.TeamscaleServer;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import com.teamscale.client.TeamscaleServer;
Tools
GitHub Check: teamscale-findings

[warning] 4-4: agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java#L4
Unused import: com.teamscale.client.TeamscaleServer
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=C48DC6E5D1ACFE39B84A16AD31440951

import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException;
import com.teamscale.jacoco.agent.logging.LogToTeamscaleAppender;
import com.teamscale.jacoco.agent.options.AgentOptionParseException;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.options.AgentOptionsParser;
Expand All @@ -11,10 +13,11 @@
import com.teamscale.jacoco.agent.options.TeamscalePropertiesUtils;
import com.teamscale.jacoco.agent.testimpact.TestwiseCoverageAgent;
import com.teamscale.jacoco.agent.upload.UploaderException;
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;

Check warning on line 16 in agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java#L16

Unused import: `com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig` https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=E09766D42C397B0AAAD6C0E9AE6A8365
Copy link

Choose a reason for hiding this comment

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

Remove unused import.

The import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig is not used in the file.

-	import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
Tools
GitHub Check: teamscale-findings

[warning] 16-16: agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java#L16
Unused import: com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=E09766D42C397B0AAAD6C0E9AE6A8365

import com.teamscale.jacoco.agent.util.AgentUtils;
import com.teamscale.jacoco.agent.util.DebugLogDirectoryPropertyDefiner;
import com.teamscale.jacoco.agent.util.LogDirectoryPropertyDefiner;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.DebugLogDirectoryPropertyDefiner;
import com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.conqat.lib.commons.collections.CollectionUtils;
import org.conqat.lib.commons.filesystem.FileSystemUtils;
import org.conqat.lib.commons.string.StringUtils;
Expand All @@ -31,6 +34,8 @@
import java.util.List;
import java.util.Optional;

import static com.teamscale.jacoco.agent.logging.LoggingUtils.getLoggerContext;

/** Container class for the premain entry point for the agent. */
public class PreMain {

Expand Down Expand Up @@ -149,6 +154,10 @@
loggingResources = LoggingUtils.initializeLogging(agentOptions.getLoggingConfig());
logger.info("Logging to " + new LogDirectoryPropertyDefiner().getPropertyValue());
}

if (agentOptions.getTeamscaleServerOptions().isConfiguredForServerConnection()) {
LogToTeamscaleAppender.addTeamscaleAppenderTo(getLoggerContext(), agentOptions);
}
}

/** Closes the opened logging contexts. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.teamscale.client.CommitDescriptor;
import com.teamscale.client.TeamscaleServer;
import com.teamscale.jacoco.agent.testimpact.TestwiseCoverageAgent;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.model.RevisionInfo;
import org.conqat.lib.commons.string.StringUtils;
import org.slf4j.Logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.teamscale.jacoco.agent.options.ProjectAndCommit;
import com.teamscale.jacoco.agent.upload.teamscale.DelayedTeamscaleMultiProjectUploader;
import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.jetbrains.annotations.VisibleForTesting;
import org.slf4j.Logger;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.teamscale.jacoco.agent.commit_resolution.git_properties;

import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.util.ClasspathWildcardIncludeFilter;
import org.conqat.lib.commons.collections.Pair;
import org.conqat.lib.commons.string.StringUtils;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.teamscale.jacoco.agent.upload.delay.DelayedUploader;
import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.slf4j.Logger;

import java.io.File;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.teamscale.client.CommitDescriptor;
import com.teamscale.jacoco.agent.options.sapnwdi.DelayedSapNwdiMultiUploader;
import com.teamscale.jacoco.agent.options.sapnwdi.SapNwdiApplication;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.util.ClasspathWildcardIncludeFilter;
import org.conqat.lib.commons.string.StringUtils;
import org.slf4j.Logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.teamscale.client.ProfilerRegistration;
import com.teamscale.client.TeamscaleServiceGenerator;
import com.teamscale.jacoco.agent.options.AgentOptionParseException;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.util.ILogger;
import okhttp3.HttpUrl;
import okhttp3.ResponseBody;
Expand Down Expand Up @@ -131,5 +131,7 @@ private void unregisterProfiler() {
}
}


public String getProfilerId() {
return profilerId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ private String getHostName() {
}
}


/**
* Returns a string that <i>probably</i> contains the PID.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.teamscale.client.TestDetails;
import com.teamscale.jacoco.agent.options.AgentOptionParseException;
import com.teamscale.jacoco.agent.util.Benchmark;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.ReportUtils;
import com.teamscale.report.jacoco.EmptyReportException;
import com.teamscale.report.jacoco.JaCoCoXmlReportGenerator;
Expand All @@ -26,7 +26,7 @@
import java.nio.file.Paths;
import java.util.List;

import static com.teamscale.jacoco.agent.util.LoggingUtils.wrap;
import static com.teamscale.jacoco.agent.logging.LoggingUtils.wrap;

/** Converts one .exec binary coverage file to XML. */
public class Converter {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.teamscale.jacoco.agent.util;
package com.teamscale.jacoco.agent.logging;

import java.nio.file.Path;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.teamscale.jacoco.agent.util;
package com.teamscale.jacoco.agent.logging;

import ch.qos.logback.core.PropertyDefinerBase;
import com.teamscale.jacoco.agent.util.AgentUtils;

import java.nio.file.Path;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package com.teamscale.jacoco.agent.logging;

import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.AppenderBase;
import com.teamscale.client.ProfilerLogEntry;
import com.teamscale.client.TeamscaleClient;
import com.teamscale.jacoco.agent.options.AgentOptions;
import retrofit2.Call;

import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

Check warning on line 15 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L15

Star import of `java.util.concurrent.*` should not be used https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=37AC472955602FF3B639815C2FEAECFF
Copy link

Choose a reason for hiding this comment

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

Avoid star imports.

Star import of java.util.concurrent.* should not be used. Import only the necessary classes to improve readability and maintainability.

- import java.util.concurrent.*;
+ import java.util.concurrent.Executors;
+ import java.util.concurrent.ScheduledExecutorService;
+ import java.util.concurrent.TimeUnit;
+ import java.util.concurrent.CompletableFuture;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import java.util.concurrent.*;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.CompletableFuture;
Tools
GitHub Check: teamscale-findings

[warning] 15-15: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L15
Star import of java.util.concurrent.* should not be used
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=37AC472955602FF3B639815C2FEAECFF


public class LogToTeamscaleAppender extends AppenderBase<ILoggingEvent> {

Check warning on line 17 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L17

Interface comment missing https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=D63084C6B2A4A673F56C9102501F76C1
Copy link

Choose a reason for hiding this comment

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

Add class-level Javadoc.

The class LogToTeamscaleAppender is missing a class-level Javadoc comment. Adding documentation helps in understanding the purpose and usage of the class.

/**
 * Custom appender that sends logs to Teamscale.
 */
public class LogToTeamscaleAppender extends AppenderBase<ILoggingEvent> {
Tools
GitHub Check: teamscale-findings

[warning] 17-17: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L17
Interface comment missing
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=D63084C6B2A4A673F56C9102501F76C1


private String profilerId;
private TeamscaleClient teamscaleClient;
private int batchSize = 10;
private Duration flushInterval = Duration.ofSeconds(3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is flushInterval and batchSize something that should be configurable or is it OK to hardcode?

private final List<ProfilerLogEntry> logBuffer = new ArrayList<>();
private final ScheduledExecutorService scheduler;

public LogToTeamscaleAppender() {
this.scheduler = Executors.newScheduledThreadPool(1, r -> {
// Make the thread a daemon so that it does not prevent the JVM from terminating.
Thread t = Executors.defaultThreadFactory().newThread(r);
t.setDaemon(true);
return t;
});
}

public void setTeamscaleClient(TeamscaleClient teamscaleClient) {
this.teamscaleClient = teamscaleClient;
}

public void setProfilerId(String profilerId) {
this.profilerId = profilerId;
}

public void setBatchSize(int batchSize) {
this.batchSize = batchSize;
}

public void setFlushInterval(Duration flushInterval) {
this.flushInterval = flushInterval;
}

@Override
public void start() {
super.start();
scheduler.scheduleAtFixedRate(this::flush, flushInterval.toMillis(), flushInterval.toMillis(), TimeUnit.MILLISECONDS);
}

@Override
protected void append(ILoggingEvent eventObject) {
synchronized (logBuffer) {
logBuffer.add(formatLog(eventObject));
if (logBuffer.size() >= batchSize) {
flush();
}
}
}

private ProfilerLogEntry formatLog(ILoggingEvent eventObject) {
long timestamp = eventObject.getTimeStamp();
String message = eventObject.getFormattedMessage();
String severity = eventObject.getLevel().toString();
return new ProfilerLogEntry(timestamp, message, severity);
}

private void flush() {
List<ProfilerLogEntry> logsToSend;
synchronized (logBuffer) {
if (logBuffer.isEmpty()) {
return;
}
logsToSend = new ArrayList<>(logBuffer);
logBuffer.clear();
}
sendLogs(logsToSend);
}

private void sendLogs(List<ProfilerLogEntry> logs) {
CompletableFuture.runAsync(() -> {
try {
Call<Void> call = teamscaleClient.service.postProfilerLog(profilerId, logs);
retrofit2.Response<Void> response = call.execute();
if (!response.isSuccessful()) {
throw new RuntimeException("Failed to send log: HTTP error code : " + response.code());

Check failure on line 92 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L92

Throw of generic exception RuntimeException https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=5CB34F16E944F6F3A9411E26B3C29CE4
Copy link

Choose a reason for hiding this comment

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

Avoid throwing generic exceptions.

Throwing generic exceptions like RuntimeException should be avoided. Use specific exceptions to provide more context about the error.

- throw new RuntimeException("Failed to send log: HTTP error code : " + response.code());
+ throw new TeamscaleLogException("Failed to send log: HTTP error code : " + response.code());

Add a custom exception class TeamscaleLogException:

public class TeamscaleLogException extends Exception {
    public TeamscaleLogException(String message) {
        super(message);
    }
}
Tools
GitHub Check: teamscale-findings

[failure] 92-92: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L92
Throw of generic exception RuntimeException
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=5CB34F16E944F6F3A9411E26B3C29CE4

}
} catch (Exception e) {
e.printStackTrace(); // Handle exceptions appropriately in production code

Check warning on line 95 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L95

The `printStackTrace()` method should not be called https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=F2573AF30253FB566038844D28E137D4
}

Check failure on line 96 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L94-L96

Catch clause catches generic exception `Exception` https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=904768600913FA54D8D1A30491F4A438
Comment on lines +94 to +96
Copy link

Choose a reason for hiding this comment

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

Avoid catching generic exceptions.

Catching generic exceptions like Exception should be avoided. Catch specific exceptions to handle different error cases appropriately.

- } catch (Exception e) {
+ } catch (IOException e) {

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: teamscale-findings

[failure] 94-96: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L94-L96
Catch clause catches generic exception Exception
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=904768600913FA54D8D1A30491F4A438


[warning] 95-95: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L95
The printStackTrace() method should not be called
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=F2573AF30253FB566038844D28E137D4

});

Check warning on line 97 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L87-L97

The `printStackTrace()` method should not be called https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=29937ABB61FC34E3405E25D9F7F83D6F
Comment on lines +87 to +97
Copy link

Choose a reason for hiding this comment

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

Avoid using printStackTrace() for exception handling.

The printStackTrace() method should not be called. Use proper logging mechanisms to handle exceptions.

- e.printStackTrace(); // Handle exceptions appropriately in production code
+ logger.error("Failed to send logs to Teamscale: " + e.getMessage(), e);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CompletableFuture.runAsync(() -> {
try {
Call<Void> call = teamscaleClient.service.postProfilerLog(profilerId, logs);
retrofit2.Response<Void> response = call.execute();
if (!response.isSuccessful()) {
throw new RuntimeException("Failed to send log: HTTP error code : " + response.code());
}
} catch (Exception e) {
e.printStackTrace(); // Handle exceptions appropriately in production code
}
});
CompletableFuture.runAsync(() -> {
try {
Call<Void> call = teamscaleClient.service.postProfilerLog(profilerId, logs);
retrofit2.Response<Void> response = call.execute();
if (!response.isSuccessful()) {
throw new RuntimeException("Failed to send log: HTTP error code : " + response.code());
}
} catch (Exception e) {
logger.error("Failed to send logs to Teamscale: " + e.getMessage(), e);
}
});
Tools
GitHub Check: teamscale-findings

[warning] 87-97: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L87-L97
The printStackTrace() method should not be called
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=29937ABB61FC34E3405E25D9F7F83D6F


[failure] 92-92: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L92
Throw of generic exception RuntimeException
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=5CB34F16E944F6F3A9411E26B3C29CE4


[failure] 94-96: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L94-L96
Catch clause catches generic exception Exception
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=904768600913FA54D8D1A30491F4A438


[warning] 95-95: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L95
The printStackTrace() method should not be called
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=F2573AF30253FB566038844D28E137D4

}

@Override
public void stop() {
scheduler.shutdown();
try {
if (!scheduler.awaitTermination(5, TimeUnit.SECONDS)) {
scheduler.shutdownNow();
}
} catch (InterruptedException e) {
scheduler.shutdownNow();
}
flush(); // Ensure remaining logs are sent
super.stop();
}


public static void addTeamscaleAppenderTo(LoggerContext context, AgentOptions agentOptions) {

Check warning on line 115 in agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L115

Interface comment missing https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=B124499941D52D84AF24B3034B19C0F9
Copy link

Choose a reason for hiding this comment

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

Add method-level Javadoc.

The method addTeamscaleAppenderTo is missing a method-level Javadoc comment. Adding documentation helps in understanding the purpose and usage of the method.

/**
 * Adds the `LogToTeamscaleAppender` to the specified logger context.
 *
 * @param context The logger context.
 * @param agentOptions The agent options containing Teamscale configuration.
 */
public static void addTeamscaleAppenderTo(LoggerContext context, AgentOptions agentOptions) {
Tools
GitHub Check: teamscale-findings

[warning] 115-115: agent/src/main/java/com/teamscale/jacoco/agent/logging/LogToTeamscaleAppender.java#L115
Interface comment missing
https://cqse.teamscale.io/findings/details/teamscale-jacoco-agent?t=ts%2F31571_send_log_to_teamscale_via_appender%3AHEAD&id=B124499941D52D84AF24B3034B19C0F9

LogToTeamscaleAppender logToTeamscaleAppender = new LogToTeamscaleAppender();
logToTeamscaleAppender.setContext(context);
logToTeamscaleAppender.setProfilerId(agentOptions.configurationViaTeamscale.getProfilerId());
logToTeamscaleAppender.setTeamscaleClient(agentOptions.createTeamscaleClient());
logToTeamscaleAppender.start();

Logger rootLogger = context.getLogger(Logger.ROOT_LOGGER_NAME);
rootLogger.addAppender(logToTeamscaleAppender);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
| Copyright (c) 2009-2018 CQSE GmbH |
| |
+-------------------------------------------------------------------------*/
package com.teamscale.jacoco.agent.util;
package com.teamscale.jacoco.agent.logging;

import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.joran.JoranConfigurator;
import ch.qos.logback.core.joran.spi.JoranException;
import ch.qos.logback.core.util.StatusPrinter;
import com.teamscale.jacoco.agent.Agent;
import com.teamscale.jacoco.agent.util.NullOutputStream;
import com.teamscale.report.util.ILogger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -51,7 +52,10 @@ public static LoggingResources initializeDefaultLogging() {
return new LoggingResources();
}

private static LoggerContext getLoggerContext() {
/**
* Returns the logger context.
*/
public static LoggerContext getLoggerContext() {
return (LoggerContext) LoggerFactory.getILoggerFactory();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.teamscale.jacoco.agent.options;

import com.teamscale.jacoco.agent.util.AgentUtils;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import org.slf4j.Logger;

import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
import com.teamscale.report.testwise.model.TestExecution;
import com.teamscale.report.testwise.model.TestInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import com.teamscale.client.PrioritizableTestCluster;
import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
import com.teamscale.report.testwise.model.TestExecution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.jacoco.dump.Dump;
import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
import com.teamscale.report.testwise.model.TestExecution;
import com.teamscale.report.testwise.model.TestInfo;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.teamscale.jacoco.agent.testimpact;

import com.teamscale.client.JsonUtils;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.model.TestExecution;
import org.slf4j.Logger;

Expand Down
Loading
Loading