Skip to content

Allow comma in request header tag values#6376

Merged
bantonsson merged 1 commit into
masterfrom
ban/allow-comma-in-request-header-tags
Dec 18, 2023
Merged

Allow comma in request header tag values#6376
bantonsson merged 1 commit into
masterfrom
ban/allow-comma-in-request-header-tags

Conversation

@bantonsson

@bantonsson bantonsson commented Dec 15, 2023 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Contributor

What Does This Do

Allows the trace header tags values to contain commas (previously the value was split at the first comma, and only the first part was used).

It is possible to switch to the old behavior by setting the Java property -Ddd.trace.request_header.tags.comma.allowed=false or the environment variable DD_TRACE_REQUEST_HEADER_TAGS_COMMA_ALLOWED=false

Motivation

Splitting at the first value breaks some encodings that use commas in the values. This was also the default behavior for some propagators up until v1.4.0.

Additional Notes

Although it's not possible in HTTP to distinguish between multiple instances of the same header being combined into a comma separate list of the values, and a single header with a value with comma in it, we could be lenient here since the header names to read are user supplied.

Jira ticket: APMJAVA-1140
GitHub Ticket: #6148

@bantonsson bantonsson added tag: breaking change Breaking changes comp: context propagation Trace context propagation labels Dec 15, 2023
@bantonsson bantonsson mentioned this pull request Dec 15, 2023
Closed
@bantonsson
bantonsson force-pushed the ban/allow-comma-in-request-header-tags branch from 659a165 to dfe9016 Compare December 15, 2023 16:56
@bantonsson
bantonsson marked this pull request as ready for review December 15, 2023 16:56
@bantonsson
bantonsson requested a review from a team as a code owner December 15, 2023 16:56
@pr-commenter

pr-commenter Bot commented Dec 15, 2023 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master ban/allow-comma-in-request-header-tags
git_commit_date 1702652090 1702659361
git_commit_sha 7a1a4ca dfe9016
release_version 1.26.0-SNAPSHOT~7a1a4cac29 1.26.0-SNAPSHOT~dfe9016c2e
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1702661887 1702661887
ci_job_id 391028378 391028378
ci_pipeline_id 25173190 25173190
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 49 metrics, 5 unstable metrics.

Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1053478
Total [baseline] (9.315 s) : 0, 9315337
Agent [candidate] (1.046 s) : 0, 1046214
Total [candidate] (9.351 s) : 0, 9351367
section appsec
Agent [baseline] (1.146 s) : 0, 1146078
Total [baseline] (9.457 s) : 0, 9456734
Agent [candidate] (1.146 s) : 0, 1145760
Total [candidate] (9.425 s) : 0, 9425346
section iast
Agent [baseline] (1.166 s) : 0, 1166174
Total [baseline] (9.539 s) : 0, 9538703
Agent [candidate] (1.165 s) : 0, 1164668
Total [candidate] (9.637 s) : 0, 9636563
section profiling
Agent [baseline] (1.247 s) : 0, 1247053
Total [baseline] (9.59 s) : 0, 9589697
Agent [candidate] (1.247 s) : 0, 1247380
Total [candidate] (9.626 s) : 0, 9625814
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.053 s -
Agent appsec 1.146 s 92.6 ms (8.8%)
Agent iast 1.166 s 112.696 ms (10.7%)
Agent profiling 1.247 s 193.575 ms (18.4%)
Total tracing 9.315 s -
Total appsec 9.457 s 141.397 ms (1.5%)
Total iast 9.539 s 223.365 ms (2.4%)
Total profiling 9.59 s 274.359 ms (2.9%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.046 s -
Agent appsec 1.146 s 99.546 ms (9.5%)
Agent iast 1.165 s 118.455 ms (11.3%)
Agent profiling 1.247 s 201.166 ms (19.2%)
Total tracing 9.351 s -
Total appsec 9.425 s 73.979 ms (0.8%)
Total iast 9.637 s 285.196 ms (3.0%)
Total profiling 9.626 s 274.447 ms (2.9%) 
gantt
    title petclinic - break down per module: candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (651.496 ms) : 0, 651496
BytebuddyAgent [candidate] (646.796 ms) : 0, 646796
GlobalTracer [baseline] (308.222 ms) : 0, 308222
GlobalTracer [candidate] (306.723 ms) : 0, 306723
AppSec [baseline] (51.318 ms) : 0, 51318
AppSec [candidate] (50.855 ms) : 0, 50855
Remote Config [baseline] (676.763 µs) : 0, 677
Remote Config [candidate] (663.821 µs) : 0, 664
Telemetry [baseline] (7.264 ms) : 0, 7264
Telemetry [candidate] (7.144 ms) : 0, 7144
section appsec
BytebuddyAgent [baseline] (647.681 ms) : 0, 647681
BytebuddyAgent [candidate] (648.258 ms) : 0, 648258
GlobalTracer [baseline] (306.548 ms) : 0, 306548
GlobalTracer [candidate] (306.633 ms) : 0, 306633
AppSec [baseline] (149.404 ms) : 0, 149404
AppSec [candidate] (149.179 ms) : 0, 149179
Remote Config [baseline] (658.472 µs) : 0, 658
Remote Config [candidate] (645.935 µs) : 0, 646
Telemetry [baseline] (7.48 ms) : 0, 7480
Telemetry [candidate] (6.802 ms) : 0, 6802
section iast
BytebuddyAgent [baseline] (769.856 ms) : 0, 769856
BytebuddyAgent [candidate] (767.805 ms) : 0, 767805
GlobalTracer [baseline] (284.058 ms) : 0, 284058
GlobalTracer [candidate] (285.504 ms) : 0, 285504
AppSec [baseline] (48.797 ms) : 0, 48797
AppSec [candidate] (48.839 ms) : 0, 48839
IAST [baseline] (19.796 ms) : 0, 19796
IAST [candidate] (21.171 ms) : 0, 21171
Remote Config [baseline] (622.964 µs) : 0, 623
Remote Config [candidate] (607.747 µs) : 0, 608
Telemetry [baseline] (8.633 ms) : 0, 8633
Telemetry [candidate] (6.493 ms) : 0, 6493
section profiling
BytebuddyAgent [baseline] (660.422 ms) : 0, 660422
BytebuddyAgent [candidate] (661.012 ms) : 0, 661012
GlobalTracer [baseline] (377.268 ms) : 0, 377268
GlobalTracer [candidate] (378.299 ms) : 0, 378299
AppSec [baseline] (51.514 ms) : 0, 51514
AppSec [candidate] (51.496 ms) : 0, 51496
Remote Config [baseline] (676.939 µs) : 0, 677
Remote Config [candidate] (678.338 µs) : 0, 678
Telemetry [baseline] (7.493 ms) : 0, 7493
Telemetry [candidate] (7.523 ms) : 0, 7523
ProfilingAgent [baseline] (95.335 ms) : 0, 95335
ProfilingAgent [candidate] (94.041 ms) : 0, 94041
Profiling [baseline] (95.36 ms) : 0, 95360
Profiling [candidate] (94.066 ms) : 0, 94066
Loading
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.052 s) : 0, 1052316
Total [baseline] (8.743 s) : 0, 8742698
Agent [candidate] (1.045 s) : 0, 1045399
Total [candidate] (8.739 s) : 0, 8738736
section iast
Agent [baseline] (1.163 s) : 0, 1163055
Total [baseline] (9.241 s) : 0, 9241317
Agent [candidate] (1.176 s) : 0, 1175615
Total [candidate] (9.312 s) : 0, 9311597
section iast_TELEMETRY_OFF
Agent [baseline] (1.171 s) : 0, 1171033
Total [baseline] (9.269 s) : 0, 9269037
Agent [candidate] (1.161 s) : 0, 1161471
Total [candidate] (9.282 s) : 0, 9282148
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.052 s -
Agent iast 1.163 s 110.739 ms (10.5%)
Agent iast_TELEMETRY_OFF 1.171 s 118.717 ms (11.3%)
Total tracing 8.743 s -
Total iast 9.241 s 498.619 ms (5.7%)
Total iast_TELEMETRY_OFF 9.269 s 526.34 ms (6.0%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.045 s -
Agent iast 1.176 s 130.216 ms (12.5%)
Agent iast_TELEMETRY_OFF 1.161 s 116.072 ms (11.1%)
Total tracing 8.739 s -
Total iast 9.312 s 572.861 ms (6.6%)
Total iast_TELEMETRY_OFF 9.282 s 543.412 ms (6.2%) 
gantt
    title insecure-bank - break down per module: candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (651.132 ms) : 0, 651132
BytebuddyAgent [candidate] (645.969 ms) : 0, 645969
GlobalTracer [baseline] (307.574 ms) : 0, 307574
GlobalTracer [candidate] (306.417 ms) : 0, 306417
AppSec [baseline] (51.297 ms) : 0, 51297
AppSec [candidate] (51.123 ms) : 0, 51123
Remote Config [baseline] (675.663 µs) : 0, 676
Remote Config [candidate] (671.913 µs) : 0, 672
Telemetry [baseline] (7.159 ms) : 0, 7159
Telemetry [candidate] (7.156 ms) : 0, 7156
section iast
BytebuddyAgent [baseline] (766.343 ms) : 0, 766343
BytebuddyAgent [candidate] (773.985 ms) : 0, 773985
GlobalTracer [baseline] (284.119 ms) : 0, 284119
GlobalTracer [candidate] (287.79 ms) : 0, 287790
AppSec [baseline] (49.017 ms) : 0, 49017
AppSec [candidate] (49.573 ms) : 0, 49573
IAST [baseline] (22.192 ms) : 0, 22192
IAST [candidate] (22.447 ms) : 0, 22447
Remote Config [baseline] (616.345 µs) : 0, 616
Remote Config [candidate] (612.794 µs) : 0, 613
Telemetry [baseline] (6.473 ms) : 0, 6473
Telemetry [candidate] (6.538 ms) : 0, 6538
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (770.217 ms) : 0, 770217
BytebuddyAgent [candidate] (762.631 ms) : 0, 762631
GlobalTracer [baseline] (288.471 ms) : 0, 288471
GlobalTracer [candidate] (286.417 ms) : 0, 286417
AppSec [baseline] (49.714 ms) : 0, 49714
AppSec [candidate] (49.314 ms) : 0, 49314
IAST [baseline] (19.984 ms) : 0, 19984
IAST [candidate] (20.747 ms) : 0, 20747
Remote Config [baseline] (622.765 µs) : 0, 623
Remote Config [candidate] (633.348 µs) : 0, 633
Telemetry [baseline] (7.308 ms) : 0, 7308
Telemetry [candidate] (7.397 ms) : 0, 7397
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2023-12-15T17:17:19 2023-12-15T17:33:54
git_branch master ban/allow-comma-in-request-header-tags
git_commit_date 1702652090 1702659361
git_commit_sha 7a1a4ca dfe9016
release_version 1.26.0-SNAPSHOT~7a1a4cac29 1.26.0-SNAPSHOT~dfe9016c2e
start_time 2023-12-15T17:17:06 2023-12-15T17:33:41
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1702661887 1702661887
ci_job_id 391028378 391028378
ci_pipeline_id 25173190 25173190
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 8 metrics, 14 unstable metrics.

Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.349 ms) : 1330, 1368
.   : milestone, 1349,
appsec (1.743 ms) : 1718, 1768
.   : milestone, 1743,
iast (1.511 ms) : 1487, 1536
.   : milestone, 1511,
profiling (1.516 ms) : 1491, 1542
.   : milestone, 1516,
tracing (1.492 ms) : 1468, 1517
.   : milestone, 1492,
section candidate
no_agent (1.358 ms) : 1339, 1378
.   : milestone, 1358,
appsec (1.757 ms) : 1732, 1782
.   : milestone, 1757,
iast (1.511 ms) : 1486, 1535
.   : milestone, 1511,
profiling (1.562 ms) : 1535, 1588
.   : milestone, 1562,
tracing (1.493 ms) : 1469, 1518
.   : milestone, 1493,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.349 ms [1.33 ms, 1.368 ms] -
appsec 1.743 ms [1.718 ms, 1.768 ms] 393.212 µs (29.1%)
iast 1.511 ms [1.487 ms, 1.536 ms] 162.158 µs (12.0%)
profiling 1.516 ms [1.491 ms, 1.542 ms] 166.906 µs (12.4%)
tracing 1.492 ms [1.468 ms, 1.517 ms] 142.779 µs (10.6%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.358 ms [1.339 ms, 1.378 ms] -
appsec 1.757 ms [1.732 ms, 1.782 ms] 398.487 µs (29.3%)
iast 1.511 ms [1.486 ms, 1.535 ms] 152.18 µs (11.2%)
profiling 1.562 ms [1.535 ms, 1.588 ms] 203.366 µs (15.0%)
tracing 1.493 ms [1.469 ms, 1.518 ms] 134.912 µs (9.9%)
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.26.0-SNAPSHOT~dfe9016c2e, baseline=1.26.0-SNAPSHOT~7a1a4cac29
    dateFormat X
    axisFormat %s
section baseline
no_agent (365.909 µs) : 346, 386
.   : milestone, 366,
iast (480.431 µs) : 459, 502
.   : milestone, 480,
iast_FULL (535.929 µs) : 515, 556
.   : milestone, 536,
iast_INACTIVE (451.773 µs) : 430, 473
.   : milestone, 452,
iast_TELEMETRY_OFF (468.647 µs) : 448, 489
.   : milestone, 469,
tracing (436.789 µs) : 416, 457
.   : milestone, 437,
section candidate
no_agent (365.496 µs) : 345, 386
.   : milestone, 365,
iast (470.96 µs) : 450, 491
.   : milestone, 471,
iast_FULL (534.987 µs) : 514, 556
.   : milestone, 535,
iast_INACTIVE (445.01 µs) : 424, 466
.   : milestone, 445,
iast_TELEMETRY_OFF (464.423 µs) : 444, 485
.   : milestone, 464,
tracing (445.206 µs) : 425, 466
.   : milestone, 445,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.909 µs [345.965 µs, 385.854 µs] -
iast 480.431 µs [459.359 µs, 501.504 µs] 114.522 µs (31.3%)
iast_FULL 535.929 µs [515.456 µs, 556.403 µs] 170.02 µs (46.5%)
iast_INACTIVE 451.773 µs [430.474 µs, 473.072 µs] 85.863 µs (23.5%)
iast_TELEMETRY_OFF 468.647 µs [447.83 µs, 489.464 µs] 102.738 µs (28.1%)
tracing 436.789 µs [416.222 µs, 457.356 µs] 70.879 µs (19.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.496 µs [345.079 µs, 385.912 µs] -
iast 470.96 µs [450.452 µs, 491.469 µs] 105.465 µs (28.9%)
iast_FULL 534.987 µs [514.378 µs, 555.596 µs] 169.492 µs (46.4%)
iast_INACTIVE 445.01 µs [424.113 µs, 465.908 µs] 79.515 µs (21.8%)
iast_TELEMETRY_OFF 464.423 µs [443.658 µs, 485.187 µs] 98.927 µs (27.1%)
tracing 445.206 µs [424.786 µs, 465.626 µs] 79.711 µs (21.8%)

ygree
ygree approved these changes Dec 16, 2023
View reviewed changes

@ygree ygree left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for a quick fix!

@bantonsson
bantonsson merged commit 9b7fc7a into master Dec 18, 2023
@bantonsson
bantonsson deleted the ban/allow-comma-in-request-header-tags branch December 18, 2023 08:27
@github-actions github-actions Bot added this to the 1.26.0 milestone Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ygree ygree ygree approved these changes
@am312 am312 Awaiting requested review from am312 am312 is a code owner automatically assigned from DataDog/apm-java
@nayeem-kamal nayeem-kamal Awaiting requested review from nayeem-kamal nayeem-kamal is a code owner automatically assigned from DataDog/apm-java

Assignees

No one assigned

Labels

comp: context propagation Trace context propagation tag: breaking change Breaking changes

Projects

None yet

Milestone

1.26.0

Development

Successfully merging this pull request may close these issues.

2 participants

@bantonsson @ygree