-
Notifications
You must be signed in to change notification settings - Fork 457
feat(tracing): resource renaming #14580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(tracing): resource renaming #14580
Conversation
0768898
to
a1a885d
Compare
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 275 ± 5 ms. The average import time from base is: 273 ± 2 ms. The import time difference between this PR and base is: 1.8 ± 0.2 ms. Import time breakdownThe following import paths have appeared:
|
b0126e8
to
cdfa633
Compare
Performance SLOsComparing candidate florentin.labelle/APPSEC-58892/resource-renaming (dc489a5) with baseline main (7042e95) 🟡 Near SLO Breach (5 suites)🟡 djangosimple - 26/26✅ appsecTime: ✅ 20.476ms (SLO: <22.300ms -8.2%) vs baseline: -0.3% Memory: ✅ 64.464MB (SLO: <66.000MB -2.3%) vs baseline: +4.9% ✅ exception-replay-enabledTime: ✅ 1.350ms (SLO: <1.450ms -6.9%) vs baseline: +0.1% Memory: ✅ 63.696MB (SLO: <66.000MB -3.5%) vs baseline: +5.2% ✅ iastTime: ✅ 20.486ms (SLO: <22.250ms -7.9%) vs baseline: ~same Memory: ✅ 64.487MB (SLO: <66.000MB -2.3%) vs baseline: +5.0% ✅ profilerTime: ✅ 15.311ms (SLO: <16.550ms -7.5%) vs baseline: +0.7% Memory: ✅ 52.999MB (SLO: <53.500MB 🟡 -0.9%) vs baseline: +5.0% ✅ span-code-originTime: ✅ 26.110ms (SLO: <28.200ms -7.4%) vs baseline: ~same Memory: ✅ 66.854MB (SLO: <68.500MB -2.4%) vs baseline: +5.2% ✅ tracerTime: ✅ 20.537ms (SLO: <21.750ms -5.6%) vs baseline: +0.2% Memory: ✅ 64.487MB (SLO: <66.000MB -2.3%) vs baseline: +5.0% ✅ tracer-and-profilerTime: ✅ 22.131ms (SLO: <23.500ms -5.8%) vs baseline: +0.3% Memory: ✅ 65.857MB (SLO: <67.000MB 🟡 -1.7%) vs baseline: +5.0% ✅ tracer-dont-create-db-spansTime: ✅ 19.289ms (SLO: <21.500ms 📉 -10.3%) vs baseline: -0.3% Memory: ✅ 64.527MB (SLO: <66.000MB -2.2%) vs baseline: +5.1% ✅ tracer-nativeTime: ✅ 20.503ms (SLO: <21.750ms -5.7%) vs baseline: ~same Memory: ✅ 65.726MB (SLO: <66.000MB 🟡 -0.4%) vs baseline: +5.1% ✅ tracer-no-cachesTime: ✅ 18.462ms (SLO: <19.650ms -6.0%) vs baseline: ~same Memory: ✅ 64.468MB (SLO: <66.000MB -2.3%) vs baseline: +4.8% ✅ tracer-no-databasesTime: ✅ 18.805ms (SLO: <20.100ms -6.4%) vs baseline: +0.4% Memory: ✅ 64.507MB (SLO: <66.000MB -2.3%) vs baseline: +5.0% ✅ tracer-no-middlewareTime: ✅ 20.240ms (SLO: <21.500ms -5.9%) vs baseline: +0.3% Memory: ✅ 64.487MB (SLO: <66.000MB -2.3%) vs baseline: +4.9% ✅ tracer-no-templatesTime: ✅ 20.325ms (SLO: <22.000ms -7.6%) vs baseline: ~same Memory: ✅ 64.507MB (SLO: <66.000MB -2.3%) vs baseline: +5.0% 🟡 errortrackingdjangosimple - 6/6✅ errortracking-enabled-allTime: ✅ 18.052ms (SLO: <19.850ms -9.1%) vs baseline: +0.1% Memory: ✅ 64.468MB (SLO: <65.500MB 🟡 -1.6%) vs baseline: +5.0% ✅ errortracking-enabled-userTime: ✅ 18.125ms (SLO: <19.400ms -6.6%) vs baseline: +0.3% Memory: ✅ 64.507MB (SLO: <65.500MB 🟡 -1.5%) vs baseline: +5.0% ✅ tracer-enabledTime: ✅ 18.107ms (SLO: <19.450ms -6.9%) vs baseline: +0.4% Memory: ✅ 64.444MB (SLO: <65.500MB 🟡 -1.6%) vs baseline: +5.0% 🟡 flasksimple - 17/17✅ appsec-getTime: ✅ 4.588ms (SLO: <4.750ms -3.4%) vs baseline: +0.2% Memory: ✅ 62.325MB (SLO: <64.500MB -3.4%) vs baseline: +4.9% ✅ appsec-postTime: ✅ 6.590ms (SLO: <6.750ms -2.4%) vs baseline: +0.3% Memory: ✅ 62.364MB (SLO: <64.500MB -3.3%) vs baseline: +5.0% ✅ appsec-telemetryTime: ✅ 4.574ms (SLO: <4.750ms -3.7%) vs baseline: -0.4% Memory: ✅ 62.384MB (SLO: <64.500MB -3.3%) vs baseline: +5.0% ✅ debuggerTime: ✅ 1.856ms (SLO: <2.000ms -7.2%) vs baseline: ~same Memory: ✅ 44.709MB (SLO: <45.000MB 🟡 -0.6%) vs baseline: +5.1% ✅ iast-getTime: ✅ 1.858ms (SLO: <2.000ms -7.1%) vs baseline: +0.2% Memory: ✅ 41.760MB (SLO: <49.000MB 📉 -14.8%) vs baseline: +5.2% ✅ profilerTime: ✅ 1.917ms (SLO: <2.100ms -8.7%) vs baseline: ~same Memory: ✅ 44.551MB (SLO: <46.500MB -4.2%) vs baseline: +5.3% ✅ tracerTime: ✅ 3.378ms (SLO: <3.650ms -7.5%) vs baseline: +0.3% Memory: ✅ 51.433MB (SLO: <53.500MB -3.9%) vs baseline: +4.8% ✅ tracer-nativeTime: ✅ 3.374ms (SLO: <3.650ms -7.6%) vs baseline: +0.2% Memory: ✅ 52.691MB (SLO: <53.500MB 🟡 -1.5%) vs baseline: +5.0% 🟡 flasksqli - 6/6✅ appsec-enabledTime: ✅ 3.942ms (SLO: <4.200ms -6.1%) vs baseline: +0.4% Memory: ✅ 62.618MB (SLO: <66.000MB -5.1%) vs baseline: +5.0% ✅ iast-enabledTime: ✅ 2.451ms (SLO: <2.800ms 📉 -12.5%) vs baseline: -0.6% Memory: ✅ 58.191MB (SLO: <59.000MB 🟡 -1.4%) vs baseline: +4.9% ✅ tracer-enabledTime: ✅ 2.078ms (SLO: <2.250ms -7.7%) vs baseline: -0.2% Memory: ✅ 51.234MB (SLO: <53.500MB -4.2%) vs baseline: +5.0% 🟡 otelspan - 22/22✅ add-eventTime: ✅ 46.005ms (SLO: <47.150ms -2.4%) vs baseline: +1.7% Memory: ✅ 44.704MB (SLO: <46.500MB -3.9%) vs baseline: +5.4% ✅ add-metricsTime: ✅ 320.341ms (SLO: <344.800ms -7.1%) vs baseline: -0.7% Memory: ✅ 553.767MB (SLO: <562.000MB 🟡 -1.5%) vs baseline: +5.0% ✅ add-tagsTime: ✅ 292.738ms (SLO: <314.000ms -6.8%) vs baseline: +1.3% Memory: ✅ 554.090MB (SLO: <563.500MB 🟡 -1.7%) vs baseline: +4.9% ✅ get-contextTime: ✅ 82.624ms (SLO: <92.350ms 📉 -10.5%) vs baseline: -0.2% Memory: ✅ 39.770MB (SLO: <46.500MB 📉 -14.5%) vs baseline: +5.4% ✅ is-recordingTime: ✅ 42.814ms (SLO: <44.500ms -3.8%) vs baseline: -0.9% Memory: ✅ 44.080MB (SLO: <46.500MB -5.2%) vs baseline: +5.1% ✅ record-exceptionTime: ✅ 61.930ms (SLO: <67.650ms -8.5%) vs baseline: +0.5% Memory: ✅ 40.113MB (SLO: <46.500MB 📉 -13.7%) vs baseline: +5.3% ✅ set-statusTime: ✅ 48.854ms (SLO: <50.400ms -3.1%) vs baseline: -0.3% Memory: ✅ 44.082MB (SLO: <46.500MB -5.2%) vs baseline: +5.2% ✅ startTime: ✅ 42.240ms (SLO: <43.450ms -2.8%) vs baseline: +0.2% Memory: ✅ 44.125MB (SLO: <46.500MB -5.1%) vs baseline: +5.5% ✅ start-finishTime: ✅ 84.374ms (SLO: <88.000ms -4.1%) vs baseline: +1.3% Memory: ✅ 34.053MB (SLO: <46.500MB 📉 -26.8%) vs baseline: +5.7% ✅ start-finish-telemetryTime: ✅ 84.918ms (SLO: <89.000ms -4.6%) vs baseline: ~same Memory: ✅ 34.013MB (SLO: <46.500MB 📉 -26.9%) vs baseline: +5.5% ✅ update-nameTime: ✅ 44.143ms (SLO: <45.150ms -2.2%) vs baseline: -0.5% Memory: ✅ 44.351MB (SLO: <46.500MB -4.6%) vs baseline: +5.4%
|
cdfa633
to
0870b87
Compare
0817401
to
6067107
Compare
--- | ||
features: | ||
- | | ||
tracing: Added support for resource renaming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we get a better description here? what is this? why/when should someone enable? are there any public docs on this feature we can link to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a description of the feature. Trying to answer both questions.
There are no public docs for now on this feature, this is experimental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a scenario to flask_simple and django_simple benchmarks for this feature? would be good to know the performance trade off of enabling it.
class ResourceRenamingProcessor(SpanProcessor): | ||
def __init__(self): | ||
self._URL_PATH_EXTRACTION_RE = re.compile( | ||
r"^(?P<protocol>[a-z]+://(?P<host>[^?/]+))?(?P<path>/[^?]*)(?P<query>(\?).*)?$" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason not to use urllib.parse.urlparse
to separate out the path from query/fragment/etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I started to work on that feature, I discovered that, in the general case, urllib.parse and any regular parsing method could be really strict, while we could have, on some orgs, weird urls for many different reasons.
Instead of failing completely in those cases, using a regex is more robust and in many occasion, can successfully extract the path of the url, even if it's polluted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of those examples are covered in the tests? (some of those are parsed weird with urlparse
, but fine with the regex?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed it with @christophe-papazian and we can use urllib here.
629f064
to
39939c9
Compare
5d6de1a
to
01d4016
Compare
BenchmarksBenchmark execution time: 2025-09-19 14:58:11 Comparing candidate commit 629f064 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 553 metrics, 9 unstable metrics. scenario:iastaspectsospath-ospathsplit_aspect
scenario:telemetryaddmetric-1-distribution-metric-1-times
|
01d4016
to
e61c3ad
Compare
Description
This PR implements the resource renaming feature specified in RFC-1051. This contains the following changes:
http.endpoint
tag whenhttp.route
is not reported by an integration. Its value is a "simplified endpoint" representation of the url path.The feature is turned off by default behind the env var: "DD_TRACE_RESOURCE_RENAMING_ENABLED"
APPSEC-58892
Notes
Added the TRACING_CONFIG_NONDEFAULT_3 scenario to the CI. It contains the system tests for this feature.
Checklist
Reviewer Checklist