Closed PROFeNoM closed 2 weeks ago
Attention: Patch coverage is 57.14286%
with 21 lines
in your changes missing coverage. Please review.
Project coverage is 80.94%. Comparing base (
31382ec
) to head (e44f4e4
).
Files with missing lines | Patch % | Lines |
---|---|---|
src/DDTrace/OpenTelemetry/Span.php | 59.09% | 18 Missing :warning: |
src/DDTrace/OpenTelemetry/Context.php | 40.00% | 3 Missing :warning: |
Benchmark execution time: 2024-09-16 08:08:18
Comparing candidate commit e4b186a5ae9a1705aeee5209318aa7293b56a948 in PR branch alex/fix/otel-addlink-fiber
with baseline commit 1a7182ce5f844d39cd203191bc2d0d5e07870405 in branch master
.
Found 3 performance improvements and 5 performance regressions! Performance is the same for 170 metrics, 0 unstable metrics.
execution_time
[+86.391µs; +206.589µs] or [+2.856%; +6.830%]execution_time
[-13.842µs; -11.256µs] or [-7.187%; -5.844%]execution_time
[-14.699µs; -12.174µs] or [-4.740%; -3.926%]execution_time
[+333.793ns; +713.207ns] or [+4.872%; +10.410%]execution_time
[+149.073ns; +441.727ns] or [+2.140%; +6.340%]execution_time
[+142.816ns; +503.984ns] or [+2.102%; +7.419%]execution_time
[+3.996µs; +7.604µs] or [+2.374%; +4.518%]execution_time
[-21.847ms; -21.541ms] or [-83.051%; -81.888%]I'm putting it back to draft. +7-9% performance regression for a simple check is simply too much. I'll try and optimize that a little bit.
@PROFeNoM Feels like the benchmarks are simply flaky. ... But I suppose for that one the micro-optimization is worth it.
Benchmark execution time: 2024-09-16 13:24:15
Comparing candidate commit e44f4e456c07253aff91cad77cbc68921fadb820 in PR branch alex/fix/otel-addlink-fiber
with baseline commit 31382ec7e2f822cf156883b11e249f4bfd4db72e in branch master
.
Found 2 performance improvements and 4 performance regressions! Performance is the same for 172 metrics, 0 unstable metrics.
execution_time
[+188.236ns; +617.764ns] or [+2.531%; +8.307%]execution_time
[+183.937ns; +568.063ns] or [+2.446%; +7.554%]execution_time
[-439.363ns; -243.637ns] or [-5.657%; -3.137%]execution_time
[+9.677µs; +14.422µs] or [+5.432%; +8.096%]execution_time
[+14.523µs; +17.533µs] or [+5.064%; +6.114%]execution_time
[-21.839ms; -21.599ms] or [-82.871%; -81.961%]
Description
SpanInterface::addLink
was added with https://github.com/open-telemetry/opentelemetry-php/pull/1289 and is included in 1.1+. It is currently missing from our shim. For backward compatibility reasons,ImmutableSpan
will take a new argument,totalRecordedLinks
if the latter method exists.test_opentelemetry_1
suite (currently:@stable
) will be split into two:@beta
and1.0.*
.Reviewer checklist