Bug 41913

Created attachment 19763 [details]
Suggested patch

The suggested patch makes TransactionController deliver samples that contains
all the samples that are part of the transaction, as subresults of the
transaction sample.
The transaction sample is only marked as successfull if all of the subsamples
are successfull. The transaction sample also contains the sum of number of
bytes for all the subsamples.
The threadname of the sample from the TransactionController is also the same as
that of the subsamples, I did not understand why the thread name of the
controller is used in the current version of the TransactionController.

I have also changed the ViewResultsFullVisualizer, so that is shows the
assertions that failed for subresults as well, and the assertions are now added
after the subrestults in the tree.

So this patch makes it easy to see if a transaction was a success or failure,
and if it failed, it is easy to see which sample failed, and if it was an
assertion which failed, you also easily see that.

If someone knows how to get a visualizer to only view results from the same
level in the tree as the visualizer is placed, I would be happy. I cannot think
of a way that is currently possible. The visualizer always seems to view the
samples from the current level and lower levels.

Good idea, and the View Results Tree patch works for me, but the Transaction 
Controller patch does not seem to do what I expected.

The child samples of the Transaction Controller now appear twice in the View 
Results Tree. I think this needs to be fixed before the patch is committed.

If I have the following test plan :

Threadgroup
--Transaction Controller
----Request A
----Request B
----Request C

then I currently see the following in the "View Result Tree" after sampling
Request A
Request B
Request C
Transaction Controller
--Request A
--Request B
--Request C

Is that what you are seeing ?

The point is that the "Transaction Controller" is making a "fake" sample, and
adding all the "real" samples of the transaction as the sub results of the
"fake" transaction sample.

Do you only want the "Transaction sample" sent to the visualizers ? I'm not sure
if that is possible to do.
But as I think I said earlier, I would want one option to display only the
"transaction sample" in the visualizers, and not all the "real samples". I'm not
sure how to do it. I was thinking about telling a visualizer to only "accept"
samples from controllers on the same level as the visualizer in the three. But I
found no way of doing that.

Let me know what you think.

