Skip to content

Commit

Permalink
Merge branch 'main' into cbeauchesne/crash-investigation
Browse files Browse the repository at this point in the history
  • Loading branch information
juanjux authored Jul 25, 2024
2 parents 79ce71f + 3e1a49a commit 3486631
Show file tree
Hide file tree
Showing 16 changed files with 168 additions and 22 deletions.
1 change: 1 addition & 0 deletions .gitlab/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ variables:
UPSTREAM_COMMIT_SHA: $CI_COMMIT_SHA # The commit revision the project is built for.
KUBERNETES_SERVICE_ACCOUNT_OVERWRITE: dd-trace-py
FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY: "true"
CARGO_NET_GIT_FETCH_WITH_CLI: "true" # use system git binary to pull git dependencies

benchmarks-pr-comment:
stage: benchmarks-pr-comment
Expand Down
7 changes: 7 additions & 0 deletions ddtrace/_trace/_span_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,10 @@ def to_dict(self):
d["flags"] = self.flags

return d

def __str__(self) -> str:
attrs_str = ",".join([f"{k}:{v}" for k, v in self.attributes.items()])
return (
f"trace_id={self.trace_id} span_id={self.span_id} attributes={attrs_str} "
f"tracestate={self.tracestate} flags={self.flags} dropped_attributes={self._dropped_attributes}"
)
6 changes: 6 additions & 0 deletions ddtrace/_trace/span.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ def __dict__(self):
d["attributes"] = self.attributes
return d

def __str__(self):
attrs_str = ",".join([f"{k}:{v}" for k, v in self.attributes.items()])
return f"name={self.name} time={self.time_unix_nano} attributes={attrs_str}"


log = get_logger(__name__)

