Skip to content

tracer: fix potential concurrency bug for trace source tag#3442

Merged
genesor merged 1 commit into
mainfrom
ben.db/APPSEC-57301-fix-ts-concurrency
Apr 16, 2025
Merged

tracer: fix potential concurrency bug for trace source tag#3442
genesor merged 1 commit into
mainfrom
ben.db/APPSEC-57301-fix-ts-concurrency

Conversation

@genesor

@genesor genesor commented Apr 16, 2025

Copy link
Copy Markdown
Member

What does this PR do?

This PR fixes a potential concurrency bug introduced in #3230, the tag setting logic is using the read lock instead of write.

Motivation

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running golangci-lint run locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@genesor
genesor requested a review from a team as a code owner April 16, 2025 08:02
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Apr 16, 2025 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: ben.db/APPSEC-57301-fix-ts-concurrency
Commit report: e1c178c
Test service: dd-trace-go

✅ 0 Failed, 4478 Passed, 64 Skipped, 3m 57.51s Total Time

@pr-commenter

pr-commenter Bot commented Apr 16, 2025

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-04-16 08:28:41

Comparing candidate commit 401cfd7 in PR branch ben.db/APPSEC-57301-fix-ts-concurrency with baseline commit 23a4bfe in branch main.

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

scenario:BenchmarkInjectW3C-24

  • 🟥 execution_time [+158.408ns; +184.792ns] or [+3.916%; +4.568%]

@genesor

genesor commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

/merge

@genesor

genesor commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

/remove

@DataDog DataDog deleted a comment from dd-devflow Bot Apr 16, 2025
@genesor
genesor merged commit a284cf8 into main Apr 16, 2025
@genesor
genesor deleted the ben.db/APPSEC-57301-fix-ts-concurrency branch April 16, 2025 08:47
@DataDog DataDog deleted a comment from dd-devflow Bot Apr 16, 2025
darccio pushed a commit that referenced this pull request Apr 16, 2025
@genesor @darccio
tracer: fix potential concurrency bug for trace source tag (#3442)
3d0e78f
darccio pushed a commit that referenced this pull request Apr 25, 2025
@genesor @darccio
tracer: fix potential concurrency bug for trace source tag (#3442)
c7d610b
darccio pushed a commit that referenced this pull request Apr 25, 2025
@genesor @darccio
tracer: fix potential concurrency bug for trace source tag (#3442)
04c76a3
darccio pushed a commit that referenced this pull request Apr 29, 2025
@genesor @darccio
tracer: fix potential concurrency bug for trace source tag (#3442)
40ff71e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@darccio darccio darccio approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@genesor @darccio