Set _dd.base_service when overriding service name for a span

[pablomartinezbernardo]

Description

Add _dd.base_service to child spans when child service name is updated manually. AIT-8051

Integration tests have been changed to by default have DD_TRACE_GENERATE_ROOT_SPAN=0, to minimize the amount of tests that would have needed to be modified to add by adding the _dd.base_service tag

Readiness checklist

• (only for Members) Changelog has been added to the release document.
• Tests added for this feature/bug.

Reviewer checklist

• Appropriate labels assigned.
• Milestone is set.
• Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

[bwoebi]

Just for the record, the API you are targeting is mostly the "legacy API". A common way to change the service name would be directly editing the DDTrace\SpanData object (i.e. a simple span object defined directly in C).

E.g. (snippet from CurlIntegration):

        \DDTrace\trace_function('curl_exec', [
            'posthook' => function (SpanData $span, $args, $retval) use ($integration) {
                $span->name = $span->resource = 'curl_exec';
                $span->type = Type::HTTP_CLIENT;
                $span->service = 'curl';
// ...
             },
        ]);

What you want to do is most likely checking the service of the root span during serialization (in ext/serializer.c) and then dynamically add _dd.base_service = , if the service is not equal to the root spans service.

[bwoebi]

Suggested change

	if (strcasecmp(Z_STRVAL(prop_service_as_string), Z_STRVAL(prop_root_service_as_string)) != 0) {
	if (zend_string_equals(Z_STR(prop_service_as_string), Z_STR(prop_root_service_as_string))) {

zend_string_equals is aware of the lengths directly and also does a pointer equality comparison (which likely will match if both are equal).

[pablomartinezbernardo]

👍 modified the suggestion a bit by using zend_string_equals_ci instead of zend_string_equals to keep the behavior consistent with strcasecmp

[bwoebi]

Yeah, missed that it wasn't strcmp :-)

[bwoebi]

Suggested change

	zend_hash_str_add_new(meta, ZEND_STRL("_dd.base_service"), &prop_root_service_as_string);
	zend_hash_str_update(meta, ZEND_STRL("_dd.base_service"), &prop_root_service_as_string);

Don't use _add_new if you cannot be sure that there's no user-input you could overwrite. If there's already a manually added meta key from PHP code named _dd.base_service, it'll leak. And just _add needs to be checked for success or failure to potentially free the value.

[bwoebi]

Suggested change

	zval *prop_root_service = &span->root->property_service;
	ZVAL_DEREF(prop_root_service);
	zval prop_root_service_as_string;
	ddtrace_convert_to_string(&prop_root_service_as_string, prop_root_service);
	zval prop_root_service_as_string;
	ddtrace_convert_to_string(&prop_root_service_as_string, &span->root->property_service);

ddtrace_convert_to_string is able to handle refs directly

[bwoebi]

Generally such code just inserting into meta should be probably moved into _serialize_meta directly and avoid altering the meta of the original span itself.

[bwoebi]

Looks good to me now :-)

[bwoebi]

Wait, this needs a refcount increase, or the release below needs to be moved into an else.

[github-actions]

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.