symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.75k stars 9.46k forks source link

Integrate unique request id #11573

Closed hacfi closed 5 years ago

hacfi commented 10 years ago

With the introduction of the DX label I was wondering whether you support the idea of integrating a unique request id. I know you can easily add it yourself but I think it’s useful in most projects.

My suggestion would be to allow to pass a generated environment variable if you already generate one before (I’m using https://github.com/newobj/nginx-x-rid-header for example) or generate one if not available. We could also have a look at how to use it in monolog. You should be able to configure whether to pass it to the frontend or not (X-Request-Id header).

weaverryan commented 10 years ago

Hey @hacfi!

I'll admit that I don't really know much about this idea. What's the typical use-case? In other words, why would I want the unique request id? How would I use on the backend or the frontend?

Thanks!

hacfi commented 10 years ago

@weaverryan A unique request id makes it possible to trace a request along the entire lifecycle/infrastructure and thereby helps to debug errors. If a user complains about an error you can have a look at which server(s) processed the request and what log entries were made. In my case Varnish, nginx and Symfony logs contain the id and I’m trying to integrate it with Doctrine/MySQL as well. It’s also passed to the JavaScript analytics tool so you can always trace back where an issue is coming from.

Not that it matters but it’s also included in rails: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/request_id.rb

weaverryan commented 10 years ago

Hmm, that's interesting! If Rails does it, it's a proven thing, at least. Assuming there's no security concerns we need to be careful of, I like the idea and I like integrating it with Monolog. With a little cookbook article, we could also show you how to add this to a few other things (like your web-server) for end-to-end tracking.

hacfi commented 10 years ago

@weaverryan I can maybe work on a demo the next few weeks, put it on Github and then we can discuss it further. I think it’s a must-have for big setups and useful in small ones as well.

phramz commented 10 years ago

hey there! when it comes to monolog ... there already is a very basic example in the cookbook: http://symfony.com/doc/current/cookbook/logging/monolog.html#adding-a-session-request-token

... but i really like the idea of having a unique request-id apart from monolog. to get this token directly from the request itself sounds tempting to me.

stof commented 10 years ago

Well, if you want to correlate all logs, it should be generated as early as possible. This would mean generating it in Nginx (or even in Varnish) when receiving the request. If you generate it only in Symfony, some logs will not have the request id yet. Writing a Monolog processor reading it is very easy. Note that Monolog already provides a WebProcessor able to include some request header in the log messages. The logic would be very similar here (it already supports a unique id, but takes it from $_SERVER['UNIQUE_ID'], which cannot be an HTTP header unfortunately) The Rails middleware also get the request id from a request header first before tryign to generate its own.

hacfi commented 10 years ago

@stof I do it this way too but it might make sense for beginners so they don’t have to set this up themselves. But do you think it should be part of Symfony?

fabpot commented 9 years ago

@hacfi I think it makes sense... if it does not add too much overhead.

asm89 commented 9 years ago

@fabpot How about pulling in: https://github.com/qandidate-labs/stack-request-id? It contains logic to generate a request id if it wasn't set in a configured header and ships the monolog processor.

fabpot commented 9 years ago

@asm89 Indeed, that looks interesting.

asm89 commented 9 years ago

@fabpot I guess there are a few options:

fabpot commented 9 years ago

@asm89 The only requirement for integration into Symfony would be to convert RequestId into a proper listener.

psampaz commented 8 years ago

this would be a great feature :+1:

psampaz commented 8 years ago

I'm interested in working on this one and create a proper listener as @fabpot proposed, but I need to clarify some things first:

[Event] kernel.request
+-------+----------------------------------------------------------------------------------+
| Order | Callable                                                                         |
+-------+----------------------------------------------------------------------------------+
| #1    | Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::configure()    |
| #2    | Symfony\Component\HttpKernel\EventListener\ProfilerListener::onKernelRequest()   |
| #3    | Symfony\Component\HttpKernel\EventListener\DumpListener::configure()             |
| #4    | Symfony\Bundle\FrameworkBundle\EventListener\SessionListener::onKernelRequest()  |
| #5    | Symfony\Component\HttpKernel\EventListener\FragmentListener::onKernelRequest()   |
| #6    | Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest()     |
| #7    | Symfony\Component\HttpKernel\EventListener\LocaleListener::onKernelRequest()     |
| #8    | Symfony\Component\HttpKernel\EventListener\TranslatorListener::onKernelRequest() |
| #9    | Symfony\Component\Security\Http\Firewall::onKernelRequest()                      |
| #10   | Symfony\Bundle\AsseticBundle\EventListener\RequestListener::onKernelRequest()    |
+-------+----------------------------------------------------------------------------------+

thanks

dosten commented 8 years ago

@phramz I think that he's talking about (apache|nginx|etc) logs.

nicolas-grekas commented 6 years ago

What is the proposed way to generate unique ids

http://php.net/random_bytes

babaorum commented 6 years ago

Since, this issue is still open I guess you are still searching for an appropriate way to do this. I just add the following monolog processor for my current project :

<?php

namespace App\Monolog\Processor;

class RequestIdProcessor
{
    private const HEADER = 'X-Request-Id';

    /**
     * @var null|string 
     */
    private $requestId;

    public function __construct(RequestStack $requestStack)
    {
        $this->requestId = $this->id($requestStack);
    }

    public function processRecord(array $record): array
    {
        $record['extra']['request_id'] = $this->requestId;

        return $record;
    }

    private function id(RequestStack $requestStack): string
    {
        $request = $requestStack->getCurrentRequest();
        if ($request === null || !$request->headers->has(self::HEADER)) {
            return \bin2hex(\random_bytes(30));
        }

        return $request->headers->get(self::HEADER);
    }
}

Do you see any downsight on this solution ?

FYI: I used bin2hex around random bytes to get something simpler to manipulate.

fabpot commented 5 years ago

Closing as there is no traction on this issue. If one wants to submit a PR for that feature, that would be great.