-
Notifications
You must be signed in to change notification settings - Fork 620
feat: otel javaagent #1189
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
base: 11.1
Are you sure you want to change the base?
feat: otel javaagent #1189
Conversation
commands.add( | ||
installationDir + "core/*:" + installationDir + "plugin-interface/*:" + installationDir + "ee/*"); | ||
if (javaagentEnabled) { | ||
commands.add("-javaagent:\"" + installationDir + "/agent/opentelemetry-javaagent.jar\""); |
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.
The path for the Java agent on non-Windows systems should not include escaped quotes. Please update to:
commands.add("-javaagent:" + installationDir + "/agent/opentelemetry-javaagent.jar");
The quotes are only needed for Windows paths due to potential spaces in directory names, but on Unix-like systems they can cause the JVM to incorrectly interpret the path.
commands.add("-javaagent:\"" + installationDir + "/agent/opentelemetry-javaagent.jar\""); | |
if (Utils.isWindows()) { | |
commands.add("-javaagent:\"" + installationDir + "/agent/opentelemetry-javaagent.jar\""); | |
} else { | |
commands.add("-javaagent:" + installationDir + "/agent/opentelemetry-javaagent.jar"); | |
} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Map<String, String> methodArguments = new HashMap<>(); | ||
for (Object argument : joinPoint.getArgs()) { | ||
methodArguments.put(argument.getClass().getCanonicalName(), String.valueOf(argument)); | ||
} | ||
span.setAttribute("method.arguments", methodArguments.keySet().stream().map(key -> key + ": " + methodArguments.get(key)) | ||
.collect(Collectors.joining(", ", "{", "}"))); |
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.
The code as written could throw a NullPointerException
if any argument in joinPoint.getArgs()
is null. Consider adding null checks before accessing argument.getClass()
:
Map<String, String> methodArguments = new HashMap<>();
for (Object argument : joinPoint.getArgs()) {
if (argument != null) {
methodArguments.put(argument.getClass().getCanonicalName(), String.valueOf(argument));
} else {
methodArguments.put("null", "null");
}
}
This ensures the code handles null arguments gracefully while still capturing all method parameters for telemetry.
Map<String, String> methodArguments = new HashMap<>(); | |
for (Object argument : joinPoint.getArgs()) { | |
methodArguments.put(argument.getClass().getCanonicalName(), String.valueOf(argument)); | |
} | |
span.setAttribute("method.arguments", methodArguments.keySet().stream().map(key -> key + ": " + methodArguments.get(key)) | |
.collect(Collectors.joining(", ", "{", "}"))); | |
Map<String, String> methodArguments = new HashMap<>(); | |
for (Object argument : joinPoint.getArgs()) { | |
if (argument != null) { | |
methodArguments.put(argument.getClass().getCanonicalName(), String.valueOf(argument)); | |
} else { | |
methodArguments.put("null", "null"); | |
} | |
} | |
span.setAttribute("method.arguments", methodArguments.keySet().stream().map(key -> key + ": " + methodArguments.get(key)) | |
.collect(Collectors.joining(", ", "{", "}"))); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your
changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here
highlighting the necessary changes)
Checklist for important updates
coreDriverInterfaceSupported.json
file has been updated (if needed)pluginInterfaceSupported.json
file has been updated (if needed)build.gradle
getPaidFeatureStats
function in FeatureFlag.java filebuild.gradle
, please make sure to add themin
implementationDependencies.json
.getValidFields
inio/supertokens/config/CoreConfig.java
if new aliases were added for any coreconfig (similar to the
access_token_signing_key_update_interval
config alias).git tag
) in the formatvX.Y.Z
, and then find thelatest branch (
git branch --all
) whoseX.Y
is greater than the latest released tag.app_id_to_user_id
table, make sure to delete from this table when deletingthe user as well if
deleteUserIdMappingToo
is false.Remaining TODOs for this PR