Add helpful remarks on custom DataCollector #7773

xosofox · 2017-04-08T11:36:11Z

When creating my own data colletor, I was not clear on how to exactly collect my data (unrelated to request/response)

Here are two remarks that would have helped me a lot by the time

collect is only called once to "pick-up, not "gather"
how/where to store and access data that is not related to request/response

When creating my own data colletor, I was not clear on how to exactly collect my data (unrelated to request/response) Here are two remarks that would have helped me a lot by the time * collect is only called once to "pick-up, not "gather" * how/where to store and access data that is not related to request/response

Add helpful remarks on custom DataCollector

"make accessible to" instead of "for"

xabbuh

This looks like a pretty nice addition. 👍 I just left two minor comments. If you would like to address them, that would be really nice. Otherwise, we can address them while merging.

xabbuh · 2017-04-12T12:32:00Z

profiler/data_collector.rst

+    If the data that is not directly related to the request or response, you need to make the data accessible to your DataCollector.
+    This can be achieved by injecting the service into your DataCollector.
+
+


one blank line is enough here

xabbuh · 2017-04-12T12:32:41Z

profiler/data_collector.rst

@@ -73,6 +78,12 @@ The getters are added to give the template access to the collected information.

 .. caution::

+    If the data that is not directly related to the request or response, you need to make the data accessible to your DataCollector.
+    This can be achieved by injecting the service into your DataCollector.


I think we can be a bit more specific what we mean with "the service" (something like "by injecting a service that provides this information into your data collector).

xosofox · 2017-04-13T10:44:57Z

profiler/data_collector.rst

@@ -73,6 +78,11 @@ The getters are added to give the template access to the collected information.

 .. caution::

+    If the data that is not directly related to the request or response, you need to make the data accessible to your DataCollector.
+    This can be achieved by injecting the service that holds the information you intend to profile into your DataCollector.


Would a code example (CustomService in constructor, gathering data in the collect method) be helpful here? Or would you consider it to be too specific?

I think that's a bit too specific. But let's see what other members of the @symfony/team-symfony-docs team think about it.

I don't think this is needed, this note sounds clear enough, also a constructor is not this only way to inject data.
Note that the collector itself can be used as listener in order to grab anything, and then collect data using it, both injection and observation are used in the core btw.

Good point. Then why not put this into the doc as a note as well?
I think this is clear once you are familiar with the concepts of Symfony but not for the n00b reading this for the first time

I don't think a code example would improve anything here. At the point you're creating your own DataCollector, you're probably already aware of injection and DI. The comment is a nice addition as it is right now imo. In case people get stuck here, there's always IRC or Slack, where they will probably be forwarded to the DIC docs.

xabbuh · 2017-05-17T06:47:18Z

Thank you @xosofox.

This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #7773). Discussion ---------- Add helpful remarks on custom DataCollector When creating my own data colletor, I was not clear on how to exactly collect my data (unrelated to request/response) Here are two remarks that would have helped me a lot by the time * collect is only called once to "pick-up, not "gather" * how/where to store and access data that is not related to request/response Commits ------- 29522b3 Add helpful remarks on custom DataCollector

* 2.7: (21 commits) Update optional_dependencies.rst Fix xml blocks [#7875] minor tweaks Minor fix Minor changes [#7773] fix line length Add helpful remarks on custom DataCollector Remove use of deprecated security.exception_listener.class parameter Update resources.rst Fix incoherent ut8mb4 collation in Doctrine setup Fix decorating service definition fix empty XML element [#7873] add missing toctree entries Reworded the introduction of the contributing docs [HttpFoundation] Fix clearstatcache first arg Explain how to provide a stack trace Add link to the Slack support channel Mentioned FOSRestBundle's content negotiation Used the article example consistently in the example Update formats.rst ...

* 2.8: (24 commits) Update optional_dependencies.rst Fix xml blocks pass only strings to loadUserByUsername() Fix Authenticator Class (getCredentials) example Added the picture that shows how GuardAuthenticationListener calls Authentication Guard methods. [#7875] minor tweaks Minor fix Minor changes [#7773] fix line length Add helpful remarks on custom DataCollector Remove use of deprecated security.exception_listener.class parameter Update resources.rst Fix incoherent ut8mb4 collation in Doctrine setup Fix decorating service definition fix empty XML element [#7873] add missing toctree entries Reworded the introduction of the contributing docs [HttpFoundation] Fix clearstatcache first arg Explain how to provide a stack trace Add link to the Slack support channel ...

* origin/2.7: (51 commits) Avoid backticks in shell scripts Update optional_dependencies.rst Fix xml blocks [#7875] minor tweaks Minor fix Minor changes [#7773] fix line length Add helpful remarks on custom DataCollector Remove use of deprecated security.exception_listener.class parameter Update resources.rst Fix incoherent ut8mb4 collation in Doctrine setup Fix decorating service definition fix empty XML element [#7873] add missing toctree entries Reworded the introduction of the contributing docs [#7844] add XML and PHP config examples Property access [HttpFoundation] Fix clearstatcache first arg Explain how to provide a stack trace fix remaining setDefinition() call ...

* 3.2: (39 commits) updating instance Avoid backticks in shell scripts Update optional_dependencies.rst Fix xml blocks pass only strings to loadUserByUsername() Fix Authenticator Class (getCredentials) example Documented addAnnotatedClassesToCompile() and the use of class patterns Added the picture that shows how GuardAuthenticationListener calls Authentication Guard methods. [#7205] minor tweak Simplified the use of transChoice() [#7875] minor tweaks Minor fix Minor changes Properly show all events and describe guard events [#7891] remove not needed sentence [#7773] fix line length Add helpful remarks on custom DataCollector Remove use of deprecated security.exception_listener.class parameter Update resources.rst Fix incoherent ut8mb4 collation in Doctrine setup ...

* 3.4: (40 commits) Adding an article to explain the 3.3 changes, and how to upgrade updating instance Avoid backticks in shell scripts Update optional_dependencies.rst Fix xml blocks pass only strings to loadUserByUsername() Fix Authenticator Class (getCredentials) example Documented addAnnotatedClassesToCompile() and the use of class patterns Added the picture that shows how GuardAuthenticationListener calls Authentication Guard methods. [#7205] minor tweak Simplified the use of transChoice() [#7875] minor tweaks Minor fix Minor changes Properly show all events and describe guard events [#7891] remove not needed sentence [#7773] fix line length Add helpful remarks on custom DataCollector Remove use of deprecated security.exception_listener.class parameter Update resources.rst ...

xosofox added 2 commits April 8, 2017 13:29

Merge pull request #1 from Xosofox/datacollector-remarks

9f8f97a

Add helpful remarks on custom DataCollector

carsonbot added the Status: Needs Review label Apr 8, 2017

grammar fix

d80da02

"make accessible to" instead of "for"

xabbuh reviewed Apr 12, 2017

View reviewed changes

xabbuh added Status: Reviewed and removed Status: Needs Review labels Apr 12, 2017

HeahDude added this to the 2.7 milestone Apr 12, 2017

Updated according to remarks by @xabbuh

0617174

xosofox commented Apr 13, 2017

View reviewed changes

xabbuh added the needs comments label Apr 14, 2017

xabbuh added a commit that referenced this pull request May 17, 2017

[#7773] fix line length

ca895b2

xabbuh closed this May 17, 2017

javiereguiluz added Waiting feedback and removed needs comments labels Jan 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add helpful remarks on custom DataCollector #7773

Add helpful remarks on custom DataCollector #7773

Uh oh!

xosofox commented Apr 8, 2017

Uh oh!

xabbuh left a comment

Uh oh!

xabbuh Apr 12, 2017

Uh oh!

xabbuh Apr 12, 2017

Uh oh!

xosofox Apr 13, 2017

Uh oh!

xabbuh Apr 14, 2017

Uh oh!

HeahDude Apr 15, 2017

Uh oh!

xosofox Apr 15, 2017

Uh oh!

linaori Apr 22, 2017

Uh oh!

xabbuh commented May 17, 2017

Uh oh!

Uh oh!

		If the data that is not directly related to the request or response, you need to make the data accessible to your DataCollector.
		This can be achieved by injecting the service into your DataCollector.

Uh oh!

Add helpful remarks on custom DataCollector #7773

Add helpful remarks on custom DataCollector #7773

Uh oh!

Conversation

xosofox commented Apr 8, 2017

Uh oh!

xabbuh left a comment

Choose a reason for hiding this comment

Uh oh!

xabbuh Apr 12, 2017

Choose a reason for hiding this comment

Uh oh!

xabbuh Apr 12, 2017

Choose a reason for hiding this comment

Uh oh!

xosofox Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

xabbuh Apr 14, 2017

Choose a reason for hiding this comment

Uh oh!

HeahDude Apr 15, 2017

Choose a reason for hiding this comment

Uh oh!

xosofox Apr 15, 2017

Choose a reason for hiding this comment

Uh oh!

linaori Apr 22, 2017

Choose a reason for hiding this comment

Uh oh!

xabbuh commented May 17, 2017

Uh oh!

Uh oh!