-
Notifications
You must be signed in to change notification settings - Fork 199
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
Agent findbugs fixes #945
Agent findbugs fixes #945
Conversation
@@ -120,11 +120,14 @@ private static void appendJarsToBootstrapClassLoader(Instrumentation inst) throw | |||
|
|||
String agentJarName = null; | |||
File agentFolder = new File(agentJarLocation); | |||
for (File file : agentFolder.listFiles()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
listFiles()
returns null if not a directory. The path should be a directory, but we don't verify so I'll let this fall through
@@ -131,7 +131,7 @@ public AgentConfiguration parseConfigurationFile(String baseFolder) { | |||
setBuiltInInstrumentation(agentConfiguration, instrumentationTag); | |||
|
|||
NodeList addClasses = getAllClassesToInstrument(instrumentationTag); | |||
if (addClasses == null) { | |||
if (addClasses.getLength() == 0) { // can't be null; if empty, return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Element.getElementsByTagName
returns in empty NodeList
in place of null
@@ -242,7 +238,7 @@ private void setBuiltInInstrumentation(AgentConfigurationDefaultImpl agentConfig | |||
nodes = builtInElement.getElementsByTagName(MAX_STATEMENT_QUERY_LIMIT_TAG); | |||
builtInConfigurationBuilder.setSqlMaxQueryLimitInMS(XmlParserUtils.getLong(XmlParserUtils.getFirst(nodes), MAX_STATEMENT_QUERY_LIMIT_TAG)); | |||
|
|||
new BuiltInInstrumentedClassesBuilder().setSimpleBuiltInClasses(builtInConfigurationBuilder, builtInElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is static
@@ -429,7 +421,7 @@ private void addMethods(ClassInstrumentationData classData, Element eClassNode) | |||
valueStr = methodElement.getAttribute(THRESHOLD_ATTRIBUTE); | |||
if (!StringUtils.isNullOrEmpty(valueStr)) { | |||
try { | |||
thresholdInMS = Long.valueOf(valueStr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary unbox rebox
return Long.parseLong(strValue); | ||
} catch (NumberFormatException nfe) { | ||
InternalLogger.INSTANCE.warn("Invalid attribute value. Expected number but was '%s'", strValue); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (unbox/rebox), but we need a try/catch
2451bb6
to
25a4484
Compare
No description provided.