Expand Down Expand Up @@ -569,6 +573,8 @@ def _pprint(self) -> str:
("error", self.error),
("tags", dict(sorted(self._meta.items()))),
("metrics", dict(sorted(self._metrics.items()))),
("links", ", ".join([str(link) for _, link in self._links.items()])),
("events", ", ".join([str(e) for e in self._events])),
]
return " ".join(
# use a large column width to keep pprint output on one line
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/contrib/trace_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def _store_headers(headers, span, integration_config, request_or_response):
return

for header_name, header_value in headers.items():
"""config._header_tag_name gets an element of the dictionary in config.http._header_tags
"""config._header_tag_name gets an element of the dictionary in config.trace_http_header_tags
which gets the value from DD_TRACE_HEADER_TAGS environment variable."""
tag_name = integration_config._header_tag_name(header_name)
if tag_name is None:
Expand Down
10 changes: 8 additions & 2 deletions ddtrace/internal/utils/formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ def parse_tags_str(tags_str):
The expected string is of the form::
"key1:value1,key2:value2"
"key1:value1 key2:value2"
"key1,key2"
"key1 key2"
:param tags_str: A string of the above form to parse tags from.
:return: A dict containing the tags that were parsed.
Expand All @@ -86,10 +88,14 @@ def parse_tags(tags):

for tag in tags:
key, sep, value = tag.partition(":")
if not sep or not key or "," in key:
if not key.strip() or "," in key or (sep and not value):
invalids.append(tag)
else:
elif sep:
# parse key:val,key2:value2
parsed_tags.append((key, value))
else:
# parse key,key2
parsed_tags.append((key, ""))

return parsed_tags, invalids

Expand Down
13 changes: 10 additions & 3 deletions ddtrace/settings/crashtracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@
from ddtrace.internal.utils.formats import parse_tags_str


resolver_default = "full"


def _derive_stacktrace_resolver(config: "CrashtrackerConfig") -> t.Optional[str]:
resolver = str(config._stacktrace_resolver or "")
resolver = resolver.lower()
if resolver == "none":
return None
if resolver in ("fast", "full", "safe"):
return resolver
return None

# Invalid values should degrade to the default
return resolver_default


def _check_for_crashtracker_available() -> bool:
Expand Down Expand Up @@ -75,10 +82,10 @@ class CrashtrackerConfig(En):
_stacktrace_resolver = En.v(
t.Optional[str],
"stacktrace_resolver",
default=None,
default=resolver_default,
help_type="String",
help="How to collect native stack traces during a crash, if at all. Accepted values are 'none', 'fast',"
" 'safe', and 'full'. The default value is 'none' (no stack traces).",
" 'safe', and 'full'. The default value is '" + resolver_default + "'.",
)
stacktrace_resolver = En.d(t.Optional[str], _derive_stacktrace_resolver)

Expand Down
8 changes: 6 additions & 2 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ The following environment variables for the tracer are supported:
DD_TAGS:
description: |
Set global tags to be attached to every span. Value must be either comma or space separated. e.g. ``key1:value1,key2:value2`` or ``key1:value key2:value2``.

If a tag value is not supplied the value will be an empty string. e.g. ``key1,key2`` or ``key1 key2``.
version_added:
v0.38.0: Comma separated support added
v0.48.0: Space separated support added
Expand Down Expand Up @@ -290,9 +292,11 @@ The following environment variables for the tracer are supported:

DD_TRACE_HEADER_TAGS:
description: |
A map of case-insensitive header keys to tag names. Automatically applies matching header values as tags on root spans.
A map of case-insensitive http headers to tag names. Automatically applies matching header values as tags on request and response spans. For example if
``DD_TRACE_HEADER_TAGS=User-Agent:http.useragent,content-type:http.content_type``. The value of the header will be stored in tags with the name ``http.useragent`` and ``http.content_type``.

For example, ``User-Agent:http.useragent,content-type:http.content_type``.
If a tag name is not supplied the header name will be used. For example if
``DD_TRACE_HEADER_TAGS=User-Agent,content-type``. The value of http header will be stored in tags with the names ``http.<response/request>.headers.user-agent`` and ``http.<response/request>.headers.content-type``.

DD_TRACE_API_VERSION:
default: |
Expand Down
6 changes: 6 additions & 0 deletions releasenotes/notes/list_dd_header_tags-f8a9ce1daa2a1cf6.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
fixes:
- |
tracing: Updates ``DD_HEADER_TAGS`` and ``DD_TAGS`` to support the following formats:
``key1,key2,key3``, ``key1:val,key2:val,key3:val3``, ``key1:val key2:val key3:val3``, and ``key1 key2 key3``.
Key value pairs that do not match an expected format will be logged and ignored by the tracer.
6 changes: 6 additions & 0 deletions src/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ pyo3-build-config = "0.21.2"
name = "_core"
path = "lib.rs"
crate-type = ["cdylib"]


[net]
# Use git binary from the system instead of the built-in git client
# "Setting this to true can be helpful if you have special authentication requirements that Cargo does not support."
git-fetch-with-cli = true
30 changes: 30 additions & 0 deletions tests/internal/crashtracker/test_crashtracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,9 @@ def test_crashtracker_preload_default(ddtrace_run_python_code_in_subprocess):
conn = utils.listen_get_conn(sock)
assert conn
data = utils.conn_to_bytes(conn)
conn.close()
assert data
assert b"string_at" in data


@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
Expand Down Expand Up @@ -359,7 +361,35 @@ def test_crashtracker_auto_default(run_python_code_in_subprocess):
conn = utils.listen_get_conn(sock)
assert conn
data = utils.conn_to_bytes(conn)
conn.close()
assert data
assert b"string_at" in data


@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
def test_crashtracker_auto_nostack(run_python_code_in_subprocess):
# Setup the listening socket before we open ddtrace
port, sock = utils.crashtracker_receiver_bind()
assert sock

# Call the program
env = os.environ.copy()
env["DD_TRACE_AGENT_URL"] = "http://localhost:%d" % port
env["DD_CRASHTRACKER_STACKTRACE_RESOLVER"] = "none"
stdout, stderr, exitcode, _ = run_python_code_in_subprocess(auto_code, env=env)

# Check for expected exit condition
assert not stdout
assert not stderr
assert exitcode == -11

# Wait for the connection
conn = utils.listen_get_conn(sock)
assert conn
data = utils.conn_to_bytes(conn)
conn.close()
assert data
assert b"string_at" not in data


@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
Expand Down
11 changes: 11 additions & 0 deletions tests/internal/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ def _deleted_rc_config():
"expected": {"trace_http_header_tags": {"header": "value"}},
"expected_source": {"trace_http_header_tags": "code"},
},
{
"env": {"DD_TRACE_HEADER_TAGS": "X-Header-Tag-1,X-Header-Tag-2,X-Header-Tag-3:specific_tag3"},
"expected": {
"trace_http_header_tags": {
"X-Header-Tag-1": "",
"X-Header-Tag-2": "",
"X-Header-Tag-3": "specific_tag3",
}
},
"expected_source": {"trace_http_header_tags": "env_var"},
},
{
"env": {"DD_TRACE_HEADER_TAGS": "X-Header-Tag-1:header_tag_1,X-Header-Tag-2:header_tag_2"},
"rc": {
Expand Down
4 changes: 2 additions & 2 deletions tests/telemetry/test_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def test_app_started_event(telemetry_writer, test_agent_session, mock_time):
{"name": "crashtracking_available", "origin": "unknown", "value": sys.platform == "linux"},
{"name": "crashtracking_debug_url", "origin": "unknown", "value": "None"},
{"name": "crashtracking_enabled", "origin": "unknown", "value": sys.platform == "linux"},
{"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "None"},
{"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "full"},
{"name": "crashtracking_started", "origin": "unknown", "value": False},
{"name": "crashtracking_stderr_filename", "origin": "unknown", "value": "None"},
{"name": "crashtracking_stdout_filename", "origin": "unknown", "value": "None"},
Expand Down Expand Up @@ -323,7 +323,7 @@ def test_app_started_event_configuration_override(
{"name": "crashtracking_available", "origin": "unknown", "value": sys.platform == "linux"},
{"name": "crashtracking_debug_url", "origin": "unknown", "value": "None"},
{"name": "crashtracking_enabled", "origin": "unknown", "value": sys.platform == "linux"},
{"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "None"},
{"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "full"},
{"name": "crashtracking_started", "origin": "unknown", "value": sys.platform == "linux"},
{"name": "crashtracking_stderr_filename", "origin": "unknown", "value": "None"},
{"name": "crashtracking_stdout_filename", "origin": "unknown", "value": "None"},
Expand Down
8 changes: 8 additions & 0 deletions tests/tracer/test_span.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,9 @@ def test_span_pprint():
root = Span("test.span", service="s", resource="r", span_type=SpanTypes.WEB)
root.set_tag("t", "v")
root.set_metric("m", 1.0)
root._add_event("message", {"importance": 10}, 16789898242)
root.set_link(trace_id=99, span_id=10, attributes={"link.name": "s1_to_s2", "link.kind": "scheduled_by"})

root.finish()
actual = root._pprint()
assert "name='test.span'" in actual
Expand All @@ -704,6 +707,11 @@ def test_span_pprint():
assert "error=0" in actual
assert "tags={'t': 'v'}" in actual
assert "metrics={'m': 1.0}" in actual
assert "events='name=message time=16789898242 attributes=importance:10'" in actual
assert (
"links='trace_id=99 span_id=10 attributes=link.name:s1_to_s2,link.kind:scheduled_by "
"tracestate=None flags=None dropped_attributes=0'" in actual
)
assert re.search("id=[0-9]+", actual) is not None
assert re.search("trace_id=[0-9]+", actual) is not None
assert "parent_id=None" in actual
Expand Down
41 changes: 41 additions & 0 deletions tests/tracer/test_trace_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,47 @@ def test_ext_service(int_config, pin, config_val, default, expected):
assert trace_utils.ext_service(pin, int_config.myint, default) == expected


@pytest.mark.subprocess(
parametrize={
"DD_TRACE_HEADER_TAGS": ["header1 header2 header3:third-header", "header1,header2,header3:third-header"]
}
)
def test_set_http_meta_with_http_header_tags_config():
from ddtrace import config
from ddtrace._trace.span import Span
from ddtrace.contrib.trace_utils import set_http_meta

assert config.trace_http_header_tags == {
"header1": "",
"header2": "",
"header3": "third-header",
}, config.trace_http_header_tags
integration_config = config.new_integration
assert integration_config.is_header_tracing_configured

# test request headers
request_span = Span(name="new_integration.request")
set_http_meta(
request_span,
integration_config,
request_headers={"header1": "value1", "header2": "value2", "header3": "value3"},
)
assert request_span.get_tag("http.request.headers.header1") == "value1"
assert request_span.get_tag("http.request.headers.header2") == "value2"
assert request_span.get_tag("third-header") == "value3"

# test response headers
response_span = Span(name="new_integration.response")
set_http_meta(
response_span,
integration_config,
response_headers={"header1": "value1", "header2": "value2", "header3": "value3"},
)
assert response_span.get_tag("http.response.headers.header1") == "value1"
assert response_span.get_tag("http.response.headers.header2") == "value2"
assert response_span.get_tag("third-header") == "value3"


@pytest.mark.parametrize("appsec_enabled", [False, True])
@pytest.mark.parametrize("span_type", [SpanTypes.WEB, SpanTypes.HTTP, None])
@pytest.mark.parametrize(
Expand Down
4 changes: 2 additions & 2 deletions tests/tracer/test_tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,11 +979,11 @@ def test_dd_tags(self):
assert self.tracer._tags.get("key1") == "value1"
assert self.tracer._tags.get("key2") == "value2"

@run_in_subprocess(env_overrides=dict(DD_TAGS="key1:value1,key2:value2,key3"))
@run_in_subprocess(env_overrides=dict(DD_TAGS="key1:value1,key2:value2, key3"))
def test_dd_tags_invalid(self):
assert self.tracer._tags.get("key1")
assert self.tracer._tags.get("key2")
assert self.tracer._tags.get("key3") is None
assert not self.tracer._tags.get("key3")

@run_in_subprocess(env_overrides=dict(DD_TAGS="service:mysvc,env:myenv,version:myvers"))
def test_tags_from_DD_TAGS(self):
Expand Down
33 changes: 23 additions & 10 deletions tests/tracer/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,33 +56,46 @@ def test_asbool(self):
("key:val,key2:val2,key3:1234.23", dict(key="val", key2="val2", key3="1234.23"), None),
("key:val key2:val2 key3:1234.23", dict(key="val", key2="val2", key3="1234.23"), None),
("key: val", dict(key=" val"), None),
("key key: val", {"key key": " val"}, None),
(
"key key: val",
{"key": "", "val": ""},
[mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key key: val")],
),
("key: val,key2:val2", dict(key=" val", key2="val2"), None),
(" key: val,key2:val2", {"key": " val", "key2": "val2"}, None),
("key key2:val1", {"key key2": "val1"}, None),
("key key2:val1", {"key": "", "key2": "val1"}, None),
("key:val key2:val:2", {"key": "val", "key2": "val:2"}, None),
(
"key:val,key2:val2 key3:1234.23",
dict(),
[mock.call(_LOG_ERROR_FAIL_SEPARATOR, "key:val,key2:val2 key3:1234.23")],
),
("key:val key2:val2 key3: ", dict(key="val", key2="val2", key3=""), None),
(
"key:val key2:val2 key3: ",
{"key": "val", "key2": "val2"},
[mock.call(_LOG_ERROR_MALFORMED_TAG, "key3:", "key:val key2:val2 key3:")],
),
(
"key:val key2:val 2",
dict(key="val", key2="val"),
[mock.call(_LOG_ERROR_MALFORMED_TAG, "2", "key:val key2:val 2")],
{"2": "", "key": "val", "key2": "val"},
None,
),
(
"key: val key2:val2 key3:val3",
{"key": "", "key2": "val2", "key3": "val3"},
[mock.call(_LOG_ERROR_MALFORMED_TAG, "val", "key: val key2:val2 key3:val3")],
{"key2": "val2", "key3": "val3", "val": ""},
[mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key: val key2:val2 key3:val3")],
),
(
"key:,key3:val1,",
{"key3": "val1"},
[mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key:,key3:val1")],
),
("key:,key3:val1,", dict(key3="val1", key=""), None),
(",", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, "")]),
(":,:", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, ":,:")]),
("key,key2:val1", {"key2": "val1"}, [mock.call(_LOG_ERROR_MALFORMED_TAG, "key", "key,key2:val1")]),
("key,key2:val1", {"key": "", "key2": "val1"}, None),
("key2:val1:", {"key2": "val1:"}, None),
("key,key2,key3", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, "key,key2,key3")]),
("key,key2,key3", {"key": "", "key2": "", "key3": ""}, None),
("key key2 key3", {"key": "", "key2": "", "key3": ""}, None),
("foo:bar,foo:baz", dict(foo="baz"), None),
("hash:asd url:https://github.com/foo/bar", dict(hash="asd", url="https://github.com/foo/bar"), None),
],
Expand Down

0 comments on commit 3486631

Please sign in to comment.