[Bug]: upstream sent invalid header: "ot-baggage-true-client-ip\x20..." after upgrading

[greg0ire]

Bug report

After upgrading to v1.15.2, I am getting 502 with the following message from nginx:

upstream sent invalid header: "ot-baggage-true-client-ip\x20...

PHP version

8.4.17

Tracer or profiler version

v1.15.2

Installed extensions

[PHP Modules]
amqp
bcmath
calendar
Core
ctype
curl
datadog-profiling
date
ddappsec
ddtrace
dom
exif
FFI
fileinfo
filter
ftp
gd
gettext
hash
iconv
igbinary
imagick
intl
json
libxml
mbstring
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
pdo_pgsql
pdo_sqlite
pgsql
Phar
posix
random
readline
redis
Reflection
session
shmop
SimpleXML
soap
sockets
sodium
SPL
sqlite3
standard
sysvmsg
sysvsem
sysvshm
tokenizer
xml
xmlreader
xmlrpc
xmlwriter
xsl
yaml
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache
datadog-profiling
ddappsec
ddtrace

Output of phpinfo()

No response

Upgrading from

v1.6.0

[bwoebi]

Hey @greg0ire,

I'm not quite sure what's up here. true-client-ip comes from our ip_extraction logic, which was introduced in 0.98, and the ot-baggage- comes from the legacy TextMap propagator (opentracing baggage).
I don't know why it would suddenly have changed.

I think I need a bit more details how you use the tracer, to understand the underlying issue.

[greg0ire]

Which kind of details.
Here is how we create the tracer, in case it helps:

        if (ddtrace_config_trace_enabled() && class_exists("\DDTrace\GlobalTracer") && class_exists("\DDTrace\OpenTracer1\Tracer")) {
            $otTracer = new \DDTrace\OpenTracer1\Tracer(\DDTrace\GlobalTracer::get());
            GlobalTracer::set($otTracer);
        }

        return GlobalTracer::get();

[bwoebi]

The thing which did change between v1.6 and current is that we also track baggage in the extension directly, and not just in PHP code.
Could you check the baggage you have on your span, where you inject? var_dump(\DDTrace\active_span()->baggage);
Is there any space in this baggage?

Also, check the result of the ->inject() - is there some space in the $carrier array (second parameter)?

Maybe this baggage was always empty in the past and it no longer is? And the code after the inject() (i.e. not ours) is misbehaving?
I don't know where to look for exactly. Hence I need you to help debugging a bit :-)

[greg0ire]

It's only on an environment where I can't iterate quickly, and I will be AFK for ~ 1 week in 1 hour. I will try again when I come back, but I'm thinking it could be great to add some kind of post condition/header validation that crashes rather than sending an invalid header.

[bwoebi]

@greg0ire I agree that TextMap should probably mangle invalid baggage keys. (e.g. a simple regex replacing invalid header bytes by dashes)