(In reply to comment #3)
[...]

> Is that what you are seeing ?

Yes

> The point is that the "Transaction Controller" is making a "fake" sample, and
> adding all the "real" samples of the transaction as the sub results of the
> "fake" transaction sample.

At present it seems to be adding copies of the samples; the samples need to 
appear only as children of the TC. 

C.f. the HTTP Sampler when it retrieves embedded resources.

(In reply to comment #4)
Is this what you want as the samples shown :
Transaction Controller
--Request A
--Request B
--Request C

The HTTP Request Sampler has an easier job, because that is creating one Sample,
and has full control over what is put into that sample.
The Transaction Controller gets it's subsamples via an event, since it is a
SampleListener. I doubt, at least I do not know how, that the Transaction
Controller can prevent the "real" Samples from appearing at other
SampleListeners. With events and listeners, you can never assume who will
receive the event first.

I'm not sure if I understand you comments. Could you try to explain a bit more ?

(In reply to comment #5)
One think which would be easy, is to add an option to the "Transaction
Controller GUI" along the lines of a checkbox with text "Transaction sample
contains sub results".
If this checkbox is true, then you would get test results like this :
Request A
Request B
Request C
Transaction Controller
--Request A
--Request B
--Request C

And if the checkbox is false, you would get
Request A
Request B
Request C
Transaction Controller


I would like to suggest that the checkbox is true by default.
I see a number of sitations where it would be useful to have the samples which
are part of the transaction as a sub result. For example to add an assertion to
the transaction, to check that all the expected subsamples are part of the
transaction.
If you don't want to see the subsamples twice in some vizualisers, you unselect
this option.

It could be an option, but if so should _not_ be on by default.

Otherwise it will be confusing to users (it confused me), and may generate a 
lot of questions on the mailing lists...

Created attachment 19777 [details]
Updated patch

This is an updated patch, which adds an option to the transaction controller
gui.
The option is a radio button group :
"Samples of the transaction to include in the transaction sample"
with values "None", "Failing", and "All".
The default value is "None".

I also looked into adding an option "Abort transaction when sample fails", to
kind of simulate a rollback. It was not trivial to implement. If I was to
implement it, then I would have had to call the "Sampler.sample(null)" method
in the TransactionControll, i.e. duplicate the code in JMeterThread.run that
sets up the environment for the sampler, calls the sample method, and then
calls post processors, assertions and samplelisteners and so on. I don't think
that code should be duplicated into TransactionController.
If, however, the TransactionController somehow had been able to call the
"sample" method of it's sub samplers, then I think we could have avoided that
the subsamples reached the sample listeners. It would be kind of neat, because
then you could have an option to only see the transaction sample in the
visualizers.

Anyway, I hope the patch is ok now, and will investigate the issue of
"rollback" and visualizers only seeing transaction samples if you could give me
some advice on how to proceed. The patch adds useful functionality as it is
now.

What do you think about the currently suggested patch ?

By the way, I think this also solves 40539, as far as it is possible.
What you will be able to see, is the percentage of transactions which failed.
You will not see the percentage of samples of a transaction that failed.

But I think it will help a lot if the status of a transaction sample is set to
failure if any of it's subsamples fails.

I'm not sure I understand why one would only want to add failing samplers as 
children of the TC.

Also I'm still concerned about the confusion that can be generated by having 
samples appear twice.

If a flag is added to the Sample class to show which samples are included in 
the TC, then it ought to be possible to use this to prevent the samples from 
showing up twice. This would probably mean a small change to JMeterThread.

The other aspects of the patch are OK:
* accumulating byte count
* setting TC status failed if any sample fails

Created attachment 19860 [details]
Minimal patch to let transaction sample fail if subsamples fails

Perhaps you could apply this minimal patch first.

It sets the status of a transaction sample to failure if any of its' subsamples
fails. And it set the byte count to the sum of its' subamples.

It also improves the ViewResultsFullVisualizer, by allowing any subresults to
have assertions that fails, and therefore will be shown in the GUI.
This is no strictly needed, but I think it is a useful improvement to the
ViewResultsFullVisualizer.

I think this minimal patch is enough to solve the 40539 as good as it is
possible.

I suggest you leave this bug open, and then I will look more into your
suggestion on how to achieve the subsamples not to appear.
If I am able to do that, I think I will add an GUI option to the
TransactionController : "Include samples as subsamples of the transaction
sample (checkbox)", with default Off, i.e. like it is today.
Then people like me, who want the subsamples to appear as subresults of the
transaction sample can turn this option on.
I want this functionality, because I am doing functional testing, doing 8 HTTP
requests in sequence, as a transaction, and then when the transaction fails, i
want to be able to easily see which step of the transaction failed.

Created attachment 19861 [details]
Minimal patch to let transaction sample fail if subsamples fails

Sorry, I was to quick when making the minimal patch.

Here is the real minimal patch, which works as described above.

OK, I've applied patch 19861 (SVN r542617).

Created attachment 20157 [details]
Patch for letting TransactionSample contain sub results

Suggested patch to let TransactionSample contain the samples of the transaction
as sub results, and avoid the subsamples to appear in sample listeners.

This suggested patch restructures the transaction handling code quite a bit.
The TransactionController and TransactionControllerGui is moved from components
to core, because the TransactionController is now referenced in JMeterThread,
which is in core.

The source code in TransactionControllerGUI is not changed at all.

I have made a new class TransactionSampler, which is responsible for handling
the sub sampler results.

The JMeterThread is changed, to handle transactions.
The transaction sample is only sent to listeners who do not receive the sub
samples. The transaction sample contains the sub samples as sub results.
It is also possible to have assertions for the transaction sample, for example
DurationAssertion, to check the duration for the transaction. This assertion
will also be applied to the sub samples.

The TransactionController code is simpler than before.

I will attach a test plan, to show one way of using the Transactions, Listeners
and Assertions.

It could also be possible to add an option in the TransactionControllerGUI, to
specify that the transaction sample should not contain sub results, for more
backwards compatibility.

For now, I would appreciate comments about the code restructuring and the
functionality. Do you think it makes sense, and does the code look ok to you,
or is the code becoming to complex with these changes ?

Ideally, I would have liked to handle everything inside the
TransactionSampler.sample method instead of in JMeterThread, but I do not see
how that is possible currently.

Created attachment 20158 [details]
Test plan to show new transaction functionality

The test plan works before and after the suggested patch is applied.

Have a look at what the different listeners are showing.

You might have to tweak the Duration Assertion time, to make the subsamples
pass, but the transaction fail in some cases.

Created attachment 20159 [details]
Patch for letting TransactionSample contain sub results

Sorry, I moved the TransactionControllerGui from components to core in last
minute, so in the previous patch it was accidentially placed in /config/
instead of the correct /control/ package.

Please read the text for the 20157 attachment, which describes this patch.

Created attachment 20237 [details]
Patch for letting TransactionSample contain sub results

Updated patch. The subsampler result were delivered to soon to the transaction
sample in JMeterThread. It was delivered before postprocessors and assertions
had been run. It is now delivered after those steps, so that the "successfull"
state of the subsampler result has been set before being delivered to
transaction sample.

Thanks, that works much better.

The code changes have been applied to SVN in r541883.

The move of two classes from components to core was applied separately in 
r541877 to make it easier to follow the subsequent changes.

This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/1894