Skip to content

fix (internal/civisibility): fix code coverage stdout data race#3674

Merged
tonyredondo merged 2 commits into
mainfrom
tony/civisibility-fix-codecoverage-stdout-datarace
Jun 20, 2025
Merged

fix (internal/civisibility): fix code coverage stdout data race#3674
tonyredondo merged 2 commits into
mainfrom
tony/civisibility-fix-codecoverage-stdout-datarace

Conversation

@tonyredondo

@tonyredondo tonyredondo commented Jun 20, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Removes the stdout replacement while extracting the coverage of a test due to a possible race condition with any function writing to stdout at the same time.

Motivation

WARNING: DATA RACE
Read at 0x000002334aa8 by goroutine 54:
  fmt.Printf()
      /opt/hostedtoolcache/go/1.23.10/x64/src/fmt/print.go:233 +0x145
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.setUpHTTPServer.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testcontroller_test.go:995 +0xa8
  net/http.HandlerFunc.ServeHTTP()
      /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2220 +0x47
  net/http.serverHandler.ServeHTTP()
      /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3210 +0x2a1
  net/http.(*conn).serve()
      /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:2092 +0x12a4
  net/http.(*Server).Serve.gowrap3()
      /opt/hostedtoolcache/go/1.23.10/x64/src/net/http/server.go:3360 +0x4f

Previous write at 0x000002334aa8 by main goroutine:
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting/coverage.InitializeCoverage.func1.1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/coverage/test_coverage.go:105 +0x30
  runtime.deferreturn()
      /opt/hostedtoolcache/go/1.23.10/x64/src/runtime/panic.go:611 +0x5d
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting/coverage.GetCoverage()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/coverage/test_coverage.go:160 +0x3b7
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.instrumentTestingM.func2()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation_orchestrion.go:88 +0x69
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.(*M).Run()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:96 +0xa2
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.RunM()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:578 +0x485
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.runFlakyTestRetriesTests()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testcontroller_test.go:121 +0x426
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.TestMain()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testcontroller_test.go:47 +0x264
  main.main()
      _testmain.go:87 +0x171

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!

@tonyredondo
tonyredondo requested a review from a team as a code owner June 20, 2025 08:03
@pr-commenter

pr-commenter Bot commented Jun 20, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-06-20 08:34:20

Comparing candidate commit 790d024 in PR branch tony/civisibility-fix-codecoverage-stdout-datarace with baseline commit bd44033 in branch main.

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

@tonyredondo
tonyredondo enabled auto-merge (squash) June 20, 2025 08:16
@tonyredondo
tonyredondo merged commit 936d805 into main Jun 20, 2025
232 of 233 checks passed
@tonyredondo
tonyredondo deleted the tony/civisibility-fix-codecoverage-stdout-datarace branch June 20, 2025 08:39
genesor pushed a commit that referenced this pull request Jun 20, 2025
@tonyredondo @genesor
fix (internal/civisibility): fix code coverage stdout data race (#3674)
49cdd7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anmarchenko anmarchenko anmarchenko approved these changes
@kakkoyun kakkoyun kakkoyun approved these changes
@ManuelPalenzuelaDD ManuelPalenzuelaDD Awaiting requested review from ManuelPalenzuelaDD ManuelPalenzuelaDD is a code owner automatically assigned from DataDog/ci-app-libraries

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tonyredondo @anmarchenko @kakkoyun @darccio