Skip to content

Commit

Permalink
lint
Browse files Browse the repository at this point in the history
  • Loading branch information
rachelyangdog committed Aug 27, 2024
1 parent 8384630 commit ce0e420
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 91 deletions.
3 changes: 2 additions & 1 deletion ddtrace/internal/opentelemetry/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def attach(self, otel_context):
otel_baggage = get_baggage(otel_context)
if ddcontext and otel_baggage:
for key, value in otel_baggage.items():
# value is from opentelemetry and can be any object. make sure this object is compatiable w/ datadog internals
# value is from opentelemetry and can be any object.
# make sure this object is compatiable w/ datadog internals
ddcontext._baggage[key] = value # potentially convert to json

# A return value with the type `object` is required by the otel api to remove/deactivate spans.
Expand Down
74 changes: 33 additions & 41 deletions ddtrace/propagation/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from typing import Tuple # noqa:F401
from typing import cast # noqa:F401
import urllib.parse

import ddtrace
from ddtrace._trace.span import Span # noqa:F401

Expand Down Expand Up @@ -885,37 +886,29 @@ def _inject(span_context, headers):
# type: (Context , Dict[str, str]) -> Dict[str, str]
return headers

class _BaggageHeader:

class _BaggageHeader:
@staticmethod
def _encode_key(key):
safe_characters = (
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
"0123456789"
"!#$%&'*+-.^_`|~"
)
safe_characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" "0123456789" "!#$%&'*+-.^_`|~"
encoded = urllib.parse.quote(key, safe=safe_characters)
return encoded

def _encode_value(value):
safe_characters = (
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
"0123456789"
"!#$%&'()*+-./:<>?@[]^_`{|}~"
)
"ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" "0123456789" "!#$%&'()*+-./:<>?@[]^_`{|}~"
)
encoded = urllib.parse.quote(value, safe=safe_characters)
return encoded

@staticmethod
def _inject(span_context, headers):
baggage_items = span_context._baggage.items()
baggage_items = span_context._baggage.items()
if not baggage_items:
return

header_value = ",".join(
f"{_BaggageHeader._encode_key(str(key).strip())}={_BaggageHeader._encode_value(str(value).strip())}"
f"{_BaggageHeader._encode_key(str(key).strip())}={_BaggageHeader._encode_value(str(value).strip())}"
for key, value in baggage_items
)

Expand All @@ -926,27 +919,25 @@ def _extract(headers):
header_value = headers.get("baggage")
if not header_value:
return None

baggage = {}
baggages = header_value.split(',')
baggages = header_value.split(",")
for key_value in baggages:
key, value = key_value.split('=', 1)
key, value = key_value.split("=", 1)
key = urllib.parse.unquote(key.strip())
value = urllib.parse.unquote(value.strip())
baggage[key] = value

return Context(
baggage=baggage
)

return Context(baggage=baggage)


_PROP_STYLES = {
PROPAGATION_STYLE_DATADOG: _DatadogMultiHeader,
PROPAGATION_STYLE_B3_MULTI: _B3MultiHeader,
PROPAGATION_STYLE_B3_SINGLE: _B3SingleHeader,
_PROPAGATION_STYLE_W3C_TRACECONTEXT: _TraceContext,
_PROPAGATION_STYLE_NONE: _NOP_Propagator,
_PROPAGATION_STYLE_BAGGAGE: _BaggageHeader,
PROPAGATION_STYLE_DATADOG: _DatadogMultiHeader,
PROPAGATION_STYLE_B3_MULTI: _B3MultiHeader,
PROPAGATION_STYLE_B3_SINGLE: _B3SingleHeader,
_PROPAGATION_STYLE_W3C_TRACECONTEXT: _TraceContext,
_PROPAGATION_STYLE_NONE: _NOP_Propagator,
_PROPAGATION_STYLE_BAGGAGE: _BaggageHeader,
}


Expand All @@ -972,17 +963,18 @@ def _resolve_contexts(contexts, styles_w_ctx, normalized_headers):
primary_context = contexts[0]
links = []

# special case where baggage is first in propagation styles
# why would you do that? I don't know, but it's possible
if style_w_ctx[0] == _PROPAGATION_STYLE_BAGGAGE:
baggage_context = contexts[0]
contexts.append(baggage_context)
del contexts[0]
styles_w_ctx.append(_PROPAGATION_STYLE_BAGGAGE)
del style_w_ctx[0]

for context in contexts[1:]:
style_w_ctx = styles_w_ctx[contexts.index(context)]

# special case where baggage is first in propagation styles
# why would you do that? I don't know, but it's possible
if style_w_ctx[0] == _PROPAGATION_STYLE_BAGGAGE:
baggage_context = contexts[0]
contexts.append(baggage_context)
del contexts[0]
styles_w_ctx.append(_PROPAGATION_STYLE_BAGGAGE)
del style_w_ctx[0]

# encoding expects at least trace_id and span_id
if context.span_id and context.trace_id and context.trace_id != primary_context.trace_id:
links.append(
Expand Down Expand Up @@ -1018,11 +1010,11 @@ def _resolve_contexts(contexts, styles_w_ctx, normalized_headers):
primary_context._meta[LAST_DD_PARENT_ID_KEY] = "{:016x}".format(dd_context.span_id)
# the span_id in tracecontext takes precedence over the first extracted propagation style
primary_context.span_id = context.span_id

# baggage is always merged into the primary context
if style_w_ctx == _PROPAGATION_STYLE_BAGGAGE:
primary_context._baggage.update(context._baggage)

primary_context._span_links = links
return primary_context

Expand Down Expand Up @@ -1146,4 +1138,4 @@ def my_controller(url, headers):

except Exception:
log.debug("error while extracting context propagation headers", exc_info=True)
return Context()
return Context()
2 changes: 1 addition & 1 deletion ddtrace/settings/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -884,4 +884,4 @@ def convert_rc_trace_sampling_rules(self, rc_rules: List[Dict[str, Any]]) -> Opt
if rc_rules:
return json.dumps(rc_rules)
else:
return None
return None
69 changes: 21 additions & 48 deletions tests/tracer/test_propagation.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
from ddtrace.constants import AUTO_REJECT
from ddtrace.constants import USER_KEEP
from ddtrace.constants import USER_REJECT
from ddtrace.internal.constants import _PROPAGATION_STYLE_BAGGAGE
from ddtrace.internal.constants import _PROPAGATION_STYLE_NONE
from ddtrace.internal.constants import _PROPAGATION_STYLE_W3C_TRACECONTEXT
from ddtrace.internal.constants import LAST_DD_PARENT_ID_KEY
from ddtrace.internal.constants import PROPAGATION_STYLE_B3_MULTI
from ddtrace.internal.constants import PROPAGATION_STYLE_B3_SINGLE
from ddtrace.internal.constants import PROPAGATION_STYLE_DATADOG
from ddtrace.internal.constants import _PROPAGATION_STYLE_BAGGAGE
from ddtrace.propagation._utils import get_wsgi_header
from ddtrace.propagation.http import _HTTP_BAGGAGE_PREFIX
from ddtrace.propagation.http import _HTTP_HEADER_B3_FLAGS
Expand All @@ -44,6 +44,7 @@

from ..utils import override_global_config


NOT_SET = object()


Expand Down Expand Up @@ -2915,7 +2916,7 @@ def test_span_links_set_on_root_span_not_child(fastapi_client, tracer, fastapi_t
_HTTP_HEADER_TRACESTATE: "",
},
),
(
(
"only_baggage",
[
_PROPAGATION_STYLE_BAGGAGE,
Expand All @@ -2925,9 +2926,9 @@ def test_span_links_set_on_root_span_not_child(fastapi_client, tracer, fastapi_t
},
{
"baggage": "foo=bar",
}
},
),
(
(
"baggage_and_datadog",
[
PROPAGATION_STYLE_DATADOG,
Expand All @@ -2942,7 +2943,7 @@ def test_span_links_set_on_root_span_not_child(fastapi_client, tracer, fastapi_t
HTTP_HEADER_TRACE_ID: "13088165645273925489",
HTTP_HEADER_PARENT_ID: "8185124618007618416",
"baggage": "foo=bar",
}
},
),
(
"baggage_order",
Expand Down Expand Up @@ -3096,37 +3097,23 @@ def test_llmobs_parent_id_not_injected_by_default():
@pytest.mark.parametrize(
"span_context,expected_headers",
[
(
Context(baggage={"key1": "val1"}),
{"baggage": "key1=val1"}
),
(
Context(baggage={"key1": "val1", "key2": "val2"}),
{"baggage": "key1=val1,key2=val2"}
),
(
Context(baggage={"serverNode": "DF 28"}),
{"baggage": "serverNode=DF%2028"}
),
(
Context(baggage={"userId": "Amélie"}),
{"baggage": "userId=Am%C3%A9lie"}
),
(
Context(baggage={"user!d(me)": "false"}),
{"baggage": "user!d%28me%29=false"}
),
(Context(baggage={"key1": "val1"}), {"baggage": "key1=val1"}),
(Context(baggage={"key1": "val1", "key2": "val2"}), {"baggage": "key1=val1,key2=val2"}),
(Context(baggage={"serverNode": "DF 28"}), {"baggage": "serverNode=DF%2028"}),
(Context(baggage={"userId": "Amélie"}), {"baggage": "userId=Am%C3%A9lie"}),
(Context(baggage={"user!d(me)": "false"}), {"baggage": "user!d%28me%29=false"}),
],
ids=[
"single_key_value",
"multiple_key_value_pairs",
"space_in_value",
"special_characters_in_value",
"special_characters_in_key",
]
],
)
def test_baggageheader_inject(span_context, expected_headers):
from ddtrace.propagation.http import _BaggageHeader

headers = {}
_BaggageHeader._inject(span_context, headers)
assert headers == expected_headers
Expand All @@ -3135,36 +3122,22 @@ def test_baggageheader_inject(span_context, expected_headers):
@pytest.mark.parametrize(
"headers,expected_baggage",
[
(
{"baggage": "key1=val1"},
{"key1": "val1"}
),
(
{"baggage": "key1=val1,key2=val2,foo=bar,x=y"},
{"key1": "val1", "key2": "val2", "foo": "bar", "x": "y"}
),
(
{"baggage": "user!d%28me%29=false"},
{"user!d(me)": "false"}
),
(
{"baggage": "userId=Am%C3%A9lie"},
{"userId": "Amélie"}
),
(
{"baggage": "serverNode=DF%2028"},
{"serverNode": "DF 28"}
),
({"baggage": "key1=val1"}, {"key1": "val1"}),
({"baggage": "key1=val1,key2=val2,foo=bar,x=y"}, {"key1": "val1", "key2": "val2", "foo": "bar", "x": "y"}),
({"baggage": "user!d%28me%29=false"}, {"user!d(me)": "false"}),
({"baggage": "userId=Am%C3%A9lie"}, {"userId": "Amélie"}),
({"baggage": "serverNode=DF%2028"}, {"serverNode": "DF 28"}),
],
ids=[
"single_key_value",
"multiple_key_value_pairs",
"special_characters_in_key",
"special_characters_in_value",
"space_in_value",
]
],
)
def test_baggageheader_extract(headers, expected_baggage):
from ddtrace.propagation.http import _BaggageHeader

context = _BaggageHeader._extract(headers)
assert context._baggage == expected_baggage
assert context._baggage == expected_baggage

0 comments on commit ce0e420

Please sign in to comment.