-
Couldn't load subscription status.
- Fork 315
Refactor Severless Gateway Inferred Span #9388
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
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: 6f9eee1 | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.02 s) : 0, 1019610
Total [baseline] (10.7 s) : 0, 10699545
Agent [candidate] (1.022 s) : 0, 1022423
Total [candidate] (10.681 s) : 0, 10680600
section appsec
Agent [baseline] (1.199 s) : 0, 1198955
Total [baseline] (11.063 s) : 0, 11062913
Agent [candidate] (1.196 s) : 0, 1195933
Total [candidate] (11.104 s) : 0, 11104162
section iast
Agent [baseline] (1.153 s) : 0, 1153212
Total [baseline] (11.048 s) : 0, 11048242
Agent [candidate] (1.154 s) : 0, 1154086
Total [candidate] (11.05 s) : 0, 11049691
section profiling
Agent [baseline] (1.163 s) : 0, 1162625
Total [baseline] (11.041 s) : 0, 11041465
Agent [candidate] (1.166 s) : 0, 1166365
Total [candidate] (11.064 s) : 0, 11063596
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.447 ms) : 0, 1447
crashtracking [candidate] (1.455 ms) : 0, 1455
BytebuddyAgent [baseline] (687.139 ms) : 0, 687139
BytebuddyAgent [candidate] (691.167 ms) : 0, 691167
GlobalTracer [baseline] (258.096 ms) : 0, 258096
GlobalTracer [candidate] (259.527 ms) : 0, 259527
AppSec [baseline] (31.687 ms) : 0, 31687
AppSec [candidate] (31.873 ms) : 0, 31873
Debugger [baseline] (6.352 ms) : 0, 6352
Debugger [candidate] (6.39 ms) : 0, 6390
Remote Config [baseline] (669.399 µs) : 0, 669
Remote Config [candidate] (678.945 µs) : 0, 679
Telemetry [baseline] (13.253 ms) : 0, 13253
Telemetry [candidate] (10.267 ms) : 0, 10267
section appsec
crashtracking [baseline] (1.462 ms) : 0, 1462
crashtracking [candidate] (1.459 ms) : 0, 1459
BytebuddyAgent [baseline] (712.154 ms) : 0, 712154
BytebuddyAgent [candidate] (710.303 ms) : 0, 710303
GlobalTracer [baseline] (251.437 ms) : 0, 251437
GlobalTracer [candidate] (250.723 ms) : 0, 250723
IAST [baseline] (25.072 ms) : 0, 25072
IAST [candidate] (25.009 ms) : 0, 25009
AppSec [baseline] (170.433 ms) : 0, 170433
AppSec [candidate] (170.755 ms) : 0, 170755
Debugger [baseline] (6.128 ms) : 0, 6128
Debugger [candidate] (6.145 ms) : 0, 6145
Remote Config [baseline] (614.362 µs) : 0, 614
Remote Config [candidate] (609.531 µs) : 0, 610
Telemetry [baseline] (10.587 ms) : 0, 10587
Telemetry [candidate] (9.877 ms) : 0, 9877
section iast
crashtracking [baseline] (1.455 ms) : 0, 1455
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (807.753 ms) : 0, 807753
BytebuddyAgent [candidate] (808.726 ms) : 0, 808726
GlobalTracer [baseline] (249.214 ms) : 0, 249214
GlobalTracer [candidate] (249.265 ms) : 0, 249265
IAST [baseline] (30.038 ms) : 0, 30038
IAST [candidate] (29.266 ms) : 0, 29266
AppSec [baseline] (28.919 ms) : 0, 28919
AppSec [candidate] (29.566 ms) : 0, 29566
Debugger [baseline] (6.081 ms) : 0, 6081
Debugger [candidate] (6.047 ms) : 0, 6047
Remote Config [baseline] (586.687 µs) : 0, 587
Remote Config [candidate] (583.036 µs) : 0, 583
Telemetry [baseline] (8.132 ms) : 0, 8132
Telemetry [candidate] (8.127 ms) : 0, 8127
section profiling
crashtracking [baseline] (1.422 ms) : 0, 1422
crashtracking [candidate] (1.429 ms) : 0, 1429
BytebuddyAgent [baseline] (717.688 ms) : 0, 717688
BytebuddyAgent [candidate] (720.193 ms) : 0, 720193
GlobalTracer [baseline] (236.584 ms) : 0, 236584
GlobalTracer [candidate] (237.107 ms) : 0, 237107
AppSec [baseline] (31.082 ms) : 0, 31082
AppSec [candidate] (31.427 ms) : 0, 31427
Debugger [baseline] (6.477 ms) : 0, 6477
Debugger [candidate] (6.487 ms) : 0, 6487
Remote Config [baseline] (689.313 µs) : 0, 689
Remote Config [candidate] (709.256 µs) : 0, 709
Telemetry [baseline] (16.411 ms) : 0, 16411
Telemetry [candidate] (16.37 ms) : 0, 16370
ProfilingAgent [baseline] (101.357 ms) : 0, 101357
ProfilingAgent [candidate] (101.668 ms) : 0, 101668
Profiling [baseline] (101.956 ms) : 0, 101956
Profiling [candidate] (102.264 ms) : 0, 102264
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.027 s) : 0, 1027018
Total [baseline] (8.682 s) : 0, 8681621
Agent [candidate] (1.023 s) : 0, 1023041
Total [candidate] (8.668 s) : 0, 8668282
section iast
Agent [baseline] (1.163 s) : 0, 1162915
Total [baseline] (9.393 s) : 0, 9393463
Agent [candidate] (1.16 s) : 0, 1159754
Total [candidate] (9.417 s) : 0, 9417397
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.471 ms) : 0, 1471
crashtracking [candidate] (1.437 ms) : 0, 1437
BytebuddyAgent [baseline] (691.846 ms) : 0, 691846
BytebuddyAgent [candidate] (687.169 ms) : 0, 687169
GlobalTracer [baseline] (259.551 ms) : 0, 259551
GlobalTracer [candidate] (259.79 ms) : 0, 259790
AppSec [baseline] (31.787 ms) : 0, 31787
AppSec [candidate] (31.776 ms) : 0, 31776
Debugger [baseline] (6.388 ms) : 0, 6388
Debugger [candidate] (6.37 ms) : 0, 6370
Remote Config [baseline] (686.33 µs) : 0, 686
Remote Config [candidate] (674.357 µs) : 0, 674
Telemetry [baseline] (14.164 ms) : 0, 14164
Telemetry [candidate] (14.859 ms) : 0, 14859
section iast
crashtracking [baseline] (1.458 ms) : 0, 1458
crashtracking [candidate] (1.464 ms) : 0, 1464
BytebuddyAgent [baseline] (815.925 ms) : 0, 815925
BytebuddyAgent [candidate] (811.415 ms) : 0, 811415
GlobalTracer [baseline] (250.735 ms) : 0, 250735
GlobalTracer [candidate] (251.593 ms) : 0, 251593
IAST [baseline] (29.094 ms) : 0, 29094
IAST [candidate] (31.597 ms) : 0, 31597
AppSec [baseline] (29.675 ms) : 0, 29675
AppSec [candidate] (27.653 ms) : 0, 27653
Debugger [baseline] (6.091 ms) : 0, 6091
Debugger [candidate] (6.211 ms) : 0, 6211
Remote Config [baseline] (598.263 µs) : 0, 598
Remote Config [candidate] (603.218 µs) : 0, 603
Telemetry [baseline] (8.242 ms) : 0, 8242
Telemetry [candidate] (8.248 ms) : 0, 8248
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 1 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section baseline
no_agent (4.312 ms) : 4263, 4362
. : milestone, 4312,
iast (9.642 ms) : 9478, 9805
. : milestone, 9642,
iast_FULL (15.342 ms) : 15039, 15644
. : milestone, 15342,
iast_GLOBAL (10.283 ms) : 10102, 10464
. : milestone, 10283,
profiling (8.99 ms) : 8842, 9138
. : milestone, 8990,
tracing (8.35 ms) : 8228, 8471
. : milestone, 8350,
section candidate
no_agent (4.211 ms) : 4160, 4262
. : milestone, 4211,
iast (10.58 ms) : 10399, 10761
. : milestone, 10580,
iast_FULL (14.203 ms) : 13921, 14484
. : milestone, 14203,
iast_GLOBAL (10.592 ms) : 10403, 10781
. : milestone, 10592,
profiling (8.601 ms) : 8469, 8732
. : milestone, 8601,
tracing (7.815 ms) : 7701, 7929
. : milestone, 7815,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section baseline
no_agent (36.651 ms) : 36361, 36941
. : milestone, 36651,
appsec (47.056 ms) : 46658, 47454
. : milestone, 47056,
code_origins (46.141 ms) : 45740, 46541
. : milestone, 46141,
iast (43.975 ms) : 43588, 44362
. : milestone, 43975,
profiling (47.225 ms) : 46807, 47643
. : milestone, 47225,
tracing (44.596 ms) : 44219, 44973
. : milestone, 44596,
section candidate
no_agent (36.768 ms) : 36474, 37062
. : milestone, 36768,
appsec (47.245 ms) : 46828, 47662
. : milestone, 47245,
code_origins (45.432 ms) : 45043, 45822
. : milestone, 45432,
iast (45.004 ms) : 44623, 45386
. : milestone, 45004,
profiling (47.866 ms) : 47408, 48323
. : milestone, 47866,
tracing (43.805 ms) : 43416, 44193
. : milestone, 43805,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section baseline
no_agent (1.473 ms) : 1462, 1485
. : milestone, 1473,
appsec (3.633 ms) : 3421, 3845
. : milestone, 3633,
iast (2.203 ms) : 2140, 2265
. : milestone, 2203,
iast_GLOBAL (2.244 ms) : 2181, 2307
. : milestone, 2244,
profiling (2.076 ms) : 2024, 2128
. : milestone, 2076,
tracing (2.024 ms) : 1975, 2073
. : milestone, 2024,
section candidate
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.706 ms) : 3488, 3923
. : milestone, 3706,
iast (2.199 ms) : 2136, 2262
. : milestone, 2199,
iast_GLOBAL (2.234 ms) : 2172, 2297
. : milestone, 2234,
profiling (2.063 ms) : 2012, 2115
. : milestone, 2063,
tracing (2.022 ms) : 1973, 2071
. : milestone, 2022,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~6f9eee1e64, baseline=1.54.0-SNAPSHOT~4313a72404
dateFormat X
axisFormat %s
section baseline
no_agent (15.011 s) : 15011000, 15011000
. : milestone, 15011000,
appsec (14.975 s) : 14975000, 14975000
. : milestone, 14975000,
iast (18.674 s) : 18674000, 18674000
. : milestone, 18674000,
iast_GLOBAL (17.838 s) : 17838000, 17838000
. : milestone, 17838000,
profiling (15.496 s) : 15496000, 15496000
. : milestone, 15496000,
tracing (15.069 s) : 15069000, 15069000
. : milestone, 15069000,
section candidate
no_agent (14.962 s) : 14962000, 14962000
. : milestone, 14962000,
appsec (14.928 s) : 14928000, 14928000
. : milestone, 14928000,
iast (18.765 s) : 18765000, 18765000
. : milestone, 18765000,
iast_GLOBAL (18.033 s) : 18033000, 18033000
. : milestone, 18033000,
profiling (15.394 s) : 15394000, 15394000
. : milestone, 15394000,
tracing (14.876 s) : 14876000, 14876000
. : milestone, 14876000,
|
ad6356f to
e8c2baf
Compare
| long startTime; | ||
| try { | ||
| startTime = Long.parseLong(header(PROXY_START_TIME_MS)) * 1000; // Convert to microseconds | ||
| } catch (NumberFormatException e) { | ||
| return extracted; // Invalid timestamp | ||
| } |
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 have a feeling that we need to have some sort of utility code for such places?
WDYT?
Something like: long startTime = parseLong((header(PROXY_START_TIME_MS), ?some_default? ) * 1000;
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 don't think you can have a default start time in this case.
Gateway inferred spans represent the duration spent in upstream proxy servers (that are not instrumented for distributed tracing, they only inject few Datadog headers).
The start time represents the timestamp the request was intercepted by the proxy server and will be use to create a "proxy span" into the downstream service.
So if you don't have a valid start time, it's most likely a proxy issue, and you can't really "guess" when the proxy intercepted the upstream request.
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.
The change looks good. May we keep settings tags in a decorator to be sure we are consistent with other part of the code?
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java
Show resolved
Hide resolved
eb6874e to
735271a
Compare
* Creates inferred proxy spans for API Gateway calls via presence of http headers --------- Co-authored-by: Zarir Hamza <[email protected]>
|
Rebasing to run load benchmark once again 🤷 |
| span.setTag(RESOURCE_NAME, header(PROXY_HTTP_METHOD) + " " + header(PROXY_PATH)); | ||
| span.setTag(SERVICE_NAME, header(PROXY_DOMAIN_NAME)); |
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.
service name and resource name can be set directly on the span. setting them through a tag will trigger the taginterceptor that will do the same but this is way less efficient
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 switched to the serviceName() method and removed the resource name in favor of the tag interceptor.
internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java
Outdated
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java
Outdated
Show resolved
Hide resolved
Avoid duplicate expensive context extraction Avoid subclassing tracing span for serverless but used serverless context element instead to store / track inferred span while keep tracing feature untouched Improved propagator to not create / capture inferred span context element on invalid data Rework context element to hold the inferred spans and its captured data Release captured data as soon as they span start (never read after this point so reclaiming memory) Refactor context element and propagator into the right package, not context component (product / feature agnostic) Refactor unit tests
735271a to
6f9eee1
Compare
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.
lgtm. Thanks for having addressed the comments and for the refactoring
What Does This Do
This PR continues @zarirhamza work about Serverless gateway inferred span in #8336 and brings the following improvements:
Motivation
Restore Serverless feature while meeting design and performance requirements
Additional Notes
This requires to complete the instrumentation refactoring first.
I already did a lot of them here:
The last one is pending for review:
beforeFinishMigration #9422Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: LANGPLAT-680