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

Fix data race between kafkareceiver and batchprocessor (#2956) #2957

Merged
merged 1 commit into from
Apr 19, 2021
Merged

Fix data race between kafkareceiver and batchprocessor (#2956) #2957

merged 1 commit into from
Apr 19, 2021

Conversation

hanjm
Copy link
Member

@hanjm hanjm commented Apr 17, 2021

Description:
Fix datarace between kafkareceiver and batchprocessor, more description see issue #2956
kafkareceiver should not use traces varible after invoke nextConsumer.ConsumeTraces because traces object will be operated in batchprocessor in another goroutine.

==================
WARNING: DATA RACE
Write at 0x00c0001108d0 by goroutine 115:
  go.opentelemetry.io/collector/consumer/pdata.ResourceSpansSlice.MoveAndAppendTo()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/consumer/pdata/generated_trace.go:75 +0x237
  go.opentelemetry.io/collector/processor/batchprocessor.(*batchTraces).add()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/processor/batchprocessor/batch_processor.go:269 +0xe5
  go.opentelemetry.io/collector/processor/batchprocessor.(*batchProcessor).processItem()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/processor/batchprocessor/batch_processor.go:187 +0x138
  go.opentelemetry.io/collector/processor/batchprocessor.(*batchProcessor).startProcessingCycle()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/processor/batchprocessor/batch_processor.go:143 +0x284

Previous read at 0x00c0001108d0 by goroutine 123:
  go.opentelemetry.io/collector/consumer/pdata.ResourceSpansSlice.Len()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/consumer/pdata/generated_trace.go:52 +0x113
  go.opentelemetry.io/collector/consumer/pdata.Traces.SpanCount()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/consumer/pdata/trace.go:79 +0x3a
  go.opentelemetry.io/collector/receiver/kafkareceiver.(*consumerGroupHandler).ConsumeClaim()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/receiver/kafkareceiver/kafka_receiver.go:175 +0xd0b
  github.com/Shopify/sarama.(*consumerGroupSession).consume()
      /Users/jimmiehan/go/pkg/mod/github.com/!shopify/[email protected]/consumer_group.go:690 +0x3a8
  github.com/Shopify/sarama.newConsumerGroupSession.func2()
      /Users/jimmiehan/go/pkg/mod/github.com/!shopify/[email protected]/consumer_group.go:615 +0x104

Goroutine 115 (running) created at:
  go.opentelemetry.io/collector/processor/batchprocessor.(*batchProcessor).Start()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/processor/batchprocessor/batch_processor.go:103 +0x4c
  go.opentelemetry.io/collector/service/internal/builder.BuiltPipelines.StartProcessors()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/service/internal/builder/pipelines_builder.go:58 +0x154
  go.opentelemetry.io/collector/service.(*service).startPipelines()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/service/service.go:206 +0x264
  go.opentelemetry.io/collector/service.(*service).Start()
      /Users/jimmiehan/github.com/hanjm/opentelemetry-collector/service/service.go:90 +0x169

Link to tracking Issue:
#2956

Testing:
run unittest and go run with --race

@codecov
Copy link

codecov bot commented Apr 17, 2021

Codecov Report

Merging #2957 (b000971) into main (90b2dec) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2957   +/-   ##
=======================================
  Coverage   91.58%   91.58%           
=======================================
  Files         312      312           
  Lines       15351    15352    +1     
=======================================
+ Hits        14059    14060    +1     
  Misses        883      883           
  Partials      409      409           
Impacted Files Coverage Δ
receiver/kafkareceiver/kafka_receiver.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90b2dec...b000971. Read the comment docs.

@bogdandrutu bogdandrutu merged commit 4e1fd23 into open-telemetry:main Apr 19, 2021
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…en-telemetry#2957)

* Add ability to run isolated collectd subprocess

* sort logrus fields for zap consistency

* tests: add collectd-mysql and isolated logging coverage
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.

3 participants