fix: correct unit mismatch in php.compilation.total_time_ms#3915
Open
dortort wants to merge 1 commit into
Open
fix: correct unit mismatch in php.compilation.total_time_ms#3915dortort wants to merge 1 commit into
dortort wants to merge 1 commit into
Conversation
PR DataDog#2230 replaced _get_microseconds() (which returned microseconds via clock_gettime) with zend_hrtime() (which returns nanoseconds) without updating the accumulator. The serializer divides by 1000 expecting microseconds, so the metric has been reporting microseconds labeled as milliseconds since v0.98.0. Convert nanoseconds to microseconds at the accumulator to restore correct behavior, matching the pattern used in circuit_breaker.c from the same PR. Fixes DataDog#3914
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR #2230 (v0.98.0) replaced
_get_microseconds()(which usedclock_gettimeand returned microseconds) withzend_hrtime()(which returns nanoseconds) inext/engine_hooks.c, but did not update the accumulator. The serializer inext/serializer.cdivides by1000.expecting microseconds → milliseconds, so since v0.98.0 the metricphp.compilation.total_time_mshas been reporting microseconds labeled as milliseconds — a 1000x inflation.The same PR correctly handled this conversion in
circuit_breaker.c(return zend_hrtime() / 1000;). This change applies the same pattern to the compile time accumulator.Evidence
A request with a total duration of ~4,350 ms reported
php.compilation.total_time_msof ~28,346 — impossible if in milliseconds (exceeds request duration), but consistent with ~28.3 ms when interpreted as microseconds.Fixes #3914