Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel: tracing Queues (+ hook refactoring) #250

Merged
merged 51 commits into from
Apr 25, 2024

Conversation

ChrisLightfootWild
Copy link
Contributor

@ChrisLightfootWild ChrisLightfootWild commented Apr 2, 2024

Tracing Laravel Queues

I have added some message tracing to Laravel.

Ref: https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/

An example trace:

image


The above screenshot was produced with the following Laravel test code:

Route::get('/job', function (\Illuminate\Contracts\Queue\Queue $queue) {
    logger()->info('Start request');

    $queue->push(new \App\Jobs\TestJob());

    $queue->bulk([
        new \App\Jobs\TestJob(),
        new \App\Jobs\TestJob(),
        new \App\Jobs\TestJob(),
        new \App\Jobs\TestJob(),
        new \App\Jobs\TestJob(),
    ]);

    $queue->later(
        now()->addSeconds(5),
        new \App\Jobs\TestJob(),
    );

    dispatch_sync(new \App\Jobs\TestJob());

    return [
        'queue' => config('queue.default'),
        'time' => time(),
    ];
});

and the following test job:

<?php

namespace App\Jobs;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Queue\InteractsWithQueue;

class TestJob implements ShouldQueue
{
    use InteractsWithQueue, Queueable;

    public function handle(): void
    {
        logger()->info('Start job');

        logger()->info('Job info', [
            'job' => $this->job->getRawBody(),
            'trace' => \OpenTelemetry\API\Trace\Span::getCurrent()->getContext()->getTraceId(),
        ]);

        logger()->debug('Working...', [
            'sleep' => $sleep = rand(10, 1000),
        ]);

        usleep($sleep * 1000);
        logger()->info('End job');
    }
}

This is an example of a successful read from the queue:
image

Trace whereby no message received, thus the span was dropped:
image

I tried linking this span to a different parent by extracting the traceparent, but it didn't seem possible to change the existing span's parent.

Refactoring

During the course of this PR, I decided to try and organise the numerous additional hooks in a way which reflects the layout of the Laravel framework. I hope that this might ultimately make it easier to add further hooks in future.

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 55.12465% with 162 lines in your changes are missing coverage. Please review.

Project coverage is 82.47%. Comparing base (2a601c3) to head (a3e9dba).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #250      +/-   ##
============================================
- Coverage     84.38%   82.47%   -1.92%     
+ Complexity      983      648     -335     
============================================
  Files            89       61      -28     
  Lines          3946     2630    -1316     
============================================
- Hits           3330     2169    -1161     
+ Misses          616      461     -155     
Flag Coverage Δ
7.4 ?
8.0 79.82% <ø> (-3.56%) ⬇️
8.1 ?
8.2 75.73% <55.12%> (-8.58%) ⬇️
8.3 82.52% <55.12%> (-1.79%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...el/src/Hooks/Illuminate/Foundation/Application.php 100.00% <100.00%> (ø)
...tion/Laravel/src/Propagators/HeadersPropagator.php 42.85% <ø> (ø)
...avel/src/Propagators/ResponsePropagationSetter.php 0.00% <ø> (ø)
...trumentation/Laravel/src/Watchers/CacheWatcher.php 88.37% <ø> (ø)
...tion/Laravel/src/Watchers/ClientRequestWatcher.php 96.00% <ø> (ø)
...entation/Laravel/src/Watchers/ExceptionWatcher.php 46.15% <ø> (ø)
...nstrumentation/Laravel/src/Watchers/LogWatcher.php 92.30% <ø> (ø)
...trumentation/Laravel/src/Watchers/QueryWatcher.php 95.65% <ø> (ø)
...vel/src/Hooks/Illuminate/Contracts/Http/Kernel.php 84.50% <89.47%> (ø)
...n/Laravel/src/Hooks/Illuminate/Queue/SyncQueue.php 92.30% <92.30%> (ø)
... and 10 more

... and 40 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac8ab0d...a3e9dba. Read the comment docs.

@ChrisLightfootWild ChrisLightfootWild changed the title Laravel: start of QueueWatcher. Laravel: tracing Queues. Apr 10, 2024
@ChrisLightfootWild ChrisLightfootWild changed the title Laravel: tracing Queues. Laravel: tracing Queues (+ phan lint) Apr 21, 2024
@ChrisLightfootWild ChrisLightfootWild requested review from agoallikmaa, brettmc and a team April 22, 2024 19:16
@ChrisLightfootWild ChrisLightfootWild changed the title Laravel: tracing Queues (+ phan lint) Laravel: tracing Queues (+ hook refactoring) Apr 22, 2024
"require": {
"php": "^8.0",
"ext-json": "*",
"ext-opentelemetry": "*",
"laravel/framework": ">=6.0",
"laravel/framework": "^6.0 || ^7.0 || ^8.0 || ^9.0 || ^10.0 || ^11.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Source: https://laravel.com/docs/11.x/releases#support-policy

Listing the currently supported versions explicitly. I'd like to drop versions 6-9 after we make a 1.0 release of the package so that we can align with the API on PHP ^8.1.

@ChrisLightfootWild ChrisLightfootWild requested a review from a team April 24, 2024 16:54
@brettmc brettmc merged commit 491827d into open-telemetry:main Apr 25, 2024
106 of 107 checks passed
@ChrisLightfootWild ChrisLightfootWild deleted the laravel/queues branch April 25, 2024 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants