Move env and version from meta to span properties

[bwoebi]

As a fallback, direct assignments to $span->meta["env"] (or version) do still take precedence over the new properties. This was deprecated, and will be eventually removed.

Side note: There were tests for DD_TAGS vs DD_ENV and DD_VERSION. These tests started failing. I elected to remove the tests instead of adding special handling in ddtrace_set_global_span_properties() (if key equals env and property_env non-empty, then ignore). These tests came from a 4 year old migration, at which time these tags had to be set via DD_TAGS, thus prioritizing DD_ENV and DD_VERSION explicitly made sense. By now it would just be a waste of CPU cycles to check that.

This change is important for being able to track changes to service/env/version in the future context of remote configuration.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.72%. Comparing base (3d5be50) to head (abbbb6b).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2665      +/-   ##
============================================
+ Coverage     77.66%   77.72%   +0.05%     
- Complexity     2210     2212       +2     
============================================
  Files           225      225              
  Lines         26080    26109      +29     
  Branches        988      988              
============================================
+ Hits          20256    20292      +36     
+ Misses         5298     5291       -7     
  Partials        526      526              

Flag	Coverage Δ
appsec-extension	69.13% <ø> (ø)
tracer-extension	78.55% <100.00%> (+0.09%)	⬆️
tracer-php	80.30% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
ext/compat_string.c	60.86% <100.00%> (+0.45%)	⬆️
ext/compatibility.h	76.82% <ø> (ø)
ext/configuration.h	100.00% <ø> (ø)
ext/ddtrace.c	73.65% <100.00%> (+0.06%)	⬆️
ext/ddtrace.h	62.50% <ø> (ø)
ext/ddtrace_arginfo.h	100.00% <100.00%> (ø)
ext/priority_sampling/priority_sampling.c	95.62% <100.00%> (+0.04%)	⬆️
ext/serializer.c	81.50% <100.00%> (+1.02%)	⬆️
ext/span.c	92.82% <100.00%> (-0.19%)	⬇️
ext/span.h	100.00% <ø> (ø)
... and 2 more

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d5be50...abbbb6b. Read the comment docs.

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-05-15 16:26:51

Comparing candidate commit abbbb6b in PR branch bob/version-env-props with baseline commit 3d5be50 in branch master.

Found 10 performance improvements and 2 performance regressions! Performance is the same for 166 metrics, 0 unstable metrics.

scenario:ContextPropagationBench/benchExtractTraceContext128Bit-opcache

• 🟥 execution_time [+58.771ns; +127.229ns] or [+2.603%; +5.635%]

scenario:HookBench/benchHookOverheadInstallHookOnFunction-opcache

• 🟩 execution_time [-55.089µs; -17.528µs] or [-6.896%; -2.194%]

scenario:HookBench/benchHookOverheadInstallHookOnMethod

• 🟩 execution_time [-56.543µs; -25.832µs] or [-6.618%; -3.024%]

scenario:HookBench/benchHookOverheadTraceFunction

• 🟩 mem_peak [-350.345KB; -350.342KB] or [-9.845%; -9.845%]

scenario:HookBench/benchHookOverheadTraceMethod

• 🟩 mem_peak [-350.344KB; -350.340KB] or [-9.671%; -9.671%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1

• 🟩 execution_time [-355.289ns; -214.311ns] or [-5.683%; -3.428%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2

• 🟩 execution_time [-407.609ns; -168.791ns] or [-6.491%; -2.688%]

scenario:SamplingRuleMatchingBench/benchRegexMatching3

• 🟩 execution_time [-292.719ns; -148.681ns] or [-4.711%; -2.393%]

scenario:SpanBench/benchDatadogAPI-opcache

• 🟩 mem_peak [-224.025KB; -224.017KB] or [-2.251%; -2.251%]

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead

• 🟩 mem_peak [-350.344KB; -350.344KB] or [-9.650%; -9.650%]

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead-opcache

• 🟩 mem_peak [-51.952KB; -51.952KB] or [-2.547%; -2.547%]

scenario:TraceSerializationBench/benchSerializeTrace

• 🟥 execution_time [+4.904µs; +9.496µs] or [+2.789%; +5.400%]

[iamluc]

There is a similar hack for the service.name tag, but handled in userland:

dd-trace-php/src/DDTrace/Span.php

Lines 152 to 154 in d602b97

	if ($key === Tag::SERVICE_NAME) {
	$this->internalSpan->service = $value;
	return;

would it be worthwhile to harmonize?

[bwoebi]

@iamluc the SERVICE_NAME is less of a hack, and more of an OpenTracing thing. I.e. it comes from an API which never directly assigns to meta. So the equivalence isn't really applicable here.