Skip to content

fix (internal/civisibility): fix p.serializationTime data race#3668

Merged
darccio merged 3 commits into
mainfrom
tony/civisibility-fix-datarace-on-serializationtime
Jun 20, 2025
Merged

fix (internal/civisibility): fix p.serializationTime data race#3668
darccio merged 3 commits into
mainfrom
tony/civisibility-fix-datarace-on-serializationtime

Conversation

@tonyredondo

@tonyredondo tonyredondo commented Jun 19, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Fixes: https://github.com/DataDog/dd-trace-go/actions/runs/15762178304/job/44431218001?pr=3657#step:5:417

==================
WARNING: DATA RACE
Read at 0x00c0000b8018 by goroutine 137:
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/logs.(*logsPayload).push.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs_payload.go:52 +0x75
  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/logs.(*logsWriter).add()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs_writer.go:49 +0x50
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/logs.WriteLog()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs.go:71 +0x5ca
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations.(*tslvTest).Log()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/manual_api_ddtest.go:363 +0x19b
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.collectAndWriteLogs()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:725 +0x671
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.(*M).executeInternalTest.func1.1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:298 +0x2f0
  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.executeTestIteration.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:584 +0xe4
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.executeTestIteration.gowrap2()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:585 +0x61

Previous write at 0x00c0000b8018 by goroutine 136:
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/logs.(*logsPayload).push.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs_payload.go:52 +0x92
  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/logs.(*logsWriter).add()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs_writer.go:49 +0x50
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/logs.WriteLog()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/logs/logs.go:71 +0x5ca
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations.(*tslvTest).Log()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/manual_api_ddtest.go:363 +0x19b
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.collectAndWriteLogs()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:725 +0x671
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.(*M).executeInternalTest.func1.1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:298 +0x2f0
  runtime.Goexit()
      /opt/hostedtoolcache/go/1.23.10/x64/src/runtime/panic.go:635 +0x5d
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.TestEarlyFlakeDetection()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing_test.go:142 +0x164
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.(*M).executeInternalTest.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/testing.go:364 +0xf3d
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.executeTestIteration.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:584 +0xe4
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.executeTestIteration.gowrap2()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:585 +0x61

Goroutine 137 (running) created at:
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.executeTestIteration()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:578 +0xceb
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.runTestWithRetry.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:482 +0x92
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.runTestWithRetry.gowrap1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:483 +0x41

Goroutine 136 (running) created at:
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.executeTestIteration()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:578 +0xceb
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.runTestWithRetry.func1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:482 +0x92
  github.com/DataDog/dd-trace-go/v2/internal/civisibility/integrations/gotesting.runTestWithRetry.gowrap1()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/civisibility/integrations/gotesting/instrumentation.go:483 +0x41
==================

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!

@tonyredondo
tonyredondo requested a review from a team as a code owner June 19, 2025 21:02
@tonyredondo
tonyredondo requested a review from BSanchidrian June 19, 2025 21:02
@pr-commenter

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

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-06-19 23:12:39

Comparing candidate commit 13c4e91 in PR branch tony/civisibility-fix-datarace-on-serializationtime with baseline commit 12ae038 in branch main.

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

@darccio
darccio merged commit 2bcfa37 into main Jun 20, 2025
219 checks passed
@darccio
darccio deleted the tony/civisibility-fix-datarace-on-serializationtime branch June 20, 2025 05:08
genesor pushed a commit that referenced this pull request Jun 20, 2025
@tonyredondo @genesor
fix (internal/civisibility): fix p.serializationTime data race (#3668)
b06dfea
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
@BSanchidrian BSanchidrian Awaiting requested review from BSanchidrian BSanchidrian 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.

2 participants

@tonyredondo @darccio