-
Notifications
You must be signed in to change notification settings - Fork 51
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
Fix automatic APM instrumentation for JS tracer. Add diagnostic logging to APM configurators. Update APM instrumentation docs #388
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package org.datadog.jenkins.plugins.datadog.apm; | ||
|
||
import hudson.FilePath; | ||
import hudson.model.Node; | ||
import hudson.model.TaskListener; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
final class JavascriptConfigurator implements TracerConfigurator { | ||
|
||
private static final int GET_NPM_VERSION_TIMEOUT_MILLIS = 30_000; | ||
private static final int INSTALL_TRACER_TIMEOUT_MILLIS = 300_000; | ||
|
||
@Override | ||
public Map<String, String> configure(DatadogTracerJobProperty<?> tracerConfig, Node node, FilePath workspacePath, Map<String, String> envs, TaskListener listener) throws Exception { | ||
String nodeVersion = workspacePath.act(new ShellCommandCallable(Collections.emptyMap(), GET_NPM_VERSION_TIMEOUT_MILLIS, "npm", "-v")); | ||
listener.getLogger().println("[datadog] Configuring DD JS tracer: got npm version " + nodeVersion + " from " + workspacePath + " on " + node); | ||
|
||
FilePath datadogPath = workspacePath.child(".datadog"); | ||
datadogPath.mkdirs(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know if the plugin has permissions to create folders? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has, and we only do things inside the job's workspace |
||
|
||
// set location for installing global packages (Jenkins agent user may not have the permissions to write to the default one) | ||
Path absoluteDatadogPath = Paths.get(datadogPath.absolutize().getRemote()); | ||
Map<String, String> environment = Collections.singletonMap("NPM_CONFIG_PREFIX", absoluteDatadogPath.toString()); | ||
|
||
// we install dd-trace as a "global" package | ||
// (otherwise, doing SCM checkout might rollback the changes to package.json, and any subsequent `npm install` calls will result in removing the package) | ||
String installTracerOutput = workspacePath.act(new ShellCommandCallable(environment, INSTALL_TRACER_TIMEOUT_MILLIS, "npm", "install", "-g", "dd-trace")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we have permissions issues? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "global" package naming is a bit misleading here - what this means is we are not installing it inside the repo (because subsequent SCM checkout would overwrite this) and instead we install it inside the We could have a permissions issue if the OS user that runs the Jenkins agent is not allowed to execute |
||
Path tracerPath = absoluteDatadogPath.resolve("lib/node_modules/dd-trace"); | ||
listener.getLogger().println("[datadog] Configuring DD JS tracer: tracer installed in " + tracerPath + " on " + node + "; output: " + installTracerOutput); | ||
|
||
Map<String, String> variables = new HashMap<>(); | ||
variables.put("DD_TRACE_PATH", tracerPath.toString()); | ||
variables.put("NODE_OPTIONS", PropertyUtils.prepend(envs, "NODE_OPTIONS", "-r $DD_TRACE_PATH/ci/init")); | ||
return variables; | ||
} | ||
} |
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.
If
npm
is not installed, would this return an exception?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.
Yep, it'll throw an exception that will be caught and logged in the calling class