Skip to content

Commit

Permalink
NH-95070: address codeQl stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
cleverchuk committed Dec 2, 2024
1 parent d834d43 commit 43a5b60
Show file tree
Hide file tree
Showing 17 changed files with 56 additions and 236 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,66 +16,13 @@

public class ReporterFactory {
private static final Logger logger = LoggerFactory.getLogger();
private String tracelyzerHost = Constants.XTR_UDP_HOST;
private int tracelyzerPort = Constants.XTR_UDP_PORT;
private String datagramLocalAddress;
private Integer datagramLocalPort;

private static final String OPENSHIFT_TRACEVIEW_TLYZER_IP = "OPENSHIFT_TRACEVIEW_TLYZER_IP";
private static final String OPENSHIFT_TRACEVIEW_TLYZER_PORT = "OPENSHIFT_TRACEVIEW_TLYZER_PORT";
private static final String OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_IP = "OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_IP";
private static final String OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_PORT = "OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_PORT";

private static final String TRACELYTICS_UDPADDR = "TRACELYTICS_UDPADDR";
private static final String TRACELYTICS_UDPPORT = "TRACELYTICS_UDPPORT";

private static UDPReporter singletonUdpReporter;
private static final Object singletonUdpReporterLock = new Object();

@Getter(lazy = true)
private static final ReporterFactory instance = new ReporterFactory();

private ReporterFactory() {
Map<String, String> env = System.getenv();

if (env.containsKey(TRACELYTICS_UDPADDR)) {
tracelyzerHost = env.get(TRACELYTICS_UDPADDR);
logger.info("Setting Reporter to contact Tracelyzer host on [" + tracelyzerHost + "]");
}
if (env.containsKey(TRACELYTICS_UDPPORT)) {
tracelyzerPort = Integer.parseInt(env.get(TRACELYTICS_UDPPORT));
logger.info("Setting Reporter to contact Tracelyzer port on [" + tracelyzerPort + "]");
}

//open shift check
if (env.containsKey(OPENSHIFT_TRACEVIEW_TLYZER_IP)) {
tracelyzerHost = env.get(OPENSHIFT_TRACEVIEW_TLYZER_IP);
logger.info("Running in OpenShift environment. Setting Reporter to contact Tracelyzer host on [" + tracelyzerHost + "]");
}
if (env.containsKey(OPENSHIFT_TRACEVIEW_TLYZER_PORT)) {
tracelyzerPort = Integer.parseInt(env.get(OPENSHIFT_TRACEVIEW_TLYZER_PORT));
logger.info("Running in OpenShift environment. Setting Reporter to contact Tracelyzer port on [" + tracelyzerPort + "]");
}
if (env.containsKey(OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_IP)) {
datagramLocalAddress = env.get(OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_IP);
logger.info("Running in OpenShift environment. Setting Reporter datagram port to [" + datagramLocalAddress + "]");
}
if (env.containsKey(OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_PORT)) {
datagramLocalPort = Integer.parseInt(env.get(OPENSHIFT_TRACEVIEW_JAVA_DATAGRAM_PORT));
logger.info("Running in OpenShift environment. Setting Reporter datagram port to [" + datagramLocalPort + "]");
}

}

/**
* Builds a {@link UDPReporter}. Take note that this might create a singleton if the system has restrictions on UDP bind address/port
*
* @return
* @throws IOException
*/
public UDPReporter createUdpReporter() throws IOException {
return createUdpReporter(tracelyzerHost, tracelyzerPort);
}

/**
* Builds a {@link UDPReporter}. Take note that this might create a singleton if the system has restrictions on UDP bind address/port, in such an
Expand All @@ -91,22 +38,7 @@ UDPReporter createUdpReporter(String host, Integer port) throws IOException {
logger.error("Cannot build UDPReporter. Host and/or port params are null!");
return null;
}

logger.debug("Building UPD Reporter with host [" + host + "] and port [" + port + "]");

if (datagramLocalAddress != null && datagramLocalPort != null) {
//if using specific address and port, then we should only allow singleton; otherwise multiple UDP reporters will try to bind to the same address/port
synchronized (singletonUdpReporterLock) {
if (singletonUdpReporter == null) {
logger.debug("UPD Reporter specified datagram to bind on [" + datagramLocalAddress + ":" + datagramLocalPort + "]");
singletonUdpReporter = new UDPReporter(host, port, datagramLocalAddress, datagramLocalPort);
}
}

return singletonUdpReporter;
} else {
return new UDPReporter(host, port);
}
return new UDPReporter(host, port);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public void run() {
PrintStream timeIntervalLog = null;
PrintStream movingWindowLog = null;
PrintStream histogramPercentileLog = System.out;
Double firstStartTime = 0.0;
double firstStartTime = 0.0;
boolean timeIntervalLogLegendWritten = false;
boolean movingWindowLogLegendWritten = false;

Expand Down Expand Up @@ -442,9 +442,12 @@ public void run() {
config.percentilesOutputTicksPerHalf, config.outputValueUnitRatio, config.logFormatCsv);
}
} finally {
if (config.outputFileName != null) {
if (timeIntervalLog != null) {
timeIntervalLog.close();
histogramPercentileLog.close();
}

if (movingWindowLog != null) {
movingWindowLog.close();
}
}
}
Expand Down
14 changes: 4 additions & 10 deletions core/src/main/java/com/solarwinds/joboe/core/rpc/RpcSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import com.solarwinds.joboe.logging.Logger;
import com.solarwinds.joboe.logging.LoggerFactory;
import com.solarwinds.joboe.sampling.Settings;
import com.solarwinds.joboe.sampling.SettingsArg;
import com.solarwinds.joboe.sampling.TraceDecisionUtil;

Expand All @@ -21,12 +22,11 @@ public class RpcSettings extends com.solarwinds.joboe.sampling.Settings {
private final short flags; // required
private final long timestamp; // required, in millsec
private final long value; // required
private final String layer; // required
private final long ttl; //time to live this settings record
private final Map<SettingsArg<?>, Object> args = new HashMap<SettingsArg<?>, Object>(); //other arguments

public RpcSettings(short type, String stringFlags, long timestamp, long value, long ttl, String layer, Map<String, ByteBuffer> args) {
this.type = type;
public RpcSettings(String stringFlags, long timestamp, long value, long ttl, Map<String, ByteBuffer> args) {
this.type = Settings.OBOE_SETTINGS_TYPE_DEFAULT_SAMPLE_RATE;
this.flags = convertFlagsFromStringToShort(stringFlags);
this.timestamp = timestamp;
if (value < 0) {
Expand All @@ -39,7 +39,6 @@ public RpcSettings(short type, String stringFlags, long timestamp, long value, l
this.value = value;
}
this.ttl = ttl;
this.layer = layer;
readArgs(args);
}

Expand All @@ -54,7 +53,6 @@ public RpcSettings(RpcSettings source, long timestamp) {
this.timestamp = timestamp; //take the new timestamp
this.value = source.value;
this.ttl = source.ttl;
this.layer = source.layer;
this.args.putAll(source.args);
}

Expand Down Expand Up @@ -117,11 +115,7 @@ public short getFlags() {
return flags;
}

@Override
public String getLayer() {
return layer;
}


@Override
public long getTtl() {
return ttl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,6 @@
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class SettingsUtil {

public static Map<String, Settings> transformToKVSetting(SettingsResult settingsResult){
Map<String, Settings> updatedSettings = new LinkedHashMap<String, Settings>();
for (Settings settingsForLayer : settingsResult.getSettings()) {
LoggerFactory.getLogger().debug("Got settings from collector: " + settingsForLayer);
updatedSettings.put(settingsForLayer.getLayer(), settingsForLayer);
}
return updatedSettings;
}

public static SettingsResult transformToLocalSettings(Collector.SettingsResult result){
List<Settings> settings = new ArrayList<>();
if (result.getResult() == Collector.ResultCode.OK) {
Expand All @@ -44,12 +35,10 @@ public static Settings convertSetting(Collector.OboeSetting grpcOboeSetting) {
}

return new RpcSettings(
convertType(grpcOboeSetting.getType()),
grpcOboeSetting.getFlags().toStringUtf8(),
System.currentTimeMillis(), //use local timestamp for now, as it is easier to compare ttl with it
grpcOboeSetting.getValue(),
grpcOboeSetting.getTtl(),
grpcOboeSetting.getLayer().toStringUtf8(),
convertedArguments);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,8 @@ public short getType() {
return settingsType;
}

@Override
public String getLayer() {
// TODO Auto-generated method stub
return null;
}

@Override
public long getTtl() {
// TODO Auto-generated method stub
return 0;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@ public short getFlags() {
return TracingMode.ALWAYS.toFlags();
}

@Override
public String getLayer() {
return "";
}

@Override
public long getTtl() {
return Integer.MAX_VALUE; //don't use long, otherwise it might overflow...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,6 @@ static void setup() {
tested = ReporterFactory.getInstance();
}

@Test
public void testbuildDefaultUdpReporter() throws Exception {
UDPReporter reporter = tested.createUdpReporter();

Field addressField = reporter.getClass().getDeclaredField("addr");
addressField.setAccessible(true);

Field portField = reporter.getClass().getDeclaredField("port");
portField.setAccessible(true);

InetAddress address = (InetAddress) addressField.get(reporter);
assertEquals(InetAddress.getByName(Constants.XTR_UDP_HOST), address);
assertEquals(Constants.XTR_UDP_PORT, portField.get(reporter));
}

@Test
public void testbuildNonDefaultUdpReporter() throws Exception {
UDPReporter reporter = tested.createUdpReporter("localhost", 9999);
Expand Down
73 changes: 0 additions & 73 deletions core/src/test/java/com/solarwinds/joboe/core/TraceTest.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ protected interface TestCollector {

protected abstract TestCollector startCollector(int port) throws IOException;
protected abstract TestCollector startRedirectCollector(int port, String redirectArg) throws IOException;
protected abstract TestCollector startRatedCollector(int port, int processingTimePerMessage, ResultCode limitExceededCode) throws IOException;
protected abstract TestCollector startBiasedTestCollector(int port, Map<TaskType, ResultCode> taskToResponseCode) throws IOException;
protected abstract TestCollector startRatedCollector(int port, ResultCode limitExceededCode) throws IOException;
protected abstract TestCollector startBiasedTestCollector(int port) throws IOException;
//Test server that throws Runtime exception on every other message
protected abstract TestCollector startErroneousTestCollector(int port, double errorPercentage) throws IOException;
protected abstract TestCollector startSoftDisabledTestCollector(int port, String warning) throws IOException;
protected abstract void startSoftDisabledTestCollector(int port, String warning) throws IOException;

protected static String getServerPublicKeyLocation() {
return TEST_SERVER_CERT_LOCATION;
Expand All @@ -101,7 +101,7 @@ private static List<RpcSettings> generateTestSettings() {
arguments.put(SettingsArg.BUCKET_CAPACITY.getKey(), SettingsArg.BUCKET_CAPACITY.toByteBuffer(32.0));
arguments.put(SettingsArg.BUCKET_RATE.getKey(), SettingsArg.BUCKET_RATE.toByteBuffer(2.0));

settings.add(new RpcSettings(RpcSettings.OBOE_SETTINGS_TYPE_DEFAULT_SAMPLE_RATE, PollingSettingsFetcherTest.DEFAULT_FLAGS_STRING, TimeUtils.getTimestampMicroSeconds(), 1000000, 600, "test-layer", arguments));
settings.add(new RpcSettings(PollingSettingsFetcherTest.DEFAULT_FLAGS_STRING, TimeUtils.getTimestampMicroSeconds(), 1000000, 600, arguments));
return settings;
}

Expand Down Expand Up @@ -274,7 +274,6 @@ public void testGetSettings() throws Exception {

com.solarwinds.joboe.core.rpc.SettingsResult result = client.getSettings("", null).get();
assertEquals(com.solarwinds.joboe.core.rpc.ResultCode.OK, result.getResultCode());

assertEquals(TEST_SETTINGS.size(), result.getSettings().size());

for (int i = 0 ; i < TEST_SETTINGS.size(); i ++) {
Expand All @@ -284,8 +283,6 @@ public void testGetSettings() throws Exception {

short expectedFlags = expectedSetting.getFlags();
assertEquals(expectedFlags, receivedSetting.getFlags());
assertEquals(expectedSetting.getLayer(), receivedSetting.getLayer());
//assertEquals(expectedSetting.getTimestamp(), receivedSetting.getTimestamp()); timestamp for now we will use the machine's current timestamp instead of the incoming one as TTL is tricky (otherwise we would have to keep 2 timestamps...)
assertEquals(expectedSetting.getValue(), receivedSetting.getValue());
assertEquals(expectedSetting.getArgValue(SettingsArg.BUCKET_CAPACITY), receivedSetting.getArgValue(SettingsArg.BUCKET_CAPACITY), 0);
assertEquals(expectedSetting.getArgValue(SettingsArg.BUCKET_RATE), receivedSetting.getArgValue(SettingsArg.BUCKET_RATE), 0);
Expand Down Expand Up @@ -591,7 +588,7 @@ public void testTryLater() throws Exception {
final int TIME_PER_EVENT = 10;
int tryLaterPort = locateAvailablePort();

TestCollector tryLaterCollector = startRatedCollector(tryLaterPort, TIME_PER_EVENT, ResultCode.TRY_LATER);
TestCollector tryLaterCollector = startRatedCollector(tryLaterPort, ResultCode.TRY_LATER);
Client client = null;

try {
Expand Down Expand Up @@ -629,7 +626,7 @@ public void testLimitExceed() throws Exception {
final int TIME_PER_EVENT = 10;
int tryLaterPort = locateAvailablePort();

TestCollector tryLaterServer = startRatedCollector(tryLaterPort, TIME_PER_EVENT, ResultCode.LIMIT_EXCEEDED);
TestCollector tryLaterServer = startRatedCollector(tryLaterPort, ResultCode.LIMIT_EXCEEDED);
Client client = null;

try {
Expand Down Expand Up @@ -798,7 +795,7 @@ public void testBiasedServer() throws Exception {
System.out.println("running testBiasedServer");
int biasedServerPort = locateAvailablePort();

TestCollector basiedServer = startBiasedTestCollector(biasedServerPort, Collections.singletonMap(TaskType.POST_METRICS, ResultCode.TRY_LATER));
TestCollector basiedServer = startBiasedTestCollector(biasedServerPort);
Client client = new RpcClient(TEST_SERVER_HOST, biasedServerPort, TEST_CLIENT_ID, getProtocolClientFactory(new File(getServerPublicKeyLocation()).toURI().toURL()));
assertThrows(TimeoutException.class, () -> client.postMetrics(new ArrayList<Map<String,Object>>(), null).get(5, TimeUnit.SECONDS),"Not expecting to return any result for this call!"); //this is supposed to get held up because of TRY_LAYER)

Expand Down
Loading

0 comments on commit 43a5b60

Please sign in to comment.