Skip to content

Fix data race when sending trace data to reporter.#245

Merged
wu-sheng merged 2 commits into
apache:mainfrom
mrproliu:segment-race
May 27, 2026
Merged

Fix data race when sending trace data to reporter.#245
wu-sheng merged 2 commits into
apache:mainfrom
mrproliu:segment-race

Conversation

@mrproliu
Copy link
Copy Markdown
Contributor

No description provided.

@mrproliu mrproliu added this to the 0.7.0 milestone May 27, 2026
@mrproliu mrproliu requested a review from Copilot May 27, 2026 06:53
@mrproliu mrproliu added the bug Something isn't working label May 27, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a data race / crash (apache/skywalking#13885) where the reporter goroutine marshalled a SegmentObject whose Tags/Logs slices were still being mutated by the originating goroutine (e.g. a GORM session reused across goroutines), faulting inside protobuf size computation. TransformSegmentObject now deep-copies tag and log entries so the queued protobuf message shares no mutable storage with the live span. Send/marshal paths in the gRPC and Kafka reporters are wrapped with per-message panic recovery as a defense-in-depth measure, and a new make test-race target runs new race-only regression tests in CI.

Changes:

  • Deep-copy Tags and Logs in Transform.TransformSegmentObject via new copyKeyStringValuePairs/copyLogs helpers.
  • Wrap each per-message proto.Marshal (Kafka) and stream.Send (gRPC) call in a recover helper so a single bad message doesn't tear down the send loop.
  • Add //go:build race regression tests under plugins/core, a make test-race target, and a CI step that invokes it.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
plugins/core/reporter/transform.go Adds deep-copy helpers and uses them for span Tags/Logs when building the protobuf segment.
plugins/core/reporter/grpc/grpc.go Adds sendWithRecover and applies it to tracing, metrics, log, and profile stream sends.
plugins/core/reporter/kafka/kafka.go Adds marshalWithRecover and applies it to tracing, metrics, and log marshal calls.
plugins/core/segment_datarace_test.go New race-only regression tests covering segment/reporter decoupling and panic-free reporting under concurrent span mutation.
Makefile New test-race target iterating non-./test/* modules running TestRace* under -race.
.github/workflows/skywalking-go.yaml Runs make test-race between make test and make lint.
CHANGES.md Notes the bug fix in the bug-fix section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wu-sheng wu-sheng merged commit 9a7385f into apache:main May 27, 2026
45 checks passed
@mrproliu mrproliu deleted the segment-race branch May 27, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants