MessageDataCollector improper reset/collect

[andrei-e-random]

What is the issue?

When multiple requests happen with the profiler enabled before the first one and without a kernel reboot between them, there is no way to find out what happened in each of the requests.

For example, if request 1 sends an email and request 2 sends another email, the getMessageCount() method will return 2 after the second email, despite the fact that actually a single email has been sent in it.

When was the issue introduced?

Probably here: #197

Why does it happen?

See: https://symfony.com/doc/current/profiler/data_collector.html#creating-a-custom-data-collector

The collect method is supposed to collect the data for the given request, rather than all the logged data.

Thankfully, it is guaranteed to be called once per requesst, so having a way to match the logs with the request is not that important. However, it is important to know what has already been collected (which, in its current implementation, it doesn't).

The reset method is supposed to remove all the information collected with the collect() method. What exists there is fine now, but it has no effect because of the fact that collect() actually looks at all the data available rather than only what is new.

Potential Solution

Do not call reset each time collect is called. Instead, have collect remember where it left off logs processing and only process new logs - add results to the already existing one.

Let me know if I should propose a PR. I'm not very familiar with the code/testing infrastructure you have set up, but I could try.

[stof]

Well, all core collectors are suffering from the same if you don't reset the tracing implementations. This look more like a missing Resettable implementation on the class performing the tracking itself.

[andrei-e-random]

@stof, thanks for the quick reply!

The class performing the tracker is the Profiler. The implementation of Ressetable there seems quite straight forward to me: https://github.com/symfony/symfony/blob/2cee7f2b174be874b9debc4b06e070b8bd973cdc/src/Symfony/Component/HttpKernel/Profiler/Profiler.php#L170

The thing is, it can't clear the trace because the trace is not a collector. The fact that the trace is used in the MessageDataCollector(or any other collector, for that matter) is a implementation detail which is of no interest to the Profiler, as far as I can figure out. I could be wrong though - it is the first time I am looking at the Profiler from this perspective.

In fact, the documentation for creating custom data collectors is pretty clear. Are the core collectors not supposed to conform to it for some reason which I am missing? And is the MessageDataCollector a core collector? - I'm not entirely sure what 'core' is referring to.

A trait for collectors which get their data from the trace might be a good idea - though I'm not sure where exactly I should submit the PR with that. Anyway, such a trait would only help with code duplication - I'm not sure that the lack of it should stop us from fixing the issue at hand.

What do you think?

[stof]

@andrei-e-random no, the actual tracking of messages is not done by the Profiler class (which knows nothing about the messages). And the MessageDataCollector may not perform the tracking itself. In many cases, collectors only get the tracked data from a separate object performing the tracking (integrated in the tracked component rather than integrated in the profiler). This service may be the one not properly reset when resetting services between requests.

[andrei-e-random]

@stof I obviously misunderstood what you meant by 'tracking' then. I thought you meant whoever calls collect and reset.

I must confess that I still don't understand. Which class, specifically, performs the tracking and what does it track?

[fabpot]

Closing as this bundle is not maintained anymore.