WIP: otel baggage support initial PR #10389

rachelyangdog · 2024-08-26T20:28:48Z

First PR introducing baggage support

Checklist

PR author has checked that all the criteria below are met
The PR description includes an overview of the change
The PR description articulates the motivation for the change
The change includes tests OR the PR description describes a testing strategy
The PR description notes risks associated with the change, if any
Newly-added code is easy to change
The change follows the library release note guidelines
The change includes or references documentation updates if necessary
Backport labels are set (if applicable)

Reviewer Checklist

Reviewer has checked that all the criteria below are met
Title is accurate
All changes are related to the pull request's stated goal
Avoids breaking API changes
Testing strategy adequately addresses listed risks
Newly-added code is easy to change
Release note makes sense to a user of the library
If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
Backport labels are set in a manner that is consistent with the release branch maintenance policy

github-actions · 2024-08-26T20:29:17Z

CODEOWNERS have been resolved as:

releasenotes/notes/baggage-support-be7eed26293f1216.yaml                @DataDog/apm-python
ddtrace/_trace/context.py                                               @DataDog/apm-sdk-api-python
ddtrace/_trace/span.py                                                  @DataDog/apm-sdk-api-python
ddtrace/internal/constants.py                                           @DataDog/apm-core-python
ddtrace/internal/opentelemetry/context.py                               @DataDog/apm-sdk-api-python
ddtrace/propagation/http.py                                             @DataDog/apm-sdk-api-python
ddtrace/settings/config.py                                              @DataDog/python-guild @DataDog/apm-sdk-api-python
riotfile.py                                                             @DataDog/apm-python
tests/opentelemetry/test_context.py                                     @DataDog/apm-sdk-api-python
tests/tracer/test_propagation.py                                        @DataDog/apm-sdk-api-python
tests/tracer/test_span.py                                               @DataDog/apm-sdk-api-python

pr-commenter · 2024-08-26T21:43:03Z

Benchmarks

Benchmark execution time: 2024-10-07 20:17:20

Comparing candidate commit 6dc990a in PR branch rachel.yang/baggage with baseline commit 0bf1fe9 in branch main.

Found 0 performance improvements and 20 performance regressions! Performance is the same for 354 metrics, 52 unstable metrics.

scenario:httppropagationextract-b3_headers

🟥 execution_time [+5.648µs; +5.772µs] or [+21.743%; +22.218%]

scenario:httppropagationextract-b3_single_headers

🟥 execution_time [+5.511µs; +5.593µs] or [+24.096%; +24.452%]

scenario:httppropagationextract-full_t_id_datadog_headers

🟥 execution_time [+5.726µs; +5.873µs] or [+12.093%; +12.403%]

scenario:httppropagationextract-invalid_priority_header

🟥 execution_time [+4.795µs; +4.855µs] or [+27.945%; +28.293%]

scenario:httppropagationextract-invalid_span_id_header

🟥 execution_time [+4.921µs; +4.997µs] or [+28.696%; +29.141%]

scenario:httppropagationextract-invalid_tags_header

🟥 execution_time [+4.953µs; +5.000µs] or [+28.915%; +29.194%]

scenario:httppropagationextract-invalid_trace_id_header

🟥 execution_time [+4.863µs; +4.920µs] or [+28.366%; +28.701%]

scenario:httppropagationextract-medium_header_no_matches

🟥 execution_time [+4.780µs; +4.859µs] or [+17.159%; +17.443%]

scenario:httppropagationextract-medium_valid_headers_all

🟥 execution_time [+4.813µs; +4.904µs] or [+15.150%; +15.437%]

scenario:httppropagationextract-tracecontext_headers

🟥 execution_time [+5.640µs; +5.807µs] or [+8.148%; +8.388%]

scenario:httppropagationextract-valid_headers_all

🟥 execution_time [+4.891µs; +4.985µs] or [+28.470%; +29.018%]

scenario:httppropagationextract-valid_headers_basic

🟥 execution_time [+4.960µs; +5.015µs] or [+31.584%; +31.930%]

scenario:httppropagationextract-wsgi_invalid_priority_header

🟥 execution_time [+4.911µs; +4.965µs] or [+28.637%; +28.955%]

scenario:httppropagationextract-wsgi_invalid_tags_header

🟥 execution_time [+4.832µs; +4.938µs] or [+28.053%; +28.665%]

scenario:httppropagationextract-wsgi_invalid_trace_id_header

🟥 execution_time [+4.864µs; +4.931µs] or [+28.384%; +28.773%]

scenario:httppropagationextract-wsgi_medium_header_no_matches

🟥 execution_time [+4.855µs; +4.959µs] or [+17.340%; +17.711%]

scenario:httppropagationextract-wsgi_medium_valid_headers_all

🟥 execution_time [+4.823µs; +4.927µs] or [+15.118%; +15.445%]

scenario:httppropagationextract-wsgi_valid_headers_all

🟥 execution_time [+4.879µs; +4.965µs] or [+28.407%; +28.907%]

scenario:httppropagationextract-wsgi_valid_headers_basic

🟥 execution_time [+4.892µs; +4.949µs] or [+31.056%; +31.418%]

scenario:span-start

🟥 max_rss_usage [+3.968MB; +4.090MB] or [+10.978%; +11.316%]

datadog-dd-trace-py-rkomorn · 2024-08-27T14:50:34Z

Datadog Report

Branch report: rachel.yang/baggage
Commit report: cfec962
Test service: dd-trace-py

❌ 426 Failed (0 Known Flaky), 163772 Passed, 10479 Skipped, 8h 40m 55.13s Total duration (7h 4m 44.34s time saved)
❄️ 47 New Flaky
⌛ 3 Performance Regressions

❌ Failed Tests (426)

This report shows up to 5 failed tests.

test_distributed_tracing[scope0] - test_asgi.py - Details

Expand for error

 assert None == 1234
  +  where None = <Span(id=17947516139693682439,trace_id=136898361102384514869965136187096557328,parent_id=None,name=asgi.request)>.parent_id

test_distributed_tracing[scope0] - test_asgi.py - Details

Expand for error

 assert None == 1234
  +  where None = <Span(id=7169614202445284496,trace_id=136898361736209814984239894516612396808,parent_id=None,name=asgi.request)>.parent_id

test_distributed_tracing[scope1] - test_asgi.py - Details

Expand for error

 assert None == 1234
  +  where None = <Span(id=8388542101613764570,trace_id=136898361656981652483199378188304412895,parent_id=None,name=asgi.request)>.parent_id

test_distributed_tracing[scope1] - test_asgi.py - Details

Expand for error

 assert None == 1234
  +  where None = <Span(id=214240755496349370,trace_id=136898361023156352351399415865192883834,parent_id=None,name=asgi.request)>.parent_id

test_distributed_tracing[scope2] - test_asgi.py - Details

Expand for error

 assert None == 1234
  +  where None = <Span(id=920059719593173542,trace_id=136898361023156352364666405393914162644,parent_id=None,name=asgi.request)>.parent_id

New Flaky Tests (47)

test_django_login_events_disabled_explicitly - test_django_appsec.py - Last Failure

Expand for error

 connection failed: could not connect to server: Connection refused
 	Is the server running on host "127.0.0.1" and accepting
 	TCP/IP connections on port 5432?

test_django_login_events_disabled_noappsec - test_django_appsec.py - Last Failure

Expand for error

 connection failed: could not connect to server: Connection refused
 	Is the server running on host "127.0.0.1" and accepting
 	TCP/IP connections on port 5432?

test_django_login_failure_anonymization_but_user_set_login - test_django_appsec.py - Last Failure

Expand for error

 connection failed: could not connect to server: Connection refused
 	Is the server running on host "127.0.0.1" and accepting
 	TCP/IP connections on port 5432?

test_django_login_failure_anonymization_user_does_exist - test_django_appsec.py - Last Failure

Expand for error

 connection failed: could not connect to server: Connection refused
 	Is the server running on host "127.0.0.1" and accepting
 	TCP/IP connections on port 5432?

test_django_login_failure_identification_user_does_exist - test_django_appsec.py - Last Failure

Expand for error

 connection failed: could not connect to server: Connection refused
 	Is the server running on host "127.0.0.1" and accepting
 	TCP/IP connections on port 5432?

⌛ Performance Regressions vs Default Branch (3)

test_distributed_trace_with_flask_app[with_ddtrace_run] - test_trace.py 10.52s (+9.39s, +830%) - Details
test_distributed_trace_with_flask_app[with_ddtrace_run] - test_trace.py 10.52s (+9.51s, +945%) - Details
test_distributed_trace_with_flask_app[with_ddtrace_run] - test_trace.py 10.33s (+9.29s, +892%) - Details

ddtrace/internal/constants.py

ddtrace/internal/opentelemetry/context.py

ddtrace/propagation/http.py

mabdinur · 2024-08-27T16:29:02Z

ddtrace/propagation/http.py

+            return
+
+        header_value = ",".join(
+            f"{_BaggageHeader._encode_key(str(key).strip())}={_BaggageHeader._encode_value(str(value).strip())}"


Should we raise an unhandled exception if value can not be converted to a string? How should we handle dicts,arrays,booleans,etc. The format needs to be consistent across languages (ex: encoding True vs true)

When "extracting baggage from propagation headers, they may encounter malformed header contents." When this occurs, we "should ignore the entire header." (RFC) So instead of an error, we could potentially just ignore it? As of now, I think it would just do something like a type error

Should we raise an unhandled exception if value can not be converted to a string?

I don't know who errors are handles in Python, but an invalid header should not break the service we are instrumenting. We could definitely log a warning or whatever you usually do in cases like this.

"should ignore the entire header." (RFC)

The main point here was that we should not try to extract individual values while ignore that bad ones. If something is wrong, don't try to extract anything. I clear this up in the RFC, thanks.

ddtrace/propagation/http.py

ddtrace/_trace/span.py

ddtrace/propagation/http.py

ddtrace/_trace/context.py

WIP: otel baggage support initial

d7e6d78

rachelyangdog requested review from a team as code owners August 26, 2024 20:28

rachelyangdog requested review from zacharycmontoya, juanjux and emmettbutler August 26, 2024 20:28

rachelyangdog marked this pull request as draft August 26, 2024 20:28

adding otel context

8384630

rachelyangdog added 2 commits August 27, 2024 10:03

lint

ce0e420

spelling errors

36d68d3

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/internal/constants.py Show resolved Hide resolved

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/internal/opentelemetry/context.py Outdated Show resolved Hide resolved

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/internal/opentelemetry/context.py Outdated Show resolved Hide resolved

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/propagation/http.py Show resolved Hide resolved

mabdinur reviewed Aug 27, 2024

View reviewed changes

ddtrace/propagation/http.py Show resolved Hide resolved

safe_characters

8adbfd7

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/_trace/span.py Outdated Show resolved Hide resolved

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/propagation/http.py Show resolved Hide resolved

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

mabdinur reviewed Sep 10, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

lucaspimentel self-requested a review September 11, 2024 15:26

fixing small things

e09c681

rachelyangdog added 7 commits September 30, 2024 14:18

otel tests

956382e

changing baggage extract in propagation

c61d090

typo

fa396c0

adding malformed headers tests

6e1405f

lint

62d234a

cleaning

72a14e7

typo

06a7b69

ZStriker19 reviewed Oct 1, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

ZStriker19 reviewed Oct 1, 2024

View reviewed changes

ddtrace/propagation/http.py Show resolved Hide resolved

ZStriker19 reviewed Oct 1, 2024

View reviewed changes

ddtrace/propagation/http.py Outdated Show resolved Hide resolved

rachelyangdog added 4 commits October 1, 2024 13:24

fix baggage extract

3047d07

otel baggage hold

6a7b921

typing error

7203179

changing baggage api to be public

cfec962

datadog-datadog-prod-us1 bot reviewed Oct 2, 2024

View reviewed changes

ddtrace/_trace/context.py Show resolved Hide resolved

rachelyangdog added 15 commits October 2, 2024 16:24

Merge branch 'main' into rachel.yang/baggage

20418bd

lint

9b53ff6

circle ci is giving weird errors

555153b

still weird linting errors

8b55ef0

Merge branch 'main' into rachel.yang/baggage

cd5e67d

trace_id errors

eb1405c

trace_id errors

21f6223

still errors

27a21aa

errors

a00e974

typing

32671ce

Merge branch 'main' into rachel.yang/baggage

0294a7a

http extract

2afbfa3

added if statement

2a378d3

release notes

39a6dd0

Merge branch 'main' into rachel.yang/baggage

6dc990a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: otel baggage support initial PR #10389

WIP: otel baggage support initial PR #10389

rachelyangdog commented Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 26, 2024 •

edited

Loading

pr-commenter bot commented Aug 26, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn bot commented Aug 27, 2024 •

edited

Loading

mabdinur Aug 27, 2024

rachelyangdog Aug 27, 2024 •

edited

Loading

lucaspimentel Sep 11, 2024 •

edited

Loading

WIP: otel baggage support initial PR #10389

Are you sure you want to change the base?

WIP: otel baggage support initial PR #10389

Conversation

rachelyangdog commented Aug 26, 2024 • edited Loading

Checklist

Reviewer Checklist

github-actions bot commented Aug 26, 2024 • edited Loading

pr-commenter bot commented Aug 26, 2024 • edited Loading

Benchmarks

scenario:httppropagationextract-b3_headers

scenario:httppropagationextract-b3_single_headers

scenario:httppropagationextract-full_t_id_datadog_headers

scenario:httppropagationextract-invalid_priority_header

scenario:httppropagationextract-invalid_span_id_header

scenario:httppropagationextract-invalid_tags_header

scenario:httppropagationextract-invalid_trace_id_header

scenario:httppropagationextract-medium_header_no_matches

scenario:httppropagationextract-medium_valid_headers_all

scenario:httppropagationextract-tracecontext_headers

scenario:httppropagationextract-valid_headers_all

scenario:httppropagationextract-valid_headers_basic

scenario:httppropagationextract-wsgi_invalid_priority_header

scenario:httppropagationextract-wsgi_invalid_tags_header

scenario:httppropagationextract-wsgi_invalid_trace_id_header

scenario:httppropagationextract-wsgi_medium_header_no_matches

scenario:httppropagationextract-wsgi_medium_valid_headers_all

scenario:httppropagationextract-wsgi_valid_headers_all

scenario:httppropagationextract-wsgi_valid_headers_basic

scenario:span-start

datadog-dd-trace-py-rkomorn bot commented Aug 27, 2024 • edited Loading

Datadog Report

❌ Failed Tests (426)

New Flaky Tests (47)

⌛ Performance Regressions vs Default Branch (3)

mabdinur Aug 27, 2024

Choose a reason for hiding this comment

rachelyangdog Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

lucaspimentel Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

rachelyangdog commented Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 26, 2024 •

edited

Loading

pr-commenter bot commented Aug 26, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn bot commented Aug 27, 2024 •

edited

Loading

rachelyangdog Aug 27, 2024 •

edited

Loading

lucaspimentel Sep 11, 2024 •

edited

Loading