appsec: fix duration_ext metric#3507
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (77.55%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #3507 +/- ##
==========================================
- Coverage 61.74% 61.63% -0.11%
==========================================
Files 142 143 +1
Lines 12975 13003 +28
Branches 1700 1702 +2
==========================================
+ Hits 8011 8014 +3
- Misses 4204 4228 +24
- Partials 760 761 +1
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
It had three problems: * Value in ns instead of microseconds * It would effectively be the time of the first rasp call only * Did not fetch the span properly where to store the tags
cataphract
force-pushed
the
glopes/fix-duration_ext
branch
from
39b736b to
be6b02a
Compare
Benchmarks [ appsec ]Benchmark execution time: 2025-12-03 16:50:55 Comparing candidate commit d10e448 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. |
| static void _add_rasp_duration_ext_to_metrics( | ||
| zend_object *nonnull span, double duration) | ||
| { | ||
| zval *metrics_zv = dd_trace_span_get_metrics(span); | ||
| if (!metrics_zv) { | ||
| return; | ||
| } | ||
|
|
||
| zval zv; | ||
| ZVAL_DOUBLE(&zv, duration); | ||
| zval *nullable res = | ||
| zend_hash_add(Z_ARRVAL_P(metrics_zv), _dd_rasp_duration_ext, &zv); | ||
| if (res == NULL) { | ||
| mlog(dd_log_warning, "Failed to add metric %.*s", | ||
| (int)ZSTR_LEN(_dd_rasp_duration_ext), | ||
| ZSTR_VAL(_dd_rasp_duration_ext)); | ||
| } | ||
| } |
There was a problem hiding this comment.
I would keep this part on tags.c since everything related to metrics is there. Just an opinion
3 participants
It had three problems:
Description
Reviewer checklist