Skip to content

Commit

Permalink
fix(trace_stats): ensures stats respect the report_hostname configura…
Browse files Browse the repository at this point in the history
…tion [backport 2.5] (#8227)

Backport 306d718 from #8130 to 2.5.

This fix will mitigate cardinality issues in internal services. The
hostname will be set by the Datadog Agent when
`DD_TRACE_REPORT_HOSTNAME=False`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
  • Loading branch information
github-actions[bot] and mabdinur committed Jan 30, 2024
1 parent 947e7ea commit de3b9e4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
4 changes: 3 additions & 1 deletion ddtrace/internal/processor/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ def __init__(self, agent_url, interval=None, timeout=1.0, retry_attempts=3):
"Datadog-Meta-Tracer-Version": ddtrace.__version__,
"Content-Type": "application/msgpack",
} # type: Dict[str, str]
self._hostname = compat.ensure_text(get_hostname())
self._hostname = ""
if config.report_hostname:
self._hostname = get_hostname()
self._lock = Lock()
self._enabled = True

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
tracing: Ensures hostnames are reported in statsd metrics if ``DD_TRACE_REPORT_HOSTNAME=True`` (default value is ``False``).
25 changes: 24 additions & 1 deletion tests/integration/test_trace_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
from typing import Generator # noqa:F401

import mock
import pytest

from ddtrace import Tracer
Expand Down Expand Up @@ -97,13 +98,35 @@ def test_compute_stats_default_and_configure(run_python_code_in_subprocess, envv
from ddtrace import config
from ddtrace.internal.processor.stats import SpanStatsProcessorV06
assert config._trace_compute_stats is True
assert any(isinstance(p, SpanStatsProcessorV06) for p in tracer._span_processors)
stats_processor = None
for p in tracer._span_processors:
if isinstance(p, SpanStatsProcessorV06):
stats_processor = p
break
assert stats_processor is not None
assert stats_processor._hostname == "" # report_hostname is disabled by default
""",
env=env,
)
assert status == 0, out + err


@mock.patch("ddtrace.internal.processor.stats.get_hostname")
def test_stats_report_hostname(get_hostname):
get_hostname.return_value = "test-hostname"

# Enable report_hostname
with override_global_config(dict(report_hostname=True)):
p = SpanStatsProcessorV06("http://localhost:8126")
assert p._hostname == "test-hostname"

# Disable report_hostname
with override_global_config(dict(report_hostname=False)):
p = SpanStatsProcessorV06("http://localhost:8126")
assert p._hostname == ""


# Can't use a value between 0 and 1 since sampling is not deterministic.
@pytest.mark.parametrize("sample_rate", [1.0, 0.0])
@pytest.mark.snapshot()
Expand Down

0 comments on commit de3b9e4

Please sign in to comment.