Skip to content

chore: coerce bool tags to str#17543

Open
brettlangdon wants to merge 3 commits intomainfrom
APMLP-941/set_tag.bool.calls
Open

chore: coerce bool tags to str#17543
brettlangdon wants to merge 3 commits intomainfrom
APMLP-941/set_tag.bool.calls

Conversation

@brettlangdon
Copy link
Copy Markdown
Member

@brettlangdon brettlangdon commented Apr 15, 2026

Description

The typing for Span.set_tag requires the value to be Optional[str]. There are some callsites which depend on the implicit behavior of str() being called on the value. This behavior may and will change at any time since the method typing is requiring a str directly.

This PR updates call sites to explicitly cast boolean tags to a string to conform to the required typing of Span.set_tag.

There should be no functional change in behavior.

Testing

Risks

Additional Notes

These call sites passing bool exist because these files/methods are not being type checked, otherwise static type checking would catch them.

@brettlangdon brettlangdon added the changelog/no-changelog A changelog entry is not required for this PR. label Apr 15, 2026
@brettlangdon brettlangdon marked this pull request as ready for review April 15, 2026 15:10
@brettlangdon brettlangdon requested review from a team as code owners April 15, 2026 15:10
Comment thread ddtrace/contrib/internal/trace_utils.py
Comment thread ddtrace/contrib/internal/trace_utils.py
@cit-pr-commenter-54b7da
Copy link
Copy Markdown

Codeowners resolved as

ddtrace/contrib/internal/trace_utils.py                                 @DataDog/apm-core-python @DataDog/apm-idm-python

@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Apr 15, 2026

Performance SLOs

Comparing candidate APMLP-941/set_tag.bool.calls (ee761e9) with baseline main (a5ff574)

🟡 Near SLO Breach (1 suite)
🟡 otelspan - 22/22

✅ add-event

Time: ✅ 40.633ms (SLO: <47.150ms 📉 -13.8%) vs baseline: +0.2%

Memory: ✅ 41.232MB (SLO: <47.000MB 📉 -12.3%) vs baseline: +5.0%


✅ add-metrics

Time: ✅ 234.303ms (SLO: <344.800ms 📉 -32.0%) vs baseline: -0.5%

Memory: ✅ 45.772MB (SLO: <47.500MB -3.6%) vs baseline: +4.8%


✅ add-tags

Time: ✅ 264.902ms (SLO: <330.000ms 📉 -19.7%) vs baseline: +0.2%

Memory: ✅ 45.707MB (SLO: <47.500MB -3.8%) vs baseline: +5.0%


✅ get-context

Time: ✅ 83.728ms (SLO: <92.350ms -9.3%) vs baseline: ~same

Memory: ✅ 41.583MB (SLO: <46.500MB 📉 -10.6%) vs baseline: +4.6%


✅ is-recording

Time: ✅ 39.087ms (SLO: <44.500ms 📉 -12.2%) vs baseline: +0.1%

Memory: ✅ 41.215MB (SLO: <47.500MB 📉 -13.2%) vs baseline: +4.9%


✅ record-exception

Time: ✅ 61.124ms (SLO: <67.650ms -9.6%) vs baseline: -0.1%

Memory: ✅ 41.830MB (SLO: <47.000MB 📉 -11.0%) vs baseline: +5.1%


✅ set-status

Time: ✅ 44.928ms (SLO: <50.400ms 📉 -10.9%) vs baseline: ~same

Memory: ✅ 41.132MB (SLO: <47.000MB 📉 -12.5%) vs baseline: +5.0%


✅ start

Time: ✅ 40.109ms (SLO: <44.500ms -9.9%) vs baseline: +4.4%

Memory: ✅ 41.103MB (SLO: <47.000MB 📉 -12.5%) vs baseline: +4.6%


✅ start-finish

Time: ✅ 90.016ms (SLO: <92.000ms -2.2%) vs baseline: -0.5%

Memory: ✅ 38.653MB (SLO: <46.500MB 📉 -16.9%) vs baseline: +4.7%


✅ start-finish-telemetry

Time: ✅ 91.505ms (SLO: <93.000ms 🟡 -1.6%) vs baseline: -0.7%

Memory: ✅ 38.771MB (SLO: <46.500MB 📉 -16.6%) vs baseline: +5.0%


✅ update-name

Time: ✅ 40.224ms (SLO: <45.150ms 📉 -10.9%) vs baseline: +0.1%

Memory: ✅ 41.280MB (SLO: <47.000MB 📉 -12.2%) vs baseline: +4.8%

✅ All Tests Passing (1 suite)
otelsdkspan - 24/24

✅ add-event

Time: ✅ 40.489ms (SLO: <42.000ms -3.6%) vs baseline: +0.2%

Memory: ✅ 39.066MB (SLO: <40.750MB -4.1%) vs baseline: +5.0%


✅ add-link

Time: ✅ 36.438ms (SLO: <38.550ms -5.5%) vs baseline: ~same

Memory: ✅ 39.046MB (SLO: <40.750MB -4.2%) vs baseline: +4.9%


✅ add-metrics

Time: ✅ 219.370ms (SLO: <232.000ms -5.4%) vs baseline: -0.5%

Memory: ✅ 38.948MB (SLO: <40.750MB -4.4%) vs baseline: +4.6%


✅ add-tags

Time: ✅ 211.668ms (SLO: <221.600ms -4.5%) vs baseline: +0.2%

Memory: ✅ 39.086MB (SLO: <40.750MB -4.1%) vs baseline: +5.0%


✅ get-context

Time: ✅ 29.201ms (SLO: <31.300ms -6.7%) vs baseline: +0.4%

Memory: ✅ 39.105MB (SLO: <40.750MB -4.0%) vs baseline: +5.1%


✅ is-recording

Time: ✅ 29.084ms (SLO: <31.000ms -6.2%) vs baseline: ~same

Memory: ✅ 39.066MB (SLO: <40.750MB -4.1%) vs baseline: +4.9%


✅ record-exception

Time: ✅ 63.104ms (SLO: <65.850ms -4.2%) vs baseline: -0.4%

Memory: ✅ 39.027MB (SLO: <40.750MB -4.2%) vs baseline: +4.9%


✅ set-status

Time: ✅ 31.938ms (SLO: <34.150ms -6.5%) vs baseline: -0.5%

Memory: ✅ 39.066MB (SLO: <40.750MB -4.1%) vs baseline: +5.0%


✅ start

Time: ✅ 29.227ms (SLO: <30.150ms -3.1%) vs baseline: +1.3%

Memory: ✅ 38.987MB (SLO: <40.750MB -4.3%) vs baseline: +4.8%


✅ start-finish

Time: ✅ 33.822ms (SLO: <35.350ms -4.3%) vs baseline: -0.3%

Memory: ✅ 39.066MB (SLO: <40.750MB -4.1%) vs baseline: +5.1%


✅ start-finish-telemetry

Time: ✅ 34.030ms (SLO: <35.450ms -4.0%) vs baseline: +0.3%

Memory: ✅ 39.007MB (SLO: <40.750MB -4.3%) vs baseline: +4.9%


✅ update-name

Time: ✅ 31.021ms (SLO: <33.400ms -7.1%) vs baseline: -0.1%

Memory: ✅ 39.066MB (SLO: <40.750MB -4.1%) vs baseline: +4.5%

Comment thread ddtrace/contrib/internal/trace_utils.py Outdated
@datadog-datadog-prod-us1
Copy link
Copy Markdown
Contributor

datadog-datadog-prod-us1 bot commented Apr 15, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: ee761e9 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@brettlangdon
Copy link
Copy Markdown
Member Author

/merge

@gh-worker-devflow-routing-ef8351
Copy link
Copy Markdown

gh-worker-devflow-routing-ef8351 bot commented Apr 16, 2026

View all feedbacks in Devflow UI.

2026-04-16 15:04:44 UTC ℹ️ Start processing command /merge


2026-04-16 15:04:50 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 60m (p90).


2026-04-16 17:05:12 UTC 🚨 MergeQueue: This merge request is in error because of DDCI

DDCI pipeline didn't start (sourcing_failed)... Please retry.

DDCI Change Request: 7172755084346133367
You can get help from #ci-infra-support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants