Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change hash computation for protobuf to better represent impacting changes + save proto number in schema #8201

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Change hash computation for protobuf to better represent impacting changes + save proto number in schema #8201

wants to merge 3 commits into from

Conversation

vandonr
Copy link
Contributor

@vandonr vandonr commented Jan 14, 2025 •
edited
Loading

What Does This Do

Change the way the hash is computed for protobuf schemas:

  • to catch some changes that impact the wire format but were not changing the hash before (like changing a field number for instance)
  • to stay constant on changes that don't impact the wire format, but were changing the hash before (like reorganizing fields without changing their number)
    It should also be marginally faster as we don't hash a whole string, and also isolates the hash from json shennanigans like the OpenAPI version for instance.

This new hash computation is aligned with what's done in the dotnet version of the instrumentation in DataDog/dd-trace-dotnet#6166

ℹ️ Also add protobuf number to the OpenAPI schema saved, so that we can reconstruct the hash if needed, and also because it's an important component of a protobuf schema.

Additional Notes

Depending on how schema hashes are used now in DSM, when deploying this, it may look to users as if all their schemas changed

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@vandonr vandonr requested review from a team as code owners January 14, 2025 16:35
@vandonr vandonr requested a review from smola January 14, 2025 16:36
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 14, 2025 •
edited
Loading

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

  • Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

@vandonr vandonr added inst: protobuf Protocol Buffer instrumentation type: enhancement comp: data streams Data Streams Monitoring labels Jan 14, 2025
@pr-commenter
Copy link

pr-commenter bot commented Jan 14, 2025 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master vandonr/proto
git_commit_date 1736890492 1736938770
git_commit_sha 8b63e5a d702a13
release_version 1.46.0-SNAPSHOT~8b63e5a85b 1.46.0-SNAPSHOT~d702a13e68
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1736941339 1736941339
ci_job_id 767038699 767038699
ci_pipeline_id 53054901 53054901
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 58 metrics, 5 unstable metrics.

Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.068 s) : 0, 1068375
Total [baseline] (8.666 s) : 0, 8665547
Agent [candidate] (1.07 s) : 0, 1069896
Total [candidate] (8.677 s) : 0, 8677345
section iast
Agent [baseline] (1.186 s) : 0, 1186240
Total [baseline] (9.243 s) : 0, 9242833
Agent [candidate] (1.185 s) : 0, 1184695
Total [candidate] (9.232 s) : 0, 9231735
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.183 s) : 0, 1183065
Total [baseline] (9.186 s) : 0, 9186051
Agent [candidate] (1.198 s) : 0, 1197808
Total [candidate] (9.259 s) : 0, 9259134
section iast_TELEMETRY_OFF
Agent [baseline] (1.185 s) : 0, 1185249
Total [baseline] (9.257 s) : 0, 9256627
Agent [candidate] (1.177 s) : 0, 1177083
Total [candidate] (9.261 s) : 0, 9261177
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.068 s -
Agent iast 1.186 s 117.865 ms (11.0%)
Agent iast_HARDCODED_SECRET_DISABLED 1.183 s 114.689 ms (10.7%)
Agent iast_TELEMETRY_OFF 1.185 s 116.874 ms (10.9%)
Total tracing 8.666 s -
Total iast 9.243 s 577.286 ms (6.7%)
Total iast_HARDCODED_SECRET_DISABLED 9.186 s 520.504 ms (6.0%)
Total iast_TELEMETRY_OFF 9.257 s 591.08 ms (6.8%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.07 s -
Agent iast 1.185 s 114.799 ms (10.7%)
Agent iast_HARDCODED_SECRET_DISABLED 1.198 s 127.912 ms (12.0%)
Agent iast_TELEMETRY_OFF 1.177 s 107.187 ms (10.0%)
Total tracing 8.677 s -
Total iast 9.232 s 554.39 ms (6.4%)
Total iast_HARDCODED_SECRET_DISABLED 9.259 s 581.789 ms (6.7%)
Total iast_TELEMETRY_OFF 9.261 s 583.832 ms (6.7%) 
gantt
    title insecure-bank - break down per module: candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (723.73 ms) : 0, 723730
BytebuddyAgent [candidate] (724.361 ms) : 0, 724361
GlobalTracer [baseline] (259.295 ms) : 0, 259295
GlobalTracer [candidate] (259.745 ms) : 0, 259745
AppSec [baseline] (55.975 ms) : 0, 55975
AppSec [candidate] (56.713 ms) : 0, 56713
Remote Config [baseline] (748.722 µs) : 0, 749
Remote Config [candidate] (739.772 µs) : 0, 740
Telemetry [baseline] (13.386 ms) : 0, 13386
Telemetry [candidate] (13.106 ms) : 0, 13106
section iast
BytebuddyAgent [baseline] (836.016 ms) : 0, 836016
BytebuddyAgent [candidate] (833.473 ms) : 0, 833473
GlobalTracer [baseline] (245.854 ms) : 0, 245854
GlobalTracer [candidate] (247.25 ms) : 0, 247250
AppSec [baseline] (58.134 ms) : 0, 58134
AppSec [candidate] (58.104 ms) : 0, 58104
IAST [baseline] (21.698 ms) : 0, 21698
IAST [candidate] (21.365 ms) : 0, 21365
Remote Config [baseline] (677.889 µs) : 0, 678
Remote Config [candidate] (670.389 µs) : 0, 670
Telemetry [baseline] (8.848 ms) : 0, 8848
Telemetry [candidate] (8.774 ms) : 0, 8774
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (831.716 ms) : 0, 831716
BytebuddyAgent [candidate] (842.058 ms) : 0, 842058
GlobalTracer [baseline] (247.048 ms) : 0, 247048
GlobalTracer [candidate] (250.044 ms) : 0, 250044
AppSec [baseline] (58.295 ms) : 0, 58295
AppSec [candidate] (58.868 ms) : 0, 58868
IAST [baseline] (21.494 ms) : 0, 21494
IAST [candidate] (21.989 ms) : 0, 21989
Remote Config [baseline] (672.115 µs) : 0, 672
Remote Config [candidate] (695.792 µs) : 0, 696
Telemetry [baseline] (8.85 ms) : 0, 8850
Telemetry [candidate] (9.026 ms) : 0, 9026
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (834.381 ms) : 0, 834381
BytebuddyAgent [candidate] (827.202 ms) : 0, 827202
GlobalTracer [baseline] (248.062 ms) : 0, 248062
GlobalTracer [candidate] (246.671 ms) : 0, 246671
AppSec [baseline] (57.816 ms) : 0, 57816
AppSec [candidate] (57.932 ms) : 0, 57932
IAST [baseline] (20.692 ms) : 0, 20692
IAST [candidate] (20.917 ms) : 0, 20917
Remote Config [baseline] (668.196 µs) : 0, 668
Remote Config [candidate] (679.741 µs) : 0, 680
Telemetry [baseline] (8.584 ms) : 0, 8584
Telemetry [candidate] (8.696 ms) : 0, 8696
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.068 s) : 0, 1067857
Total [baseline] (10.626 s) : 0, 10625789
Agent [candidate] (1.055 s) : 0, 1054605
Total [candidate] (10.435 s) : 0, 10435009
section appsec
Agent [baseline] (1.187 s) : 0, 1186807
Total [baseline] (10.731 s) : 0, 10731266
Agent [candidate] (1.19 s) : 0, 1190416
Total [candidate] (10.718 s) : 0, 10717949
section iast
Agent [baseline] (1.197 s) : 0, 1196661
Total [baseline] (11.003 s) : 0, 11003404
Agent [candidate] (1.191 s) : 0, 1190559
Total [candidate] (11.014 s) : 0, 11014071
section profiling
Agent [baseline] (1.255 s) : 0, 1255476
Total [baseline] (10.832 s) : 0, 10832349
Agent [candidate] (1.259 s) : 0, 1258748
Total [candidate] (10.82 s) : 0, 10820480
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.068 s -
Agent appsec 1.187 s 118.95 ms (11.1%)
Agent iast 1.197 s 128.804 ms (12.1%)
Agent profiling 1.255 s 187.619 ms (17.6%)
Total tracing 10.626 s -
Total appsec 10.731 s 105.476 ms (1.0%)
Total iast 11.003 s 377.615 ms (3.6%)
Total profiling 10.832 s 206.56 ms (1.9%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.055 s -
Agent appsec 1.19 s 135.811 ms (12.9%)
Agent iast 1.191 s 135.954 ms (12.9%)
Agent profiling 1.259 s 204.143 ms (19.4%)
Total tracing 10.435 s -
Total appsec 10.718 s 282.94 ms (2.7%)
Total iast 11.014 s 579.063 ms (5.5%)
Total profiling 10.82 s 385.471 ms (3.7%) 
gantt
    title petclinic - break down per module: candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (723.168 ms) : 0, 723168
BytebuddyAgent [candidate] (713.553 ms) : 0, 713553
GlobalTracer [baseline] (257.995 ms) : 0, 257995
GlobalTracer [candidate] (255.844 ms) : 0, 255844
AppSec [baseline] (55.68 ms) : 0, 55680
AppSec [candidate] (55.232 ms) : 0, 55232
Remote Config [baseline] (728.93 µs) : 0, 729
Remote Config [candidate] (725.709 µs) : 0, 726
Telemetry [baseline] (15.137 ms) : 0, 15137
Telemetry [candidate] (14.224 ms) : 0, 14224
section appsec
BytebuddyAgent [baseline] (729.988 ms) : 0, 729988
BytebuddyAgent [candidate] (732.122 ms) : 0, 732122
GlobalTracer [baseline] (252.704 ms) : 0, 252704
GlobalTracer [candidate] (253.387 ms) : 0, 253387
AppSec [baseline] (170.298 ms) : 0, 170298
AppSec [candidate] (171.057 ms) : 0, 171057
IAST [baseline] (19.349 ms) : 0, 19349
IAST [candidate] (19.614 ms) : 0, 19614
Remote Config [baseline] (669.009 µs) : 0, 669
Remote Config [candidate] (672.595 µs) : 0, 673
Telemetry [baseline] (8.567 ms) : 0, 8567
Telemetry [candidate] (8.283 ms) : 0, 8283
section iast
BytebuddyAgent [baseline] (843.349 ms) : 0, 843349
BytebuddyAgent [candidate] (837.701 ms) : 0, 837701
GlobalTracer [baseline] (248.64 ms) : 0, 248640
GlobalTracer [candidate] (248.162 ms) : 0, 248162
AppSec [baseline] (58.226 ms) : 0, 58226
AppSec [candidate] (58.294 ms) : 0, 58294
IAST [baseline] (21.655 ms) : 0, 21655
IAST [candidate] (21.49 ms) : 0, 21490
Remote Config [baseline] (671.015 µs) : 0, 671
Remote Config [candidate] (676.398 µs) : 0, 676
Telemetry [baseline] (8.877 ms) : 0, 8877
Telemetry [candidate] (8.886 ms) : 0, 8886
section profiling
BytebuddyAgent [baseline] (703.186 ms) : 0, 703186
BytebuddyAgent [candidate] (705.556 ms) : 0, 705556
GlobalTracer [baseline] (349.931 ms) : 0, 349931
GlobalTracer [candidate] (351.677 ms) : 0, 351677
AppSec [baseline] (54.755 ms) : 0, 54755
AppSec [candidate] (53.99 ms) : 0, 53990
Remote Config [baseline] (658.881 µs) : 0, 659
Remote Config [candidate] (663.146 µs) : 0, 663
Telemetry [baseline] (8.88 ms) : 0, 8880
Telemetry [candidate] (8.869 ms) : 0, 8869
ProfilingAgent [baseline] (95.973 ms) : 0, 95973
ProfilingAgent [candidate] (95.969 ms) : 0, 95969
Profiling [baseline] (95.998 ms) : 0, 95998
Profiling [candidate] (95.994 ms) : 0, 95994
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2025-01-15T11:14:03 2025-01-15T11:21:04
git_branch master vandonr/proto
git_commit_date 1736890492 1736938770
git_commit_sha 8b63e5a d702a13
release_version 1.46.0-SNAPSHOT~8b63e5a85b 1.46.0-SNAPSHOT~d702a13e68
start_time 2025-01-15T11:13:49 2025-01-15T11:20:51
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1736940417 1736940417
ci_job_id 767038700 767038700
ci_pipeline_id 53054901 53054901
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 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 16 unstable metrics.

scenario Δ mean http_req_duration Δ mean throughput candidate mean http_req_duration candidate mean throughput baseline mean http_req_duration baseline mean throughput
scenario:load:petclinic:profiling better
[-94.857µs; -43.452µs] or [-6.059%; -2.776%]		 unstable
[-534.663op/s; +534.663op/s] or [-17.377%; +17.377%]		 1.496ms 3076.923op/s 1.566ms 3076.923op/s
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b
    dateFormat X
    axisFormat %s
section baseline
no_agent (382.414 µs) : 361, 404
.   : milestone, 382,
iast (499.102 µs) : 477, 521
.   : milestone, 499,
iast_FULL (662.328 µs) : 641, 684
.   : milestone, 662,
iast_GLOBAL (517.885 µs) : 496, 539
.   : milestone, 518,
iast_HARDCODED_SECRET_DISABLED (502.426 µs) : 481, 524
.   : milestone, 502,
iast_INACTIVE (453.792 µs) : 433, 475
.   : milestone, 454,
iast_TELEMETRY_OFF (484.824 µs) : 463, 506
.   : milestone, 485,
tracing (453.682 µs) : 433, 475
.   : milestone, 454,
section candidate
no_agent (379.713 µs) : 360, 400
.   : milestone, 380,
iast (497.694 µs) : 476, 519
.   : milestone, 498,
iast_FULL (655.235 µs) : 634, 677
.   : milestone, 655,
iast_GLOBAL (528.697 µs) : 506, 552
.   : milestone, 529,
iast_HARDCODED_SECRET_DISABLED (494.784 µs) : 473, 516
.   : milestone, 495,
iast_INACTIVE (453.812 µs) : 432, 475
.   : milestone, 454,
iast_TELEMETRY_OFF (485.49 µs) : 464, 507
.   : milestone, 485,
tracing (457.438 µs) : 437, 478
.   : milestone, 457,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 382.414 µs [360.743 µs, 404.086 µs] -
iast 499.102 µs [477.485 µs, 520.719 µs] 116.688 µs (30.5%)
iast_FULL 662.328 µs [640.543 µs, 684.113 µs] 279.914 µs (73.2%)
iast_GLOBAL 517.885 µs [496.497 µs, 539.273 µs] 135.471 µs (35.4%)
iast_HARDCODED_SECRET_DISABLED 502.426 µs [480.69 µs, 524.162 µs] 120.012 µs (31.4%)
iast_INACTIVE 453.792 µs [432.871 µs, 474.712 µs] 71.377 µs (18.7%)
iast_TELEMETRY_OFF 484.824 µs [463.368 µs, 506.279 µs] 102.41 µs (26.8%)
tracing 453.682 µs [432.808 µs, 474.556 µs] 71.268 µs (18.6%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 379.713 µs [359.798 µs, 399.627 µs] -
iast 497.694 µs [476.05 µs, 519.338 µs] 117.981 µs (31.1%)
iast_FULL 655.235 µs [633.713 µs, 676.757 µs] 275.522 µs (72.6%)
iast_GLOBAL 528.697 µs [505.808 µs, 551.585 µs] 148.984 µs (39.2%)
iast_HARDCODED_SECRET_DISABLED 494.784 µs [473.082 µs, 516.485 µs] 115.071 µs (30.3%)
iast_INACTIVE 453.812 µs [432.447 µs, 475.177 µs] 74.1 µs (19.5%)
iast_TELEMETRY_OFF 485.49 µs [464.069 µs, 506.912 µs] 105.778 µs (27.9%)
tracing 457.438 µs [436.542 µs, 478.334 µs] 77.725 µs (20.5%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~d702a13e68, baseline=1.46.0-SNAPSHOT~8b63e5a85b
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.343 ms) : 1323, 1364
.   : milestone, 1343,
appsec (1.764 ms) : 1740, 1787
.   : milestone, 1764,
appsec_no_iast (1.756 ms) : 1731, 1782
.   : milestone, 1756,
iast (1.499 ms) : 1476, 1522
.   : milestone, 1499,
profiling (1.566 ms) : 1541, 1590
.   : milestone, 1566,
tracing (1.475 ms) : 1450, 1500
.   : milestone, 1475,
section candidate
no_agent (1.328 ms) : 1309, 1348
.   : milestone, 1328,
appsec (1.757 ms) : 1734, 1780
.   : milestone, 1757,
appsec_no_iast (1.76 ms) : 1737, 1783
.   : milestone, 1760,
iast (1.496 ms) : 1473, 1519
.   : milestone, 1496,
profiling (1.496 ms) : 1473, 1520
.   : milestone, 1496,
tracing (1.472 ms) : 1447, 1497
.   : milestone, 1472,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.343 ms [1.323 ms, 1.364 ms] -
appsec 1.764 ms [1.74 ms, 1.787 ms] 420.393 µs (31.3%)
appsec_no_iast 1.756 ms [1.731 ms, 1.782 ms] 412.92 µs (30.7%)
iast 1.499 ms [1.476 ms, 1.522 ms] 155.98 µs (11.6%)
profiling 1.566 ms [1.541 ms, 1.59 ms] 222.227 µs (16.5%)
tracing 1.475 ms [1.45 ms, 1.5 ms] 131.513 µs (9.8%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.328 ms [1.309 ms, 1.348 ms] -
appsec 1.757 ms [1.734 ms, 1.78 ms] 428.709 µs (32.3%)
appsec_no_iast 1.76 ms [1.737 ms, 1.783 ms] 431.608 µs (32.5%)
iast 1.496 ms [1.473 ms, 1.519 ms] 167.631 µs (12.6%)
profiling 1.496 ms [1.473 ms, 1.52 ms] 167.887 µs (12.6%)
tracing 1.472 ms [1.447 ms, 1.497 ms] 143.304 µs (10.8%)

Dacapo

@vandonr vandonr changed the title Change hash computation for protobuf to better represent impacting changes Change hash computation for protobuf to better represent impacting changes + save proto number in schema Jan 15, 2025
@vandonr vandonr added comp: api Tracer public API and removed comp: api Tracer public API labels Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piochelepiotr piochelepiotr piochelepiotr approved these changes

@amarziali amarziali amarziali approved these changes

@smola smola Awaiting requested review from smola smola is a code owner automatically assigned from DataDog/apm-java

Assignees
No one assigned
Labels
comp: data streams Data Streams Monitoring inst: protobuf Protocol Buffer instrumentation type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vandonr @amarziali @piochelepiotr