-
Notifications
You must be signed in to change notification settings - Fork 166
Collect framework endpoints #3548
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3548 +/- ##
=======================================
Coverage 62.01% 62.01%
=======================================
Files 140 140
Lines 13309 13309
Branches 1762 1762
=======================================
Hits 8253 8253
Misses 4268 4268
Partials 788 788 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ tracer ]Benchmark execution time: 2026-01-13 15:53:34 Comparing candidate commit d49f445 in PR branch Found 0 performance improvements and 8 performance regressions! Performance is the same for 185 metrics, 1 unstable metrics. scenario:SymfonyBench/benchSymfonyDdprof
scenario:SymfonyBench/benchSymfonyDdprof-opcache
scenario:SymfonyBench/benchSymfonyOverhead
scenario:SymfonyBench/benchSymfonyOverhead-opcache
scenario:WordPressBench/benchWordPressDdprof
scenario:WordPressBench/benchWordPressDdprof-opcache
scenario:WordPressBench/benchWordPressOverhead
scenario:WordPressBench/benchWordPressOverhead-opcache
|
be10f87 to
4dd7002
Compare
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f84f03e8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> bool { | ||
| let cache_entry = ddog_sidecar_telemetry_cache_get_or_update(cache, service, env); | ||
| let result = cache_entry.last_endpoints_push.elapsed().map_or(false, |d| d < Duration::from_secs(60)); // 1 minute |
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.
Keep endpoint collection from expiring after 60s
This predicate only treats endpoints as “collected” for 60 seconds (elapsed() < 60s), so in long‑running PHP workers the Symfony/Laravel/WordPress hooks will start re‑enumerating routes/posts and re‑emitting app‑endpoints after a minute. That contradicts the “collect once” behavior and can create duplicate telemetry plus repeated route/DB scans on every request once the minute elapses.
Useful? React with 👍 / 👎.
| def deadline = System.currentTimeSeconds() + timeoutSec | ||
| def lastHttpReq = System.currentTimeSeconds() - 6 | ||
| while (System.currentTimeSeconds() < deadline) { |
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.
Fix nonexistent System.currentTimeSeconds call
System.currentTimeSeconds() is not a Java/Groovy API, so this will throw MissingMethodException at runtime the first time waitForTelemetryData is used. That means the new endpoint telemetry tests that call waitForAppEndpoints/waitForMetrics/… will fail immediately instead of waiting on telemetry. Use System.currentTimeMillis() / 1000 or Instant.now().epochSecond instead.
Useful? React with 👍 / 👎.
7f84f03 to
4bdf65f
Compare
|
Description
This feature will collect(once) all endpoints of the following framework: symfony(except version 4), laravel and wordpress
Reviewer checklist