Problem/Motivation

Right now there's no way for people to discover what the tags applied to services mean, or what tags exist, aside from reading the (currently incomplete) API page https://api.drupal.org/api/drupal/core!core.api.php/group/service_tag/8 or from going to https://api.drupal.org/api/drupal/services/8 and looking at the Tags column (and filtering by tags) [This latter is a good way to find out what services have a given tag, but not a good way to learn what the tags are supposed to be.]

Proposed resolution

(0) Adopt a standard for how to document service tags. See #2745947: Adopt a method of documenting service tags

(1) Add @service_tag docs for each service tag in Core

a. Hopefully, most service tags have a corresponding interface that the service with that tag needs to implement. (The idea is that the service implements the interface, and another class that collects services with that tag will collect them and call one or more methods on the interface on each of the services with that tag.)

So, if a tag is associated with an interface, add documentation for that tag to the interface, like this:

 * @service_tag foo
 *   Documentation for foo tag goes here. Explain what the tag means, and note that services tagged
 *   with this tag should implement this interface. Also reference related classes, such as collector classes.
 *   - parameter1: Description of parameter 1.
 *   - parameter2: (optional) Description of optional parameter 2. The default parameter2 is x.

Example: ParamConverterManagerInterface is a good place to document the paramconverter tag in - { name: service_collector, tag: paramconverter, call: addConverter }

Some service tags have parameters. We need a way to document them, too.

b) If a service tag exists but there isn't an interface associated with it, find another relevant class to put the service tag documentation on, such as a class that implements CompilerPassInterface, which collects the services with that tag and uses them. In this case, the documentation should explain what methods need to be present on the service class (since there isn't an interface for them).

Example -- RegisterSerializationClassesCompilerPass could be a place to document the normalizer and encoder tags, since this class collects them, if there isn't an available interface to use.

c) Also, if the interface for the tag is in some vendor code that we cannot modify the documentation for, again find a relevant class and add the @service_tag documentation there, with a reference to the interface someone would need to implement if they use this tag.

(2) Have the API module collect these pieces of documentation and display them appropriately

This is:
#2316857: Handle @service_tag

(3) Add explanation of service tags

We also need a @defgroup that explains what service tags are and how to use them, kind of like what we did for @defgroup annotation. So we should define a @defgroup service_tags in core/modules/system/core.api.php. This was done on:
#2316861: Write a @defgroup for service_tag

(4) Finish the documentation page

https://www.drupal.org/node/2239393

Remaining tasks

0. Wait for the standard to be adopted. #2745947: Adopt a method of documenting service tags

1. Create child issues for appropriate groups of tags, the API module update (created), and the defgroup (created) . We want to have about 5 "document tag" child issues. The list of tags to be documented is on comment #43.

2. Document the new documentation tag @service_tag on https://drupal.org/node/1354

3. Update the handbook: https://www.drupal.org/node/2239393

4. Resolve the child issues (make patches).

User interface changes

api.drupal.org will give people documentation of how to use service tags and which ones are available, and there will be cross-linking.

API changes

None.

Comments

catch’s picture

This is closely related to #2244683: Add a dynamic listing of services which wants to list services with the same tag together.

jhodgdon’s picture

RE #1, how is this different from #2244683 at all?

catch’s picture

#2244683: Add a dynamic listing of services is talking about grouping services that are tagged the same together. That's like showing all implementations of a hook.

What I think chx is additionally asking for is API documentation on the existence of the tags themselves - same as hook documentation in .api.php - i.e. this tag exists, you can implement a tagged service that does x, if you need to do x.

If that's going to be covered by the other issue too, we should mark this as duplicate though. I nearly did but in the back of my mind realised the above.

jhodgdon’s picture

Hm...

So there is no declaration of services, as far as I know, outside of services.yml files (where modules can say "I have an implementation of this service"), right?

My plan in the API module (that other issue) was basically just to scan the services.yml files and make each of the top-level YML strings (the names of the services) be a "thing" in the API module, meaning that each service defined in a services.yml file will have its own page, we'll have a master list of Services linked on the sidebar (along with Classes, Functions, Topics that are there now), and there will be some cross-linking. But since the services don't have doc blocks, there wouldn't be any documentation.

I guess I'm still a bit confused as to what this issue is asking for... do we want to define a method for documenting what each service is? Maybe just adding a description element to the services when they're declared in a services.yml file (would Symfony allow that)?

catch’s picture

This issue is about the tags rather than the services themselves.

Tagged services shouldn't be used directly, in the same way that hook implementations shouldn't be called directly. So we'd want to exclude them from a general list of services (i.e. they are completely different to something like the translation manager), as well as listing services that are tagged the same somehow.

chx’s picture

> My plan in the API module (that other issue) was basically just to scan the services.yml

That presumes every tag defined in core is used by a service. This is not necessarily the case even for core and definitely might be a problem for contrib and also you need documentation on tags.

Example: RegisterSerializationClassesCompilerPass

    foreach ($container->findTaggedServiceIds('normalizer') as $id => $attributes) {

normalizer is a tag. Much as we did with hooks, we need to document that there is a tag called normalizer , wtf it does, what interface should you implement if any or what arguments can you expect if you tag your service with and so forth.

jhodgdon’s picture

Doh! I was not fully understanding this issue.

OK. So yes, we can list services and filter by tags, but that will not help people at all discover what the tags are or what they mean.

So... hum... I have no idea how we should do this. It's kind of like documenting form and render elements, and we've been trying to agree on how to do that for several years now with no resolution.

The only thing I can think of right now is to make one centralized list in a @defgroup topic.

chx’s picture

My suggestion: classes that implement CompilerPassInterface should look like this:

/**
 * Adds services tagged 'normalizer' and 'encoder' to the Serializer.
 *
 * @service_tag normalizer
 *    Description of normalizer.
 * @service_tag serializer
 *    Description of normalizer.
 */
class RegisterSerializationClassesCompilerPass implements CompilerPassInterface {
jhodgdon’s picture

OK... that seems like a reasonable idea... Then we would have the API module find these and make a list of service tags, and on each service tag page, ... do what? It could like to RegisterSerializationClassesCompilerPass, but should it also list something else?

chx’s picture

It could find perhaps the services and the classes of the services tagged by that tag. That'd kick ass.

sun’s picture

FYI, many compiler passes were replaced with a single TaggedHandlersPass by #2213319: Create a single Container CompilerPass to collect + add handlers to consumer service definitions

I've the impression that the suggested documentation here would duplicate the documentation on the respective handler interfaces as well as the add*() methods on the respective consumer/manager services.

How do we prevent duplicate documentation?

chx’s picture

I do not want to duplicate information but do the handler interfaces or add* have the tag documented on them? I haven't seen such.

jhodgdon’s picture

I don't see how the proposal in #9 is duplicating documentation. The proposal is to define what the tags mean by adding @service_tag items to the docs of the class that effectively is defining the tags, right? Where else would this information be written down?

chx’s picture

Issue summary: View changes
sun’s picture

Right now there's absolutely no way to figure out what tags exist for service definitions.

This issue targets PHPDoc, and proposes to add a custom PHPDoc tag, which requires a compatible PHPDoc parser to read out the tags.

Are you aware of https://api.drupal.org/api/drupal/services/8 ? (You can filter by tag.)

Which part of this proposal here is not covered by the existing api.d.o view yet?


The proposed solution only targets documentation generators (like api.d.o). It does not resolve the issue for local development scenarios (e.g., offline). In case that is the intended goal, then we should probably think about an optional, opt-in compiler pass in core that isn't used by default, but can be enabled via settings.php/services.yml, and which would collect all tags + services per tag for manual introspection.

chx’s picture

> Are you aware of https://api.drupal.org/api/drupal/services/8 ? (You can filter by tag.)

Yes. And?

> Which part of this proposal here is not covered by the existing api.d.o view yet?

Where you document where and how to use the tags.

> The proposed solution only targets documentation generators (like api.d.o). It does not resolve the issue for local development scenarios (e.g., offline).

The amount of FUD you layering is astonishing: this is how everything else is documented. Via doxygen. How do you get to it is off topic in this issue. Ie. if you are offline and want to know what plugin namespace you need to use then you need to find "Plugin Namespace: Plugin\migrate\process" on the annotation. Perhaps if you do not know where it is, you will search for every Plugin Namespace and look in there. You can grep for @service_tag just as easily off line.

OK, now that's done, let's actually proceed with documentation.

chx’s picture

I think we have 13 service collector tags

breadcrumb_builder
config.factory.override
http_client_subscriber
logger
mime_type_guesser
paramconverter
path_processor_inbound
path_processor_outbound
route_enhancer
route_filter
route_processor_outbound
string_translator
theme_negotiator

and I can find 14 top level tags

access_check
authentication_provider
backend_overridable
cache.bin
cache.context
encoder
entity_resolver
event_subscriber
needs_destruction
normalizer
persist
plugin_manager_cache_clear
twig.extension
event_subscriber

27 child issues?? Or just one go?

jhodgdon’s picture

Issue summary: View changes

Well, maybe we can make a few child issues? Like it seems like all the route* path* and http* and event_subscriber are related to routing, so those could go on one child issue (maybe paramconverter too? not sure what that is). Then there could be a group that are basic things like cache and plugin manager and config. Let's shoot for about 5 child issues at the most.

You have event_subscriber in the list twice by the way.

So have we decided on a format for this? Oh yes, I see: comment #9.

I'm going to update the issue summary so we can make child issues. And I'll file the API module issue that I just added to the issue summary.

jhodgdon’s picture

Issue summary: View changes

API module issue: #2316857: Handle @service_tag
@defgroup issue: #2316861: Write a @defgroup for service_tag

I haven't filed any "document a set of tags" issues...

dawehner’s picture

https://github.com/symfony/symfony/pull/12299 is a corresponding issue in symfony which would allow us to introduce a pretty easy to use standard.

jhodgdon’s picture

That pull request just adds @category events to a bunch of code. I don't think it's really what we want at all, because it is mixing up a bunch of stuff (constants, classes, methods?) and putting the same @category tag on them. This will IMO not help us much if at all?

xjm’s picture

Issue tags: +Triaged D8 critical
dawehner’s picture

I'm sorry for my last comment. That was just about documentation of events.

Just had a quick look at the proposed solutions in the issue summary. ... Given that most of the time, there is not a corresponding
collector pass, I would vote and move this code to the core implementation of the actual manager class, so ParamConverterManager, CheckProvider etc.

Beside of this, I'm fine with the approach here to use some sort of tags.

jhodgdon’s picture

Which code are we moving? I'm a bit confused as to which parts of the issue summary you are proposing to change, and how they'd change? You probably know more about service tags than I do... that isn't hard! So maybe you can update the issue summary with your suggestion?

The goal here is to make sure that each service tag is documented somewhere, and ideally they should be documented somewhere relevant, so that if the tag name or meaning is ever changed or removed, hopefully the documentation would be updated. So if there are more sensible places to put this documentation, I'm 100% in favor of that idea. I wrote the current proposal based on my understanding but I'm happy to have it adjusted. Let's just make sure that the tags are documented either where they're being generated, collected, or used in some way. Ideally, since the class/method/etc. header that contains @service_tag will presumably be linked on the service tags listing on api.drupal.org, the place where it's documented should not just be some api.php file, but some class/method related to this tag that would be the most useful for people wanting to learn about this tag to see. Whatever that is,... let's put it there.

cilefen’s picture

The @service_tag annotation will (sometimes?, always?) be put on classes that do not implement CompilerPassInterface. ParamConverterManager is an example of a class that does not implement CompilerPassInterface—it is a service collector. However it is the class that creates the "paramconverter" tag.

core.services.yml:

  paramconverter_manager:
    class: Drupal\Core\ParamConverter\ParamConverterManager
    tags:
      - { name: service_collector, tag: paramconverter, call: addConverter } 
jhodgdon’s picture

So, in the example in the issue summary, the RegisterSerializationClassesCompilerPass, the class's documentation first line says:

Adds services tagged 'normalizer' and 'encoder' to the Serializer.

Assuming that this class documentation is accurate, this seems like it would be at least one logical place to document the "normalizer" and "encoder" service tags.

@cilefen and @dawehner, are you advocating a different location? Which class/interface/method?

And maybe you can provide some other examples of service tags that we should document on other classes? Comments #24/26 are too vague/general for me to figure out, and I still do not understand whether you are saying this example of RegisterSerializationClassesCompilerPass in the issue summary is not a good idea, or that the 2nd case of "there is not a compiler class" needs adjustment, or something else.

Please provide some specific examples so I can understand? Thanks!

cilefen’s picture

As far as I know, nothing changes. The new annotations must go on the most logical class for the given situation. In the case of RegisterSerializationClassesCompilerPass, the proper place is on RegisterSerializationClassesCompilerPass, which actually creates the tag. When we talk about ParamConverterManager, the services definition technically creates the tag, nevertheless, ParamConverterManager is the right place for its documentation.

dawehner’s picture

Maybe for consistency we never use the pass as the place to document it, but always place it on the service itself?

cilefen’s picture

@dawehner: On which class would we put the documentation for the 'normalizer' tag in that case?

jhodgdon’s picture

So... Presumably multiple services can be tagged with a given service tag, right? Otherwise, it would be pretty pointless to use tags -- I think they are only useful if they are tagging a bunch of services that are related somehow and needs to be collected, right?

So... to me it makes the most sense to put the documentation on wherever that tag is being used/collected/read. If it's not being collected or read somewhere, I don't understand what the tags are for? Because which, if we are just putting the docs on one of the services that is tagged with that tag, how would we choose which one? Besides which, my vision for the tag pages on api.drupal.org is that they would show you "5 classes are tagged with 'foo'" (which it would get from the services.yml file), and also lead you to the class that documents the tag... we want to make sure that everything related to that tag is connected on that page.

So can you give me any examples of a service tag that doesn't have some class that is collecting or using it?

joachim’s picture

It seems to me that the patch at #2316861: Write a @defgroup for service_tag provides a fairly obvious answer:

+++ b/core/modules/system/core.api.php
@@ -1798,3 +1778,38 @@ function hook_display_variant_plugin_alter(array &$definitions) {
+ * how the service behaves. Typically, if you tag a service, your service class
+ * must also implement a corresponding interface. Some common examples:

If a tag corresponds to an interface, then why not add the docs to the interface?

Only thing is, when is it NOT typical to have a corresponding interface?

jhodgdon’s picture

Issue summary: View changes

That sounds like an excellent idea, if there is an interface. However, there are definitely some services that don't have interfaces for people who want to override them to use, and there are most likely some service tags also that don't have interfaces, so I don't think we can assume there will always be an interface.

But... I think this is a lot better idea than anything else that has been proposed, so I'm updating the issue summary... I guess we are just talking about reversing the order of the proposed places to document things, but I agree the interface is what people will really need to know about. ... but maybe we should also have a standard that you should add links to the collector classes as well? I've tentatively put that into the summary.

Thoughts?

dawehner’s picture

Well, in case there is no interface, I would prefer it to set on the actual class first, given that the place is more familiar than the compiler pass itself.

jhodgdon’s picture

Which actual class, one of the classes that is tagged with that tag? I'm not sure I agree that is a great idea. It seems like in that case the consumer of the tag would be the better place to document what the tag means and what methods services with that tag would need to have on them. Of course, they should just have interfaces, because that is the most straightforward way to document "these are the methods that may be called". But we have a lot of places in Core without interfaces... sigh.

Anyway maybe if we start working on the documentation we can see if there are actually tags without interfaces? The correct fix might be to add the interface.

dawehner’s picture

Which actual class, one of the classes that is tagged with that tag?

Well, a lot of the example of tags in core are used to collect a bunch of services and inject it into a single service.
At least for now, I was mostly talking about that service/interface as the canonical place to document the corresponding service tag.

jhodgdon’s picture

OK, then in that case the service that gets the tags is kind of playing the role of "collector"? Not sure how to word that. But... maybe we can just look at the actual tags we need to document, and just make sure they get documented *somewhere* that makes sense? I don't know that we need to make a strong rule about where to document them, as long as they get documented.

But the one thing I would note is that I think this documentation should be oriented towards developers who might want to add another service with that tag, not oriented towards "people who are trying to understand core" as the primary audience. In which case, the information they'd really need is "What methods do my service class need to implement?", not "Who collects these things together?".

In other words, they need to know what the interface is for the tagged service, assuming there is one (and IMO there always should be). So that should be the primary place to put the docs for the tag; we should also refer to the "collector", whatever that is, and any other relevant classes, in the service tag description, to satisfy the other audience of "person who is trying to understand core".

Agreed?

Berdir’s picture

I'm more or less trying to do that in #918538: Decouple cache tags from cache bins for the newly introduced cache_tags_invalidator service tag there. Care to have a look at the patch there, especially the latest interdiff and the docs in general?

jhodgdon’s picture

Um. This issue is about service tags, not cache tags... but yes I'll take a look. :)

Berdir’s picture

I know, and that cache tags issue is introducing a service tag called cache_tags_invalidator. And I am trying to document it there in a way that is in line with what is discussed/proposed here :)

Edit: Oh, I had a typo in there, that should read "service tag", not cache tag. Sorry. Too much tagging too late in the night.

jhodgdon’s picture

Ah. Well the proposal here is that each service tag should be documented with @service_tag as described in the issue summary, and if that is the case you'd want those lines to be put on the new CacheTagsInvalidatorInterface or whatever it was called. And I think you'd also want to add references in that description to the class(es) that collect all services with this tag.

Anyway, I think given that we haven't totally agreed to do this, your patch on the issue has good documentation.

dawehner’s picture

I'm fine with the correct proposal though, it is a place where most people will be able to find the tag, especially if it parsed / searchable using the "@service_tag" string.

cilefen’s picture

From #18:

I think we have 13 service collector tags

breadcrumb_builder
config.factory.override
http_client_subscriber
logger
mime_type_guesser
paramconverter
path_processor_inbound
path_processor_outbound
route_enhancer
route_filter
route_processor_outbound
string_translator
theme_negotiator

and I can find 14 top level tags

access_check
authentication_provider
backend_overridable
cache.bin
cache.context
encoder
entity_resolver
event_subscriber
needs_destruction
normalizer
persist
plugin_manager_cache_clear
twig.extension
event_subscriber

New tags since #18:

cache_tags_invalidator
page_cache_request_policy
page_cache_response_policy
module_install.uninstall_validator
http_middleware
render.main_content_renderer
stream_wrapper
twig.loader
cilefen’s picture

Issue summary: View changes
cilefen’s picture

Issue summary: View changes
cilefen’s picture

Issue summary: View changes
cilefen’s picture

Issue summary: View changes

Added a child issue to document the other tags.

cilefen’s picture

Issue summary: View changes

Linked to the handbook page in the summary.

xjm’s picture

Issue tags: -Triaged D8 critical

So I tagged this originally in November as part of #2238935: [meta] Complete missing documentation for special strings and metadata like annotation keys, routing parameters, tagged services, etc., before the discussion that happened on that issue, and without the branch maintainers having looked it over. That was a mistake on my part. Untagging so we will look at this issue next time around.

@jhodgdon, any thoughts on what still would need to block release here? After having reviewed the scope of the child issues, I think #2413975: Document service collector tags and the service_collector tag is definitely not critical, and #2417789: Document service tags, other than those related to service collectors also doesn't seem critical either. (The API itself is documented, as are the core implementations of these services.) Do you think that there is still anything in the scope for this issue that needs to block release?

xjm’s picture

Title: Document service definition tags » [meta] Document service definition tags
Priority: Critical » Major

@johdgdon replied in #2417789-11: Document service tags, other than those related to service collectors; downgrading to major. Thanks!

Also I guess this is a meta now?

cilefen’s picture

Oops - I thought we had already downgraded it. Any, yes, it's a meta.

jhodgdon’s picture

We still have not come to a formal agreement that we are adopting this standard for documenting service tags, as far as I can see. Work on the child issues at this point is premature.

cilefen’s picture

Issue summary: View changes
cilefen’s picture

@jhodgdon I consider this a DX issue. How best can we proceed to getting agreement?

jhodgdon’s picture

Well we need to go through the (as yet in progress of defining) process for proposing standards changes. Difficult to do right now, unfortunately.

cilefen’s picture

I understand. I don't want to rush something in that we would regret. In order to get more opinions, I opened an issue in Symfony's queue to see if this has been discussed among those developers.

cilefen’s picture

Issue summary: View changes
jhodgdon’s picture

Good idea. If Symfony has a standard, we can just sort of piggy-back on it.

And based on your latest issue summary change, it looks like we need to change what the proposal was. Can you provide an example of what you mean by parameters on tags that we need to document?

cilefen’s picture

Here is an example from the work-in-progress sub-issues:

 * @service_tag encoder
 *   Services that provide object serialization encoders must be tagged
 *   'encoder' to be registered to the serializer service. Parameters are:
 *   - format: The encoding format for this encoder. Example: json.
 *   - priority: (optional) An integer priority for this encoder. The default
 *       priority is 0.
 *   This tag is processed by
 *   \Drupal\serialization\RegisterSerializationClassesCompilerPass.
jhodgdon’s picture

OK, that looks fine to me.

cilefen’s picture

Issue summary: View changes
cilefen’s picture

There is a suggestion Symfony issue that the compiler pass class where a given tag is discovered is would be the sensible place to document the tag.

jhodgdon’s picture

Let's give that Symfony issue a little while. Maybe we'll be able to push it towards agreement. Thanks for opening it up! I commented there.

mikeker’s picture

Issue summary: View changes

Corrected API link.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhodgdon’s picture

Issue summary: View changes
Status: Active » Postponed

Spinning off the standards question to a separate issue, and postponing this Core meta issue until that is done. That is... this issue will stand as "Document the service tags", once we have adopted a Coding Standard that says how to do it.
#2745947: Adopt a method of documenting service tags

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.