Adding OpenTelemetry and provide traces - #4598
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@FredPraca I am not able to push changes to enable the CICD missing extension. Would you mind ticking the box or modify the php-test and php-dist yaml files :) |
The Allow edits by maintainers is checked so I don't know why you're unable to change it. The main problem is that opentelemetry extensions is not a standard one. The process is described here but I don't know if I can do something like this Github actions. |
|
I'm also trying to include a package providing metrics about PHP runtime, especially RAM but I face a problem with initialization order. |
f9637f6 to
47907fc
Compare
| { | ||
| $meta = $request->meta(); | ||
| $file = new UploadedFile($request->uploaded_file_chunk()); | ||
| // Reset max memory usage before measuring |
There was a problem hiding this comment.
Should this be in a middleware instead?
There was a problem hiding this comment.
I'll read it as I'm not aware of all Laravel possibilities and especially with the middleware and so on stuff.
Besides this, I was not super happy with this way of tracing memory usage.
So we might maybe leave the trace without performing these memory features and keep tracing telemetry for what it is, following calls durations.
Your way of profiling might be a better way that we may merge with telemetry afterwards.
What do you think ?
| $final->delete(); | ||
| $processable_file->close(); | ||
| // End of work-around | ||
| return $this->trace->traceMethod('photo.process', function () use ($verify, |
There was a problem hiding this comment.
It is important to note that here you are measuring either of two cases:
if the processing is done sync, you will have the full processing memory, but if it is dispatch to a queue you won't see the computation anymore.
There was a problem hiding this comment.
Yes, I figured it out and maybe we will use traces to provide time consumption instead of memory consumption.
That's the reason why I would like to import the php metrics module and experiment around it.
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
2 participants
This PR provides OpenTelemetry functionalities to Lychee.
It allows to send logs, metrics and traces to any OpenTelemetry collector.
It has been tested with Loki, Tempo and Mimir for logs, traces and metrics respectively.
The best way to test it is to use Grafana Alloy as a collector and then dispatch to other tools such as the ones mentioned before.
It requires to install the opentelemetry extension as described here.
The following environment variables may be set:
Please note that exporters may be set to console or otlp. When using otlp, you may provide the OTEL_EXPORTER_OTLP_ENDPOINT variable to point to an OTLP collector. Here http://localhost:4318/ corresponds to a local setup for Tempo.
Moreover, OTEL_PHP_DISABLED_INSTRUMENTATIONS allows to disable some instrumentation. Here, I disabled pdo to avoid having all SQL operations in my traces. Currently, only laravel, guzzle and pdo are configured.