From f8f6d29e6424352a12d28b7622c099d48b5380ce Mon Sep 17 00:00:00 2001 From: Alexandre Choura <42672104+PROFeNoM@users.noreply.github.com> Date: Tue, 19 Dec 2023 09:12:25 +0100 Subject: [PATCH] Fix analytics.event override (#2409) * fix: Explicitly check for true !(case sensitive) when overriding analytics event * Mimics Go's `strconv.ParseBool` & add tests * style: Refactor * Don't set the '_dd1.sr.eausr' on parsing failure * Don't set the '_dd1.sr.eausr' on parsing failure --- ext/serializer.c | 51 +++++++++++++++++-- .../Integration/API/TracerTest.php | 46 +++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/ext/serializer.c b/ext/serializer.c index ad7527796d..07d697bdb9 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1150,6 +1150,46 @@ void ddtrace_shutdown_span_sampling_limiter(void) { zend_hash_destroy(&dd_span_sampling_limiters); } +// ParseBool returns the boolean value represented by the string. +// It accepts 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False. +// Any other value returns -1. +static zend_always_inline double strconv_parse_bool(zend_string *str) { + // See Go's strconv.ParseBool + // https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/strconv/atob.go;drc=1f137052e4a20dbd302f947b1cf34cdf4b427d65;l=10 + size_t len = ZSTR_LEN(str); + if (len == 0) { + return -1; + } + + char *s = ZSTR_VAL(str); + switch (len) { + case 1: + switch (s[0]) { + case '1': + case 't': + case 'T': + return 1; + case '0': + case 'f': + case 'F': + return 0; + } + break; + case 4: + if (strcmp(s, "TRUE") == 0 || strcmp(s, "True") == 0 || strcmp(s, "true") == 0) { + return 1; + } + break; + case 5: + if (strcmp(s, "FALSE") == 0 || strcmp(s, "False") == 0 || strcmp(s, "false") == 0) { + return 0; + } + break; + } + + return -1; +} + void ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { bool is_root_span = span->std.ce == ddtrace_ce_root_span_data; @@ -1273,12 +1313,17 @@ void ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { if (analytics_event) { zval analytics_event_as_double; if (Z_TYPE_P(analytics_event) == IS_STRING) { - ZVAL_DOUBLE(&analytics_event_as_double, zend_is_true(analytics_event)); // 'true' => 1.0, false => 0.0 + double parsed_analytics_event = strconv_parse_bool(Z_STR_P(analytics_event)); + if (parsed_analytics_event >= 0) { + ZVAL_DOUBLE(&analytics_event_as_double, parsed_analytics_event); + zend_array *metrics = ddtrace_property_array(&span->property_metrics); + zend_hash_str_add_new(metrics, ZEND_STRL("_dd1.sr.eausr"), &analytics_event_as_double); + } } else { ZVAL_DOUBLE(&analytics_event_as_double, zval_get_double(analytics_event)); + zend_array *metrics = ddtrace_property_array(&span->property_metrics); + zend_hash_str_add_new(metrics, ZEND_STRL("_dd1.sr.eausr"), &analytics_event_as_double); } - zend_array *metrics = ddtrace_property_array(&span->property_metrics); - zend_hash_str_add_new(metrics, ZEND_STRL("_dd1.sr.eausr"), &analytics_event_as_double); zend_hash_str_del(meta, ZEND_STRL("analytics.event")); } diff --git a/tests/OpenTelemetry/Integration/API/TracerTest.php b/tests/OpenTelemetry/Integration/API/TracerTest.php index 59355c419c..b69e69eb46 100644 --- a/tests/OpenTelemetry/Integration/API/TracerTest.php +++ b/tests/OpenTelemetry/Integration/API/TracerTest.php @@ -336,6 +336,52 @@ public function providerSpanKind() ]; } + public function providerAnalyticsEvent() + { + return [ + ["true", 1], + ["TRUE", 1], + ["True", 1], + ["false", 0], + ["False", 0], + ["FALSE", 0], + ["something-else", null], + [True, 1], + [False, 0], + ['t', 1], + ['T', 1], + ['f', 0], + ['F', 0], + ['1', 1], + ['0', 0], + ['fAlse', null], + ['trUe', null] + ]; + } + + /** + * @dataProvider providerAnalyticsEvent + */ + public function testReservedAttributesOverridesAnalyticsEvent($analyticsEventValue, $expectedMetricValue) + { + $traces = $this->isolateTracer(function () use ($analyticsEventValue) { + $tracer = self::getTracer(); + $span = $tracer->spanBuilder('operation') + ->setSpanKind(SpanKind::KIND_SERVER) + ->startSpan(); + $span->setAttribute('analytics.event', $analyticsEventValue); + $span->end(); + }); + + $span = $traces[0][0]; + if ($expectedMetricValue !== null) { + $actualMetricValue = $span['metrics']['_dd1.sr.eausr']; + $this->assertEquals($expectedMetricValue, $actualMetricValue); + } else { + $this->assertArrayNotHasKey('_dd1.sr.eausr', $span['metrics']); + } + } + public function testSpanErrorStatus() { $traces = $this->isolateTracer(function () {