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.

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

CBG-4140: Cheaply clone additionalFields when producing audit event - avoids un… #7041

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

bbrks
Copy link
Member

@bbrks bbrks commented Aug 5, 2024

CBG-4140

  • Clone additionalFields when producing audit event
    • Avoids unintentional mutation by callers if additionalFields is shared for multiple audit invocations (not a super realistic scenario, but came up in benchmarking)
    • Test coverage for read-only additionalFields
    • Bench for maps.Clone - it's very cheap even compared to a pre-allocated maps.Copy thanks to Go runtime optimisations:
BenchmarkAdditionalFieldsCopy/make_empty-10         	122409171	         9.697 ns/op
BenchmarkAdditionalFieldsCopy/make_and_copy_100-10  	 1936924	       599.6 ns/op
BenchmarkAdditionalFieldsCopy/clone_100-10          	36813062	        32.65 ns/op
  • Fix malformed warning message format for non-string types

Integration Tests

bbrks added 2 commits August 5, 2024 16:26
…intentional mutation by callers if additionalFields is shared for multiple audit invocations
@bbrks bbrks merged commit c0dfa68 into main Aug 6, 2024
38 checks passed
@bbrks bbrks deleted the CBG-4140 branch August 6, 2024 13:11
bbrks added a commit that referenced this pull request Aug 9, 2024
…roducing audit event - avoids un… (#7041)

* Cheaply clone additionalFields when producing audit event - avoids unintentional mutation by callers if additionalFields is shared for multiple audit invocations

* Fix warning format and add test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants