User Research Feedback Improvements - Creating a Symfony Bundle

[bobstrecansky]

We'd like to create a Symfony bundle in order to make it easier to adopt OpenTelemetry PHP in Symfony projects. This feedback came from this comment

[bobstrecansky]

@prondubuisi is going to work on this.

[prondubuisi]

Hello @dkarlovi , I am getting started with this task later in the day, are there any goals that we have for this? Also, I think the official Symfony docs are exhaustive on building bundles, but if you have additional resources that can help us reach the goal for the bundle that would be nice.

[dkarlovi]

Generally, the bundle is a "module" or a "plugin" in Symfony's nomenclature. With a bundle, you get to inject your code in basically any part of the process: building and validation configuration, building the container, event dispatchers, security listeners, error handlers, etc. My suggestion would be to start simple, get it to just start and stop the tracing and flush it, the fidelity can be raised incrementally.

[prondubuisi]

Okay @dkarlovi , noted with thanks.

[bobstrecansky]

@prondubuisi - are you still planning on finishing this up?

[prondubuisi]

Hello @bobstrecansky , I intend to continue work on all the tasks I am tagged on. Thanks

[tidal]

Hi. Is this task still/yet in progress?

I have started working on a symfony bundle, before I have seen this discussion. The bundle is not "finished" yet, but I'm quite far in terms of DI, configuration, etc.

If you guys like, I could take over this task and/or help out.

[prondubuisi]

Hello @tidal, please feel free to take this up. Thanks.

[Blacksmoke16]

@tidal I'd also be quite interested in helping out!

[tidal]

@prondubuisi Ok, great, I will.

In general, where do you guys want the symfony bundle to live? If it's intended to be part of the open-telemetry organization, it needs a separate repository (or sub split, etc.) I could run PRs against. Sadly packagist can only cope with packages in a root directory.

@Blacksmoke16 Sounds good. I have seen you have done a lot of work on this library, so your knowledge is probabably better than mine. Once above question is solved, and I have made some adjustments, I will have some code to push/show in the next days, and we could discuss how to go from there.

[Blacksmoke16]

In general, where do you guys want the symfony bundle to live?

@bobstrecansky and I chatted a bit about this a little while ago. My understanding is that this would essentially live within https://github.com/open-telemetry/opentelemetry-php-contrib/tree/main/instrumentation as it's own symfony directory. However, I think that the exact namespace it would live under and the exact process for how these packages will be published/maintained is still up in the air. E.g. OpenTelemetry\Contrib\Instrumentation\Symfony or OpenTelemetry\Instrumentation\Symfony, etc.

IMO, it would be fine to keep it under a personal GH account while it's still under development, then could be moved when it reaches a more semi-stable spot.

[tidal]

Ok, then I will push my code to a personal repo first (probably at the WE), and it can be moved later on.

Having the code as a sub-directory of https://github.com/open-telemetry/opentelemetry-php-contrib is probabably generally a good idea, however (as I mentioned ) there would still need to be some kind of mirror, where the package code is at the top level directory, see: https://github.com/composer/packagist/issues/472 But this can be solved later, it's not a problem for now.

[bobstrecansky]

@Blacksmoke16 is correct; opentelemetry-php-contrib would be the place for the instrumentation! Thanks for you ideals on this @tidal; looking forward to helping out any way you need!

[tidal]

@bobstrecansky Oh, I think I got it now. So the idea is to have all instrumentations in opentelemetry-php-contrib as one composer package? Is that correct?

[bobstrecansky]

Yes; that is the pattern that the other language SIGs (including the Collector) are following now.

[Blacksmoke16]

@bobstrecansky Isn't it more so that the code lives in there, but they're published as dedicated packages?

Probably wouldn't hurt to start figuring out how we're going to do that. IMO I don't think it would be a good idea to have a dedicated repo for each package in the open-telemetry org at least. Wouldn't be great to pollute it with a bunch of repos for one off things. At that point might be better to just create a open-telemetry-php org to put everything in.

I did find https://blog.packagist.com/installing-composer-packages-from-monorepos/, but looks like a paid thing that could get pricey fast so also not real ideal. Doesn't actually seem to be any great ideas, so having a dedicated org might be the way to go :/

[tidal]

@Blacksmoke16 The problem is not even that Private Packagist is a paid service, but that it's meant to host proprietary packages. So users would have to add additional configuration to even be able to load the packages. However all packages/components of Symfony live in a mono repo as well, but they use subsplits to mirror individual packages to single repos. The console component for example.

I wll try for now to make the Symfony bundle work, when I integrate it under open-telemetry-php with the root composer.josn. Usually Symfony bundles expect "type": "symfony-bundle" in composer.json, since Symfony comes with its own sort of "auto instrumentation" by so called "recipes" (They inject default variables in a .env file etc.). But I guess there is a way to work around this, with a composer post install script. I will give it a try.

[Blacksmoke16]

@tidal Ahh fair enough, I missed the extra config part.

However, IMO, I wouldn't handicap the bundle just to integrate it into open-telemetry-php. It would be best to have a bundle that takes full advantage of Symfony's features, and figure out a distribution solution later. Not much we can really do to work around it it seems. At this point it's just a question of if they'd rather have a dedicated org to house the mirrored repos, or would be okay putting them in this org.

[tidal]

Ok, I just tested a minimal symfony bundle living in opentelemetry-php-contrib: https://github.com/tidal/opentelemetry-php-contrib/blob/sf-bundle-test/instrumentation/symfony/src/OpenTelemetryBundle.php

Adding the open-telemetry/opentelemetry-php-contrib package to a symfony project does result in the bundle to be detected and auto registered (config/bundles.php): return [ Symfony\Bundle\FrameworkBundle\FrameworkBundle::class => ['all' => true], OpenTelemetry\Instrumentation\Symfony\OpenTelemetryBundle\OpenTelemetryBundle::class => ['all' => true], ]; The second line has been automatically added and is all which is needed to work as a bundle, since the "bundle class" follows symfony's naming convention for bundles. So there is no problem in shipping the bundle as part of the overall opentelemetry-php-contrib package.

I will then proceed to add my code to opentelemetry-php-contrib and hopefully by sunday there is something more for you guys to see than an empty bundle.

[prondubuisi]

Yes; that is the pattern that the other language SIGs (including the Collector) are following now.

Hello @bobstrecansky, I think this is the exact opposite. Every Package should be separate and self-containing I think. Communities like that of Symfony thrive in being just minimalist and using only what you need. So having a Symfony package that also has Laravel configs and configs for other frameworks would not really fly. cc @dkarlovi

[prondubuisi]

@tidal Ahh fair enough, I missed the extra config part.

However, IMO, I wouldn't handicap the bundle just to integrate it into open-telemetry-php. It would be best to have a bundle that takes full advantage of Symfony's features, and figure out a distribution solution later. Not much we can really do to work around it it seems. At this point it's just a question of if they'd rather have a dedicated org to house the mirrored repos, or would be okay putting them in this org.

Yeah, I agree. We should get working packages first and figure out how to publish them from https://github.com/open-telemetry/opentelemetry-php-contrib later.

[tidal]

Guys, please take a look at the configuration draft for the the symfony bundle. This draft yet only reflects possible configurations to wire up the tracing SDK via dependency injection. It does not yet take into account what is actually recorded and how. This ca be dealt with once the bundle can "tame" all the boilerplate code. Most entries will have default values and there is (will be) validation for the complete configuration schmema. So when env variables for the service name and exporter dsn are provided, the configuration is actually not required. It's only purpose is to provided a more granular instrumentation when needed: https://github.com/tidal/otel-apiplatform-example/blob/config-draft/api/config/packages/open_telemetry.yaml

I have addeds ome comments in the yaml file, but if you think I missed something, got something completely wrong, or forgot something, which could possibly be configured (even if the SDK will only provide it in the future), please let me know. Btw, this is only an example in yaml. Configuration in Symfony can also be provided in XML or plain PHP. And there can be individual config files per environment (dev, test, staging, prod), which can override values from the global configuration.

In case you are wondering about the application. It's a API Platform project. API Platform is a REST framework plus Frontend and Admin component, which sits on top of Symfony and comes with a lot of neat features. It will make developing and testing a lot of things much easier (including K8S integration, since it ships with Helm charts)

[Blacksmoke16]

@tidal I think it's a good start!

Unfortunately, the SDK doesn't really have a way to configure itself at the moment, so it's not immediately clear what all needs to be configurable, or how that would be setup. Ideally once that is more ironed out the Symfony config could just forward the same config to it, then provide the configured types as services. Until then I think keeping things simple to start will making updating it to use this easier in the future. I.e. focus on the core stuff you have now and can iterate on more advanced/specialized things later on.

Just a few comments/suggestions.

1. Does it also allow custom services to be used? E.g. root_sampler: MyApp\Samplers\MySpecialSampler
2. Yes, it would make sense to allow multiple processors, there's also a MultiSpanProcessor that allows wrapping multiple into one type
3. There should probably be some configuration for ResourceInfo, which should include the service name by default, along with the application version (prob is a way to get this programmatically?)

[tidal]

@Blacksmoke16 Thanks for your feedback!

Does it also allow custom services to be used? E.g. root_sampler: MyApp\Samplers\MySpecialSampler

Not yet, but I can take care of that. There is always the possibility to create and configure custom services and reference them with @my_service in SF configs.

Yes, it would make sense to allow multiple processors, there's also a MultiSpanProcessor that allows wrapping multiple into one type

Ok.

There should probably be some configuration for ResourceInfo, which should include the service name by default, along with the application version (prob is a way to get this programmatically?)

This should be no problem.

Unfortunately, the SDK doesn't really have a way to configure itself at the moment,

Symfony Config and Dependency Injection are standalone components, so it would be possible to reuse/extract this part for the SDK in the future. I will keep this in mind. Futhermore the DI component is PSR11 compatible, so it would be possible to configure DI of other frameworks/packages as well.

Anyway, I will clean up my experimental code and integrate it in my opentelemetry-php-contrib clone so we can have a working example, an we can take it from there.

[tidal]

I have extended the bundle configuration, based on @Blacksmoke16 's feedback and some of my learnings. https://github.com/tidal/otel-apiplatform-example/blob/config-draft/api/config/packages/open_telemetry.yaml

• Added a resource section with resource limits and attributes.
• Service name has to be set in resource now, and will be populated to exporter instances
• Allowed custom samplers and the configuration of remote and local samplers
• Allowed custom span processors
• Allowed custom exporters and various configuration methods for exporters
• Removed default exporter setting. The exporter entry can now take a single string besides a list of exporters

The configuration/parsing/validation for all of this is already done. I'm working on wiring up all the services based on the config atm.

[tidal]

Regarding extracting individual packages from the opentelemetry-php-contrib mono-repo:

This is the subtree splitter used by symfony: https://github.com/splitsh/lite This is a subtree split as a service: https://www.subtreesplit.com/ . I have never used it, but it's free and the service is provided by the author of the DsnParser and used in the SDK and the nyholm/psr7 http client.

[tidal]

The first version of the symfony bundle is done. You can havea look here: https://github.com/tidal/opentelemetry-php-contrib/tree/sf_bundle/instrumentation/symfony/OtelSdkBundle

I decided to split the bundle into two parts, a sdk bundle and a instrumentation bundle, so what you can see is the first part.

The sdk bundle takes care of providing configuration for the sdk (plus defaults for nearly all config options), and turning the configuration into service definitions for the dependency injection container. As a result classes or methods only need to type hint the Tracer or TracerProvider and symfony wil automatically inject the configured TracerProvider/Tracer instance, This can look like this:

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use OpenTelemetry\SDK\Trace\Tracer;

class HelloController
{
    private Tracer $tracer;

    public function __construct(Tracer $tracer)
    {
        $this->tracer = $tracer;
    }

    /**
     * @Route("/hello", name="hello")
     */
    public function index(): Response
    {
        $span = $this->tracer->spanBuilder(__METHOD__)->startSpan();

        // DO stuff

        $span->end();

        return $this->render('hello/index.html.twig', [
            'controller_name' => 'HelloController',
        ]);
    }
}

The SdkBundle could be used to configure the sdk in non symfony environments as well, it only depends on three standalone symfony components/packages, not the overall framework.

The second part, the instrumentation bundle, will make use of all the symfony specific features to allow auto instrumentation (and configuration of it), so not even the code above will be actually needed.

However,I am working on a little demo symfony application for the sdk bundle and a an initial small documentation, then I will create a PR.

[tidal]

Ok, I finally created the PR for the SF Bundle, and you can take a look at the at the demo application here.

Next time think twice before you give a task the label bite sized! (just kidding)

[bobstrecansky]

Sorry for the additional work! It's very much appreciated! Running CI and reviewing.

[tidal]

@bobstrecansky Don't worry, I have been working on Symfony projects since its version 1, so I new upfront what work this will include.

[tidal]

@bobstrecansky
I believe this task can be closed now. If have created some follow up tasks over at open-telemetry/opentelemetry-php-contrib/issues (and probably create some more), in case someone is interested in helping out with improvement or further development.