dd-trace-php icon indicating copy to clipboard operation
dd-trace-php copied to clipboard

fix: ext/parallel support

Open morrisonlevi opened this issue 1 month ago • 2 comments

Description

Fixes #3511. Maybe, I am not an expert in this config area, but it seems plausible:

  • ext/parallel makes new threads, and config levels including log levels need to be set there.
  • Those ext/parallel threads do not call minit/mshutdown, but do call rinit/rshutdown.

Once I have the build artifacts, I can do more testing.

Reviewer checklist

  • [ ] Test coverage seems ok.
  • [ ] Appropriate labels assigned.

morrisonlevi avatar Dec 01 '25 23:12 morrisonlevi

First hiccup:

    17	static void dd_log_set_level(bool debug) {
    18	    bool once = runtime_config_first_init ? get_DD_TRACE_ONCE_LOGS() : get_global_DD_TRACE_ONCE_LOGS();
    19	    if (debug) {
    20	        if (strcmp("cli", sapi_module.name) != 0 && (runtime_config_first_init ? get_DD_TRACE_STARTUP_LOGS() : get_global_DD_TRACE_STARTUP_LOGS())) {
    21	            ddog_set_log_level(DDOG_CHARSLICE_C("debug"), once);
    22	        } else {
    23	            ddog_set_log_level(DDOG_CHARSLICE_C("debug,startup=error"), once);
    24	        }
    25	    } else if (runtime_config_first_init) {
    26	        ddog_set_log_level(dd_zend_string_to_CharSlice(get_DD_TRACE_LOG_LEVEL()), once);
    27	    } else if (zend_string_equals_literal_ci(Z_STR(zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_LOG_LEVEL].decoded_value), "error")) {
    28	        ddog_set_error_log_level(once); // optimized handling without parsing
    29	    } else {
    30	        ddog_set_log_level(dd_zend_string_to_CharSlice(get_global_DD_TRACE_LOG_LEVEL()), once);
    31	    }
    32	}

Line 27 reads uninitialized memory.

morrisonlevi avatar Dec 01 '25 23:12 morrisonlevi

Benchmarks [ tracer ]

Benchmark execution time: 2025-12-02 00:17:50

Comparing candidate commit 25a53502ae7fc2850959b9cf7c0bab8aee0af8b1 in PR branch levi/gh-3511 with baseline commit 437fc466eda8c2970bc9c110a169297592e7d3f2 in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 190 metrics, 1 unstable metrics.

scenario:ComposerTelemetryBench/benchTelemetryParsing

  • 🟥 execution_time [+547.366ns; +1452.634ns] or [+5.164%; +13.704%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-3.536µs; -2.464µs] or [-3.397%; -2.367%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-55.607µs; -39.293µs] or [-12.300%; -8.691%]

pr-commenter[bot] avatar Dec 02 '25 00:12 pr-commenter[bot]

I'm going to close this. As a non-expert in config, it's unclear how to fix it. I will shift to other bugs that are more actionable for me.

morrisonlevi avatar Dec 16 '25 16:12 morrisonlevi