ddtrace/tracer: put hostname detection behind a compat environment variable

[darccio]

What does this PR do?

Sets hostname detection off by default and behind a compat environment variable DD_TRACE_CLIENT_HOSTNAME_COMPAT to allow customers relying in this feature - if any - to activate it. Valid value to use it is v1.66.0.

DD_TRACE_CLIENT_HOSTNAME_COMPAT=v1.66.0

Deprecates the environment variable DD_CLIENT_HOSTNAME_ENABLED because it wasn't publicly documented, as it was part of an experiment.

In case somebody internally wants to keep generating the _dd.tracer_hostname, the new environment variable will allow them to do it, but we are defaulting to disable it to reduce impact in customers, as benchmarks show.

Note: this feature will be deleted after 6 releases, unless we find people relying on it.

Motivation

Hostname detection on the tracer was introduced as an experiment. After some time, we've decided to deprecate this by setting it off by default as it's causing some nuisances in customers.

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.
• Add an appropriate team label so this PR gets put in the right place for the release notes.
• Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-07-15 08:10:22

Comparing candidate commit f713b0f in PR branch dario.castane/ait-10005/remove-imds with baseline commit a87c895 in branch main.

Found 4 performance improvements and 1 performance regressions! Performance is the same for 42 metrics, 0 unstable metrics.

scenario:BenchmarkHttpServeTrace-24

• 🟥 allocated_mem [+718 bytes; +756 bytes] or [+8.353%; +8.797%]
• 🟩 execution_time [-873.863ns; -727.937ns] or [-5.412%; -4.508%]

scenario:BenchmarkOTelApiWithCustomTags/datadog_otel_api-24

• 🟩 allocated_mem [-187 bytes; -179 bytes] or [-4.964%; -4.743%]

scenario:BenchmarkOTelApiWithCustomTags/otel_api-24

• 🟩 allocated_mem [-179 bytes; -167 bytes] or [-3.565%; -3.324%]

scenario:BenchmarkTracerAddSpans-24

• 🟩 execution_time [-124.405ns; -99.595ns] or [-3.084%; -2.469%]

[darccio]

scenario:BenchmarkHttpServeTrace-24

• 🟥 allocated_mem [+718 bytes; +756 bytes] or [+8.353%; +8.797%]

This seems to not happen locally. I observe that the benchmark in main yields a higher memory consumption per operation, and both branches have the same number of allocations.

[mtoffl01]

Can you provide an example of how to use this env var? Looks like you have to pass a tracer version? I don't quite understand that. (Note: I assume this may be a Go standard thing that I'm not privy to)
Also, looks like this PR deprecates DD_CLIENT_HOSTNAME_ENABLED, right?

[darccio]

Can you provide an example of how to use this env var? Looks like you have to pass a tracer version? I don't quite understand that. (Note: I assume this may be a Go standard thing that I'm not privy to)

It's a pattern we discussed in the guild, as we were considering how to introduce breaking changes like in #2577. I updated the PR description with a usage example.

Also, looks like this PR deprecates DD_CLIENT_HOSTNAME_ENABLED, right?

Yes, it deprecates it. It was part of an experimental feature.