Add a Datadog build step#88
Conversation
a5f7360 to
55453e8
Compare
FlorianVeaux
added
changelog/Added
sarah-witt
left a comment
sarah-witt
left a comment
There was a problem hiding this comment.
Looks good so far! I'll manually test it soon. There should definitely be docs for this, though (they can be in a separate PR if it's a lot to add).
| } | ||
| } | ||
| invoker.start(); | ||
| return false; |
There was a problem hiding this comment.
What is the return value used for here?
There was a problem hiding this comment.
That step is performing work that is not yet done here
| ExtensionList.clearLegacyInstances(); | ||
| cfg.setCollectBuildLogs(false); | ||
| EnvVars env = new EnvVars(); | ||
| env.put("DD_TEST", "bar"); |
There was a problem hiding this comment.
What is this for? Shouldn't it not be needed for the integration test?
There was a problem hiding this comment.
Yes, can be removed totally. I was hesitating between putting the env var here or in the pipeline definition.
| }; | ||
| stubClient.assertMetric("jenkins.job.duration", hostname, expectedTags); | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you add a test for multiple tags and setting collectLogs: false?
There was a problem hiding this comment.
Extending the current one instead if that's ok
| import org.kohsuke.stapler.DataBoundSetter; | ||
|
|
||
| /** | ||
| * Pipeline plug-in step for recording time-stamps. |
There was a problem hiding this comment.
This doesn't seem accurate
sarah-witt
left a comment
sarah-witt
left a comment
There was a problem hiding this comment.
Looks great! One scenario I tested was creating a pipeline with multiple stages and adding new tags to only one stage, making sure only that stage had the new tags, which could be a good unit test.
|
Is there any documentation on how to use this? I tried copying the examples from the unit tests but I get Happy to open an issue if that's the right course of action here. |
|
Hey @stpierre |
3 participants
Adding a pipeline compatible build steps in order to: