-
Notifications
You must be signed in to change notification settings - Fork 1
Fixed: Eliminate tracer auto-flush latency; add periodic flush listener #3
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR disables automatic tracer flushing in OpenTelemetry configuration and implements manual flush control. The changes prevent automatic batch processor flushing while adding explicit flush handling in the middleware layer.
Key changes:
- Disables auto flush for batch processors (traces and logs)
- Adds manual flush control in TraceMiddleware using defer()
- Refactors TracerProviderFactory to support conditional instantiation based on configuration
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Middleware/TraceMiddleware.php | Adds deferred manual flush with error handling |
| src/Factory/Trace/TracerProviderFactory.php | Converts to factory pattern with conditional NoopTracerProvider |
| src/Factory/Trace/Processor/BatchSpanProcessorFactory.php | Disables auto flush and fixes max queue size constant |
| src/Factory/SDKBuilder.php | Updates to use injected TracerProviderInterface |
| src/Factory/Log/Processor/BatchLogProcessorFactory.php | Disables auto flush and fixes max queue size constant |
| src/ConfigProvider.php | Registers TracerProviderInterface binding |
| publish/open-telemetry.php | Updates configuration defaults and environment variable names |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Description
Context
Synchronous flushing of traces/metrics at the end of a request (auto-flush) was adding latency and, in some cases, causing races with spans that finish right at the tail end of the coroutine. In Hyperf/Swoole environments, the hot path must remain non-blocking.
What changed
auto_flushfrom the BatchSpanProcessor (traces) and any implicit flush in the hot path.forceFlush()periodically inside dedicated coroutines (viaCoordinator\Timer), keeping the request flow non-blocking.BeforeWorkerStart(web) andBeforeHandle(CLI).AfterHandle(CLI) andWORKER_EXIT(web).forceFlush()between ticks.AbstractFlushListenercentralizes lifecycle (start/stop/lock/logs);TraceFlushListenerandMetricFlushListeneronly implementflush()and the interval.Type of change
Checklist
feat: add SQS aspect)composer testpasses locally