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

remove locking from jaeger receiver start and stop processes #3070

Merged

Conversation

bhautikpip
Copy link
Contributor

Description: <Describe what has changed.
Remove locking from start and stop functions - jaeger receiver

Link to tracking Issue:
#3041

Testing: < Describe what testing was performed and which tests were added.>

Documentation: < Describe the documentation added.>

Please delete paragraphs that you did not use before submitting.

@bhautikpip bhautikpip requested a review from a team as a code owner April 30, 2021 16:33
@bogdandrutu
Copy link
Member

@bhautikpip have you confirmed that this is not needed?

@bhautikpip
Copy link
Contributor Author

@bogdandrutu I have not tested this changes with multiple requests to see wether we would be requiring locks or not. I kinda followed the issue description. Any recommendation on best way to test this?

@jrcamp
Copy link
Contributor

jrcamp commented May 4, 2021

I think the field mu sync.Mutex can be removed as well. Would run the tests with -race. The locks are taken during Start and Shutdown which shouldn't happen concurrently. Not sure of any other good way to test.

@bhautikpip
Copy link
Contributor Author

Ran jaegerreceiver package tests using go test -race go.opentelemetry.io/collector/receiver/jaegerreceiver command and could not find any data races warning.

@jrcamp
Copy link
Contributor

jrcamp commented May 4, 2021

@jpkrohling

@bogdandrutu bogdandrutu merged commit 7316e6b into open-telemetry:main May 5, 2021
dashpole pushed a commit to dashpole/opentelemetry-collector that referenced this pull request Jun 14, 2021
…lemetry#3070)

* remove locking from jaeger receiver start and stop processes

* remove mutex field
ankitnayan pushed a commit to SigNoz/opentelemetry-collector that referenced this pull request Jul 27, 2021
…lemetry#3070)

* remove locking from jaeger receiver start and stop processes

* remove mutex field
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.

None yet

4 participants