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: avoid using internal empty attribute.Set pointer #34903

Merged
merged 7 commits into from
Sep 5, 2024

Conversation

rogercoll
Copy link
Contributor

Description:

Data race condition when using the internal empty attribute.Set concurrently. Code changed to prevent using the internal attribute.Set instance.

Link to tracking Issue: #34882

Testing:
Same tests, race condition failures should be fixed.

Documentation:

@rogercoll
Copy link
Contributor Author

Simplified example:

package main

import (
	"go.opentelemetry.io/otel/attribute"
	semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
)

func customSet(anotherSet attribute.Set) (attribute.Set, error) {
	allAttributes := attribute.EmptySet()
	*allAttributes = attribute.NewSet(append(allAttributes.ToSlice(), anotherSet.ToSlice()...)...)
	return *allAttributes, nil
}

func main() {
	ch := make(chan bool, 2)
	go func() {
		_, _ = customSet(attribute.NewSet([]attribute.KeyValue{
			semconv.SourceAddress("1.2.3.4"),
		}...))
		ch <- true
	}()
	go func() {
		_, _ = customSet(attribute.NewSet([]attribute.KeyValue{
			semconv.SourceAddress("1.2.3.4"),
		}...))
		ch <- true
	}()
	<-ch
	<-ch
}

Check the race condition:

$ go run -race -v main.go

To fix it, replace the allAttributes := attribute.EmptySet() inti with allAttributes := &attribute.Set{}

@andrzej-stencel andrzej-stencel merged commit fa75b6e into open-telemetry:main Sep 5, 2024
156 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 5, 2024
@rogercoll rogercoll deleted the fix_set_data_race branch September 5, 2024 13:55
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024
…#34903)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Data race condition when using the internal empty `attribute.Set`
concurrently. Code changed to prevent using the internal attribute.Set
instance.


**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#34882

**Testing:** <Describe what testing was performed and which tests were
added.>
Same tests, race condition failures should be fixed. 

**Documentation:** <Describe the documentation added.>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants