Skip to content

Do not modify readonly message attributes map on SNS integration#7150

Merged
vandonr merged 3 commits into
masterfrom
vandonr/fix
Jun 11, 2024
Merged

Do not modify readonly message attributes map on SNS integration#7150
vandonr merged 3 commits into
masterfrom
vandonr/fix

Conversation

@vandonr
Copy link
Copy Markdown
Contributor

@vandonr vandonr commented Jun 10, 2024

What Does This Do

Fix a bug where the integration would crash in the sdk v1 integration if the message attributes were a readonly map
Also fix a minor bug in the sdk v2 batch requests where we'd write more than 10 message attributes, which is not supported in SQS.

Motivation

#7147

@vandonr vandonr requested a review from a team as a code owner June 10, 2024 15:19
@amarziali amarziali changed the title bugfixe(s) on SNS integration SNS integration: do not modify readonly message attributes map Jun 10, 2024
@amarziali amarziali added inst: aws sdk AWS SDK instrumentation tag: serverless Serverless support type: bug Bug report and fix labels Jun 10, 2024
@vandonr vandonr removed the inst: aws sdk AWS SDK instrumentation label Jun 10, 2024
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Jun 10, 2024

Benchmarks

Startup

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-06-10T15:39:13 2024-06-10T15:46:02
git_branch master vandonr/fix
git_commit_date 1718022484 1718032172
git_commit_sha b7b9e69 9579b09
release_version 1.36.0-SNAPSHOT~b7b9e6991e 1.36.0-SNAPSHOT~9579b09cf4
start_time 2024-06-10T15:38:59 2024-06-10T15:45:49
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1718034707 1718034707
ci_job_id 537835801 537835801
ci_pipeline_id 36294436 36294436
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 11 metrics, 17 unstable metrics.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~9579b09cf4, baseline=1.36.0-SNAPSHOT~b7b9e6991e
    dateFormat X
    axisFormat %s
section baseline
no_agent (371.317 µs) : 352, 391
.   : milestone, 371,
iast (484.659 µs) : 463, 506
.   : milestone, 485,
iast_FULL (550.414 µs) : 529, 572
.   : milestone, 550,
iast_GLOBAL (504.762 µs) : 484, 526
.   : milestone, 505,
iast_HARDCODED_SECRET_DISABLED (480.144 µs) : 459, 502
.   : milestone, 480,
iast_INACTIVE (453.204 µs) : 432, 474
.   : milestone, 453,
iast_TELEMETRY_OFF (474.661 µs) : 453, 496
.   : milestone, 475,
tracing (433.721 µs) : 414, 454
.   : milestone, 434,
section candidate
no_agent (366.993 µs) : 347, 387
.   : milestone, 367,
iast (480.601 µs) : 460, 502
.   : milestone, 481,
iast_FULL (553.533 µs) : 532, 575
.   : milestone, 554,
iast_GLOBAL (518.048 µs) : 495, 541
.   : milestone, 518,
iast_HARDCODED_SECRET_DISABLED (477.713 µs) : 457, 499
.   : milestone, 478,
iast_INACTIVE (450.386 µs) : 429, 472
.   : milestone, 450,
iast_TELEMETRY_OFF (469.593 µs) : 448, 491
.   : milestone, 470,
tracing (443.738 µs) : 423, 465
.   : milestone, 444,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 371.317 µs [351.642 µs, 390.991 µs] -
iast 484.659 µs [463.009 µs, 506.309 µs] 113.343 µs (30.5%)
iast_FULL 550.414 µs [529.138 µs, 571.69 µs] 179.097 µs (48.2%)
iast_GLOBAL 504.762 µs [483.851 µs, 525.672 µs] 133.445 µs (35.9%)
iast_HARDCODED_SECRET_DISABLED 480.144 µs [458.596 µs, 501.693 µs] 108.828 µs (29.3%)
iast_INACTIVE 453.204 µs [432.122 µs, 474.287 µs] 81.888 µs (22.1%)
iast_TELEMETRY_OFF 474.661 µs [453.111 µs, 496.21 µs] 103.344 µs (27.8%)
tracing 433.721 µs [413.615 µs, 453.827 µs] 62.405 µs (16.8%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 366.993 µs [347.053 µs, 386.933 µs] -
iast 480.601 µs [459.553 µs, 501.648 µs] 113.608 µs (31.0%)
iast_FULL 553.533 µs [532.302 µs, 574.763 µs] 186.54 µs (50.8%)
iast_GLOBAL 518.048 µs [495.316 µs, 540.78 µs] 151.055 µs (41.2%)
iast_HARDCODED_SECRET_DISABLED 477.713 µs [456.717 µs, 498.709 µs] 110.72 µs (30.2%)
iast_INACTIVE 450.386 µs [428.864 µs, 471.908 µs] 83.393 µs (22.7%)
iast_TELEMETRY_OFF 469.593 µs [448.316 µs, 490.869 µs] 102.6 µs (28.0%)
tracing 443.738 µs [422.563 µs, 464.914 µs] 76.745 µs (20.9%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~9579b09cf4, baseline=1.36.0-SNAPSHOT~b7b9e6991e
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.341 ms) : 1321, 1360
.   : milestone, 1341,
appsec (1.715 ms) : 1691, 1738
.   : milestone, 1715,
appsec_no_iast (1.722 ms) : 1699, 1746
.   : milestone, 1722,
iast (1.47 ms) : 1447, 1493
.   : milestone, 1470,
profiling (1.505 ms) : 1479, 1531
.   : milestone, 1505,
tracing (1.476 ms) : 1452, 1500
.   : milestone, 1476,
section candidate
no_agent (1.359 ms) : 1340, 1378
.   : milestone, 1359,
appsec (1.724 ms) : 1700, 1748
.   : milestone, 1724,
appsec_no_iast (1.721 ms) : 1697, 1745
.   : milestone, 1721,
iast (1.46 ms) : 1437, 1482
.   : milestone, 1460,
profiling (1.513 ms) : 1486, 1539
.   : milestone, 1513,
tracing (1.437 ms) : 1413, 1462
.   : milestone, 1437,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.341 ms [1.321 ms, 1.36 ms] -
appsec 1.715 ms [1.691 ms, 1.738 ms] 373.881 µs (27.9%)
appsec_no_iast 1.722 ms [1.699 ms, 1.746 ms] 381.549 µs (28.5%)
iast 1.47 ms [1.447 ms, 1.493 ms] 129.759 µs (9.7%)
profiling 1.505 ms [1.479 ms, 1.531 ms] 164.361 µs (12.3%)
tracing 1.476 ms [1.452 ms, 1.5 ms] 135.577 µs (10.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.359 ms [1.34 ms, 1.378 ms] -
appsec 1.724 ms [1.7 ms, 1.748 ms] 365.236 µs (26.9%)
appsec_no_iast 1.721 ms [1.697 ms, 1.745 ms] 361.984 µs (26.6%)
iast 1.46 ms [1.437 ms, 1.482 ms] 100.706 µs (7.4%)
profiling 1.513 ms [1.486 ms, 1.539 ms] 153.508 µs (11.3%)
tracing 1.437 ms [1.413 ms, 1.462 ms] 78.005 µs (5.7%)

Dacapo

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master vandonr/fix
git_commit_date 1718022484 1718032172
git_commit_sha b7b9e69 9579b09
release_version 1.36.0-SNAPSHOT~b7b9e6991e 1.36.0-SNAPSHOT~9579b09cf4
See matching parameters
Baseline Candidate
application biojava biojava
ci_job_date 1718035320 1718035320
ci_job_id 537835802 537835802
ci_pipeline_id 36294436 36294436
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant appsec appsec

Summary

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

Execution time for biojava 
gantt
    title biojava - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~9579b09cf4, baseline=1.36.0-SNAPSHOT~b7b9e6991e
    dateFormat X
    axisFormat %s
section baseline
no_agent (15.695 s) : 15695000, 15695000
.   : milestone, 15695000,
appsec (15.102 s) : 15102000, 15102000
.   : milestone, 15102000,
iast (18.719 s) : 18719000, 18719000
.   : milestone, 18719000,
iast_GLOBAL (18.047 s) : 18047000, 18047000
.   : milestone, 18047000,
profiling (16.045 s) : 16045000, 16045000
.   : milestone, 16045000,
tracing (15.074 s) : 15074000, 15074000
.   : milestone, 15074000,
section candidate
no_agent (15.38 s) : 15380000, 15380000
.   : milestone, 15380000,
appsec (14.869 s) : 14869000, 14869000
.   : milestone, 14869000,
iast (18.815 s) : 18815000, 18815000
.   : milestone, 18815000,
iast_GLOBAL (18.017 s) : 18017000, 18017000
.   : milestone, 18017000,
profiling (15.212 s) : 15212000, 15212000
.   : milestone, 15212000,
tracing (15.05 s) : 15050000, 15050000
.   : milestone, 15050000,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.695 s [15.695 s, 15.695 s] -
appsec 15.102 s [15.102 s, 15.102 s] -593.0 ms (-3.8%)
iast 18.719 s [18.719 s, 18.719 s] 3.024 s (19.3%)
iast_GLOBAL 18.047 s [18.047 s, 18.047 s] 2.352 s (15.0%)
profiling 16.045 s [16.045 s, 16.045 s] 350.0 ms (2.2%)
tracing 15.074 s [15.074 s, 15.074 s] -621.0 ms (-4.0%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.38 s [15.38 s, 15.38 s] -
appsec 14.869 s [14.869 s, 14.869 s] -511.0 ms (-3.3%)
iast 18.815 s [18.815 s, 18.815 s] 3.435 s (22.3%)
iast_GLOBAL 18.017 s [18.017 s, 18.017 s] 2.637 s (17.1%)
profiling 15.212 s [15.212 s, 15.212 s] -168.0 ms (-1.1%)
tracing 15.05 s [15.05 s, 15.05 s] -330.0 ms (-2.1%)
Execution time for tomcat 
gantt
    title tomcat - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~9579b09cf4, baseline=1.36.0-SNAPSHOT~b7b9e6991e
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.461 ms) : 1450, 1473
.   : milestone, 1461,
appsec (2.211 ms) : 2177, 2245
.   : milestone, 2211,
iast (1.979 ms) : 1938, 2021
.   : milestone, 1979,
iast_GLOBAL (2.012 ms) : 1971, 2054
.   : milestone, 2012,
profiling (1.854 ms) : 1820, 1887
.   : milestone, 1854,
tracing (1.845 ms) : 1812, 1878
.   : milestone, 1845,
section candidate
no_agent (1.458 ms) : 1446, 1469
.   : milestone, 1458,
appsec (2.199 ms) : 2165, 2233
.   : milestone, 2199,
iast (1.969 ms) : 1927, 2010
.   : milestone, 1969,
iast_GLOBAL (2.014 ms) : 1973, 2056
.   : milestone, 2014,
profiling (1.857 ms) : 1823, 1891
.   : milestone, 1857,
tracing (1.838 ms) : 1806, 1871
.   : milestone, 1838,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.461 ms [1.45 ms, 1.473 ms] -
appsec 2.211 ms [2.177 ms, 2.245 ms] 749.096 µs (51.3%)
iast 1.979 ms [1.938 ms, 2.021 ms] 517.821 µs (35.4%)
iast_GLOBAL 2.012 ms [1.971 ms, 2.054 ms] 550.874 µs (37.7%)
profiling 1.854 ms [1.82 ms, 1.887 ms] 392.136 µs (26.8%)
tracing 1.845 ms [1.812 ms, 1.878 ms] 383.403 µs (26.2%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.458 ms [1.446 ms, 1.469 ms] -
appsec 2.199 ms [2.165 ms, 2.233 ms] 741.152 µs (50.8%)
iast 1.969 ms [1.927 ms, 2.01 ms] 510.979 µs (35.1%)
iast_GLOBAL 2.014 ms [1.973 ms, 2.056 ms] 556.725 µs (38.2%)
profiling 1.857 ms [1.823 ms, 1.891 ms] 399.06 µs (27.4%)
tracing 1.838 ms [1.806 ms, 1.871 ms] 380.559 µs (26.1%)

@amarziali amarziali added the inst: aws sdk AWS SDK instrumentation label Jun 11, 2024
@vandonr vandonr merged commit 7db8941 into master Jun 11, 2024
@vandonr vandonr deleted the vandonr/fix branch June 11, 2024 10:39
@github-actions github-actions Bot added this to the 1.36.0 milestone Jun 11, 2024
@mcculls mcculls mentioned this pull request Jun 12, 2024
Merged
mcculls pushed a commit that referenced this pull request Jun 12, 2024
@vandonr @mcculls
SNS integration: do not modify readonly message attributes map (#7150)
869afab 
* fix bug when SNS message attr are readonly + test

* fix extra bug in v2 on message attr size
@PerfectSlayer PerfectSlayer changed the title SNS integration: do not modify readonly message attributes map Do not modify readonly message attributes map on SNS integration Jun 27, 2024
@ygree ygree mentioned this pull request Apr 22, 2026
Merged
@ygree
Copy link
Copy Markdown
Contributor

ygree commented Apr 23, 2026

@codex review

@vandonr
Copy link
Copy Markdown
Contributor Author

vandonr commented Apr 27, 2026 •
edited
Loading

@codex review

did you intend to call for a review on your PR ? Or was it to see if codex would have caught the bug you fixed ?

@chatgpt-codex-connector
Copy link
Copy Markdown

chatgpt-codex-connector Bot commented Apr 27, 2026

Good catch — that @codex review was intended to trigger an automated second-pass review, not to explicitly request a human review from maintainers.

So to answer directly: it was mainly to see whether Codex would catch issues (including the one later fixed), not to signal “ready for human review” by itself.

View task →

gh-worker-dd-mergequeue-cf854d Bot pushed a commit that referenced this pull request Apr 27, 2026
@ygree
fix: SNS v2 batch injection bug (#11190)
cde8f35 
# What Does This Do

Fixes SNS v2 batch injection bug that causes a batch entry to be dropped when the number of attributes exceeded limits.

+ adds a test that reproduces the bug
+ migrates one existing test from Spock to JUnit as it fits more the purpose
+ cleans up some dups 

# Motivation

Fix the bug affecting the application's logic due to an instrumentation flaw.

# Additional Notes

Ref: #7150

# Contributor Checklist

- Format the title according to [the contribution guidelines](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#title-format)
- Assign the `type:` and (`comp:` or `inst:`) labels in addition to [any other useful labels](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#labels)
- Avoid using `close`, `fix`, or [any linking keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) when referencing an issue  
  Use `solves` instead, and assign the PR [milestone](https://github.com/DataDog/dd-trace-java/milestones) to the issue
- Update the [CODEOWNERS](https://github.com/DataDog/dd-trace-java/blob/master/.github/CODEOWNERS) file on source file addition, migration, or deletion
- Update [public documentation](https://docs.datadoghq.com/tracing/trace_collection/library_config/java/) with any new configuration flags or behaviors

Jira ticket: [APMS-19177]

***Note:*** **Once your PR is ready to merge, add it to the merge queue by commenting `/merge`.** `/merge -c` cancels the queue request. `/merge -f --reason "reason"` skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see [this doc](https://datadoghq.atlassian.net/wiki/spaces/DEVX/pages/3121612126/MergeQueue).

<!--
# Opening vs Drafting a PR:
When opening a pull request, please open it as a draft to not auto assign reviewers before you feel the pull request is in a reviewable state.

# Linking a JIRA ticket:
Please link your JIRA ticket by adding its identifier between brackets (ex [PROJ-IDENT]) in the PR description, not the title.
This requirement only applies to Datadog employees.
-->



[APMS-19177]: https://datadoghq.atlassian.net/browse/APMS-19177?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: yury.gribkov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcculls mcculls mcculls approved these changes

@amarziali amarziali amarziali approved these changes

Assignees

No one assigned

Labels

inst: aws sdk AWS SDK instrumentation tag: serverless Serverless support type: bug Bug report and fix

Projects

None yet

Milestone

1.36.0

Development

Successfully merging this pull request may close these issues.

4 participants

@vandonr @ygree @mcculls @amarziali