-
Improvement
-
Resolution: Fixed
-
Major
-
None
-
Platform: PC, OS: Linux
-
Powered by SuggestiMate
It would be good to know what a build triggered. When a user triggered it via
the web-site I would like to know who it was (that requires authentication).
Other build reasons: a succeeded build of another project, a wget request by a
script, a cron trigger.
- patch.291.feb8
- 20 kB
- patch.291.feb5
- 19 kB
- patch.issue291.4feb
- 17 kB
- patch.291
- 15 kB
- is blocking
-
JENKINS-2898 Ability to change ant target depending on build trigger
-
- Open
-
-
JENKINS-324 Allow a description to be set when triggering remote build
-
- Closed
-
-
JENKINS-1628 Add logged-in username to batch-task environment vars
-
- Closed
-
-
JENKINS-2914 Audit Trail Plugin does not record schedule/poll-scm triggered builds
-
- Closed
-
-
JENKINS-3166 Token listing all triggers of a build
-
- Closed
-
- is duplicated by
-
JENKINS-1124 Information about what triggered the build
-
- Closed
-
-
JENKINS-2123 Ehancement: record trigger events
-
- Closed
-
-
JENKINS-2522 Display user name with each build
-
- Closed
-
[JENKINS-291] a record needed why a build has started (what triggered it)
+1
I really miss in Hudson (CruiseControl has this feature, whew, how about flame
bait!!!) the ability to see why a build happened. Was it because of an SCM poll,
an upstream project build (if so, which one), a timed build, or a manual build
(if so, by who)?
The trigger record should contain a short reason and an optional longer command
output (or description). The command output is interesting if a job is triggered
by a SCM, and the user is wondering why the SCM triggered it. (See discussion
http://www.nabble.com/Hudson-executes-build-even-though-no-SCM-changes-were-recorded-td18736261.html)
I'm taking a look at this. I think the best approach is to modify
BuildableItem, and change the method 'boolean scheduleBuild()' to take an
argument explaining why the build is being scheduled, then fix all callers to
include an explanation, and add a UI element to show the reason.
I have two questions though. One is how backward compatibility affects this. I
think I would have to leave the old no-argument method in place, and have it
give 'unknown' or 'legacy code' reason for launching the build. However, if
it's possible to remove the old method, I would prefer to do that.
The second question is what to store in the job itself. The boolean return
value indicates whether the job is already in the queue. This means we could
have several reasons for a job being built - the easy solution would be to
record only the first reason, but the more complete solution would be to queue
up all the reasons on the job, so they can all be browsed. I think I'll start
with the easy route, and see about adding the list later.
My second question is larger than I had realized. Browsing the code this
evening, I'm seeing Task and Item objects as part of the queue manipulation
code. On the model side, there are Job, Run, Project and Executor objects.
This is more than I was considering, so I'd like to understand better the best
place to store the reason for launching a job/project. I think the 'build
reason' needs to end up on the Run object, but that object is not created until
well after the project has been enqueued, so a series of storage places need to
be identified.
What you need is a class like ParameterizedProjectTask, that extends from
QueueTaskFilter and works as a decorator to the newly created 'Executable'
(which in this case is AbstractBuild.
By implementing the hashCode and equals without taking the explanation into
account, you can avoid stacking up builds in the queue (although that wouldn't
allow you to do merging of the items — for that we need more work in the core.)
Given that the parameterized build is in the core, too, I think
ParameterizedProjectTask should be renamed and expanded to support everything we
need to carry from the build scheduling to the actual build, including
parameters, description, initiator, etc.
I have working code for this, doing pretty much exactly what mdonahue described
(w/o even reading this first, whoa). I'll review kohsuke's comment tomorrow and
see how that might work.
Nice. I did get Kohsuke's comment from Nov 17 implemented, but never finished
the UI part.
I'm just writing "Build triggered by ..." in the job console output when the
build starts, isn't that sufficient?
Please attach a patch with your work, I'd be interested to see it... I looked a
bit at ParameterizedProjectTask and it seemed like too many hoops to jump
through to get the Task created everywhere a build can be scheduled.
Here's where I'm at:
Outermost entry point: BuildableItem.. I don't see any alternative to adding a
"triggeredBy" parameter here and deprecating the old methods. I've added
convenience methods in Trigger (passing getDescriptor().getDisplayName() as
triggeredBy) so this is pretty transparent for build triggers.
Next level in: Queue/Queue.Task.. I started by also adding triggeredBy parameter
in Queue.add(), but now I've refactored as kohsuke suggested, using
ParameterizedProjectTask to hold the triggeredBy value (I didn't actually rename
the class to something more generic yet).
Listener: Added ItemListener.onScheduled(Queue.Task) and Queue.add() invokes
this for each listener. Hudson registers one listener that remembers
triggeredBy values for scheduled tasks, then writes the info into the console
output when the build actually starts.
Code changed in hudson
User: : mindless
Path:
trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java
trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java
trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java
trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java
trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java
trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java
trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java
trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java
trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java
trunk/hudson/main/core/src/main/java/hudson/model/Queue.java
trunk/hudson/main/core/src/main/java/hudson/model/listeners/ItemListener.java
trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/Trigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/TriggerListener.java
trunk/hudson/plugins/jabber/src/main/java/hudson/plugins/jabber/im/transport/bot/BuildCommand.java
trunk/hudson/plugins/maven1-snapshot-plugin/src/main/java/hudson/plugins/mavensnapshottrigger/MavenSnapshotTrigger.java
trunk/hudson/plugins/schedule-failed-builds/src/main/java/com/progress/hudson/ScheduleFailedBuildsTrigger.java
trunk/hudson/plugins/url-change-trigger/src/main/java/com/redfin/hudson/UrlChangeTrigger.java
trunk/www/changelog.html
http://fisheye4.cenqua.com/changelog/hudson/?cs=14823
Log:
[FIXED JENKINS-291] Add new ItemListener.onScheduled(Queue.Task) interface method
and invoke this from Queue.add(). Hudson registers a listener to write what
triggered each build into the console output.
BuildableItem.scheduleBuild() now takes triggeredBy parameter,
ParameterizedProjectTask stores triggeredBy value for scheduled tasks,
and Trigger has scheduleBuild convenience methods to use its descriptor's
displayName as the triggeredBy value.
I can't really agree with how this is implemented...
Mostly I think it is wrong to use a String to represent triggeredBy.
We should have some structured information here (say a TriggerReason or BuildCause interface). This
could contain as little as a string description, but build triggers that have more information can
create their own implementations (e.g. containing a full User object instead of just "User foo").
Furthermore this can be logged permanently as an Action in the Run so it is available in the remote
API and can be used for display in the views for the Run.
Logging the TriggerReason to the build output is still possible, but can be delegated to the
TriggerReason itself.
Since changing this after a release has implications for the persistence of queues, I hope this can
be fixed before the next Hudson release, or this patch (temporarily) reverted.
There are some further minor issues:
- why is the triggeredBy for a run in a matrix build just "Matrix" and not the triggeredBy for the
containing matrix run ? - the TriggerListener is a little weird. If the TriggerReason is attached to the build ,you can just
add this log line in Run.run() where you have access to all information.
A little more work would be to try to decompose the ParameterizedProjectTask again. Just a thought:
how about if you could pass in
a list of QueueTaskFilters (e.g. one with parameters and another with the TriggerReason) when you
start a task. Then you can let the queue compose them and figure out if the build is already
scheduled,
and the QueueTaskFilters don't have to know about each other.
I think I agree with Tom. A structure would be nice instead of just String.
I also wonder if we should just extend Queue.Item to remember 'triggeredBy'
information, rather than bypassing this via ParameterizedProjectTask. The way
this change stands now, it makes it pointless for AbstractProject to extend from
Queue.Task.
My suggestion is to revert this change in the trunk, create a branch, apply the
diff over there, and do some massaging.
bummer, a simple string tells all I was interested in. matrix and maven just
say "Matrix" and "Maven" mostly because I'm not familiar with those areas..
weaving through all the interfaces, abstract classes and deprecated methods in
hudson.model took long enough.. and now I'm afraid I'm out of time to work on
this one, good luck..
Oh, I stopped getting email updates once I was no longer the assignee, which is
too bad. I've just resolved my work against the latest SVN, so should be able
to attach a patch soon. I created a Cause object hierarchy, with subtypes
LegacyCodeCause, UpstreamCause, TriggerCause, UserCause.
These get dropped into a CauseAction that is attached to runs.
All that went into hudson.model, which seems like a big package right now.
Created an attachment (id=537)
Add a build cause, showing why each build was triggered
One issue I ran into is that using the parameterized objects means some of the
UI elements show the project as parameterized. So that has to be tweaked as well.
super-quick review:
- TriggerCause doesn't distinguish among different triggers
- UserCause could have a no-param constructor, defaulting to
Hudson.getAuthentication (or should UserCause store User object and default to
User.current()?) - should base Cause class be abstract?
Created an attachment (id=539)
Updated patch to track why a build started
The new patch adds a summary block to each job's view, so this change finally
has a way of viewing the cause of a build. I'm using some generic orange square
with a gear icon. I'd like something better, but have no candidates right now.
I also took Alan's (mindless) suggestions into account. Cause is now abstract,
and TriggerCause was replaced by SCMTriggerCause and TimerTriggerCause. I
don't have any fields in those cause objects, since I don't know how to extract
the appropriate information from Hudson. But it would be nice to include the
polling log for the SCMTriggerCause object, and to include the Cron line that
caused the trigger to start in the TimerTriggerCause object. Fortunately that
can be added later with little pain.
I no longer store an Authentication object, since that seemed to pull in a lot
more than I was expecting. Instead, I'm just storing the username.
I'd like to commit this in the next few days, so I'll solicit feedback on the
developers list, and see where it goes.
good progress, thanks. I have some more comments:
- you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok?
or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be
added? - stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so
one might find out where that legacy code is, but not sure what use it has if
private and has no get method (unless maybe you go look at xml file where it is
saved?) - regarding SCM and Timer cause classes.. so I guess the design will require
every trigger plugin to implement a cause class? (ok I guess, just checking)
Not sure if it would work, but would a TriggerCause containing a Trigger object
be possible? Otherwise, Hudson uses a lot of inner classes, so maybe those 2
could be inside the corresponding Trigger classes rather than more hudson.model
classes. Regarding access to the cron schedule info, look at fields in the
parent Trigger class. - javadoc in base Cause class?
- javadoc on deprecated methods? (with @deprecated since
{version}
and what to
use instead) - kohsuke's comment on my code still applies: "The way this change stands now,
it makes it pointless for AbstractProject to extend from Queue.Task." - Still seems like UserCause could have a no param constructor for convenience..
User object (User.current()) vs simple username? - The original ParameterizedProjectTask constructor is removed.. should it be
kept (and deprecated) for compatibility? - Comment for ParameterizedProjectTask.hashCode:
// cause is NOT included so different causes won't schedule duplicate builds
Comment for ParameterizedProjectTask.equals:
// cause is NOT checked so different causes won't schedule duplicate builds - TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause..
won't that be a behavior change? (now using the default quiet period) - should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml
api? - updates for existing plugins?
- there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated
- you removed scheduleBuild(int quietPeriod) from BuildableItem.. is this ok?
or should it be deprecated, and a scheduleBuild(int quietPeriod, Cause cause) be
added?
I somehow misdirected the merge. I build the initial patch back in November of
2008, and the quietPeriod method was added later.
- stackTrace in LegacyCodeCause is inaccessible.. I guess you put that there so
one might find out where that legacy code is, but not sure what use it has if
private and has no get method (unless maybe you go look at xml file where it is
saved?)
Yeah, the stack trace is really only useful for development purposes. A plugin
developer can take a look at the serialized XML to figure out where they're
using the deprecated method, and provide their own cause.
- regarding SCM and Timer cause classes.. so I guess the design will require
every trigger plugin to implement a cause class? (ok I guess, just checking)
It seems like it. We could put in a TextCause object, which just holds a string
explanation, for use with simpler plugins. In general, it seems preferable to
have an object model representation of the cause though.
- Not sure if it would work, but would a TriggerCause containing a Trigger
object be possible? Otherwise, Hudson uses a lot of inner classes, so maybe
those 2 could be inside the corresponding Trigger classes rather than more
hudson.model classes. Regarding access to the cron schedule info, look at
fields in the parent Trigger class.
It's certainly possible to put the trigger object into the TriggerCause, but it
will probably lose it's relative link when serialized out to XML. If we go the
inner class route, I'd prefer to put all the core Cause classes inside the Cause
model object.
- javadoc in base Cause class?
OK
- javadoc on deprecated methods? (with @deprecated since
{version}
and what to
use instead)
OK
- kohsuke's comment on my code still applies: "The way this change stands now,
it makes it pointless for AbstractProject to extend from Queue.Task."
I'm going with Kohsuke's original suggestion to extend the
ParameterizedProjectTask. Others have pointed out that having the object model
right is more important than having a perfect code path, since the code can be
updated later with low cost, compared to updating the object model. I'm going
to stick with the current design on this.
- Still seems like UserCause could have a no param constructor for convenience..
User object (User.current()) vs simple username?
OK
- The original ParameterizedProjectTask constructor is removed.. should it be
kept (and deprecated) for compatibility?
Grepping the plugins folder finds one plugin that uses this constructor - jbpm,
so I'll re-add it.
- Comment for ParameterizedProjectTask.hashCode:
// cause is NOT included so different causes won't schedule duplicate builds
Comment for ParameterizedProjectTask.equals:
// cause is NOT checked so different causes won't schedule duplicate builds
OK
- TimerTrigger used to pass zero for quietPeriod, now it just passes a Cause..
won't that be a behavior change? (now using the default quiet period)
Good catch. I must have resolved the conflict incorrectly here.
- should CauseAction and/or Cause use @ExportedBean,etc to add cause info in xml
api?
I'm not familiar with the @ExportedBean annotation, and I haven't used the xml
api. Should the annotation go on CauseAction or Cause?
- updates for existing plugins?
That's what the LegacyCodeCause is supposed to help facilitate. I haven't
evaluated how many plugins make use of the now deprecated build code, but even
once those changes are made, it seems a bit rude to release the plugins. I was
planning to let the natural support, upgrade and build process take care of this.
- there are 2 scheduleBuild calls in maven.MavenModuleSetBuild to be updated
The downstream one is easy to fix. The other one I'm not sure, so I treated it
as another downstream project from teh MavenModuleSetBuild.
wow, fast work.. i'll take a look at the latest tomorrow, but sounds like it
should be getting quite close.
- I didn't mean to suggest releasing all the affected plugins, but just checking
in the changes so their next releases (whenever they happen) will have this
change. But I'm fine with whatever the usual process for plugins is here.. (you
can look at the file list for r14823 above for the plugins I found)
- Still don't follow on the Queue.Task thing.. I too followed Kohsuke's
suggestion, so it ends up with AbstractProject (which is a Queue.Task) never
being passed directly to Queue.add(). Ok with me.. we'll see if anyone else
comments on this part.
- I'm not too sure on the @ExportedBean details either, I'd have to play with
it.. try {job}/build/##/api/xml on a job. Since CauseAction is in the build, I
think that should have @ExportedBean, then put @Exported on getCause()?
something like that..
looks great..
could save a couple lines in ParameterizedProjectTask's deprecated constructor
by calling this(project, parameters, new Cause.LegacyCodeCause());
something to test:
set the quiet period high enough that you'll see jobs waiting in the queue
before the build actually starts... what happens when you click on an item here?
I think previously it would go to that job (project itself is in the queue),
but when wrapped in ParameterizedProjectTask, does the link work?
Looks good...
One detail: UpstreamCause.upstreamCause is stored but never used. If you need it
you can just ask the upstream build.
These could be done later:
- add "AbstractBuild UpstreamCause.getBuild()" and "User UserCause.getUser()".
This will get mapped by stapler so you could do /job/XYZ/1/cause/build. I'm sure
somebody will have a use for this. - @ExportedBean goes onto classes you want in the xml api (i.e. your concrete
cause classes), and @Exported onto their getters (which should be added too)
I simplified the deprecated constructor as suggested for ParameterizedProjectTask
Regarding the quiet period, I induced a change while setting the quiet period to
120 seconds. I don't see an accessible build page via the UI, so I don't think
this is an issue - the job isn't accessible until the quiet period expired.
Regarding the upstream cause, I wanted to store the chain of reasons for a given
build in the build itself. That way conflicting expiration policies won't
prevent the user from understanding the derivation of an existing job.
For example, if the upstream job expires builds quickly, you will lose the
causal chain on the downstream jobs.
This information is not presently exposed via the UI, but I think the current UI
for this feature is pretty primitive, and will get more rich as others use it
and think of better mechanisms for presentation.
I'll add the XML annotations and then commit this patch.
Code changed in hudson
User: : mdonohue
Path:
trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixBuild.java
trunk/hudson/main/core/src/main/java/hudson/matrix/MatrixConfiguration.java
trunk/hudson/main/core/src/main/java/hudson/maven/MavenBuild.java
trunk/hudson/main/core/src/main/java/hudson/maven/MavenModuleSetBuild.java
trunk/hudson/main/core/src/main/java/hudson/model/AbstractProject.java
trunk/hudson/main/core/src/main/java/hudson/model/BuildableItem.java
trunk/hudson/main/core/src/main/java/hudson/model/Cause.java
trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java
trunk/hudson/main/core/src/main/java/hudson/model/ParameterizedProjectTask.java
trunk/hudson/main/core/src/main/java/hudson/model/ParametersDefinitionProperty.java
trunk/hudson/main/core/src/main/java/hudson/tasks/BuildTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
trunk/hudson/main/core/src/main/resources/hudson/model/CauseAction/summary.jelly
http://fisheye4.cenqua.com/changelog/hudson/?cs=15150
Log:
Fix JENKINS-291
We now add a CauseAction to each job, and a summary.jelly file which renders
the cause on any job with an explanation.
Created an attachment (id=546)
Final patch - this is now committed as revision 15150
this should be mentioned in changelog.html, and check the unit tests for new
breakages..
Code changed in hudson
User: : mindless
Path:
trunk/hudson/main/core/src/main/java/hudson/model/Cause.java
trunk/hudson/main/core/src/main/java/hudson/model/CauseAction.java
trunk/hudson/main/core/src/main/java/hudson/triggers/SCMTrigger.java
trunk/hudson/main/core/src/main/java/hudson/triggers/TimerTrigger.java
http://fisheye4.cenqua.com/changelog/hudson/?cs=15237
Log:
JENKINS-291 Export CauseAction so exported Causes can be accessed via /api/.
Export base Cause class and its short description instead of on every subclass.
This is awesome! Thank you very much for doing this. However, it would be much
nicer if it displayed upstream builds as a link to that particular build rather
than just a textual description.
> it would be much nicer if it displayed upstream builds as a link
> to that particular build rather than just a textual description.
This would best be handled by a new enhancement request, or becoming a
contributor yourself.
Yes, being able to identify what triggered a build would be very useful.