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

[internal/datadog] Fix Datarace in metrics client #31964

Conversation

dineshg13
Copy link
Member

@dineshg13 dineshg13 commented Mar 26, 2024

Description:

Fixes the data race.

fatal error: concurrent map read and map write

goroutine 72665 [running]:
github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog.(*metricsClient).Gauge.func1({0x7b895a0?, 0xc07c930790?}, {0x940dac8, 0xc1996ce180})
	github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog@v0.96.0/metrics_client.go:49 +0x85
go.opentelemetry.io/otel/sdk/metric.(*meter).float64ObservableInstrument.func1.1({0x9443f08, 0xef10b00})
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/meter.go:286 +0x55
go.opentelemetry.io/otel/sdk/metric.(*pipeline).produce(0xc002852bd0, {0x9443f08, 0xef10b00}, 0xc108dd46c0)
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/pipeline.go:122 +0x168
go.opentelemetry.io/otel/sdk/metric.(*ManualReader).Collect(0xc0030d2aa0, {0x9443f08, 0xef10b00}, 0xc108dd46c0)
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/manual_reader.go:123 +0xe2
go.opentelemetry.io/otel/exporters/prometheus.(*collector).Collect(0xc0030e4aa0, 0xc10a0083c0)
	go.opentelemetry.io/otel/exporters/prometheus@v0.46.0/exporter.go:158 +0x72
github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func1()
	github.com/prometheus/client_golang@v1.19.0/prometheus/registry.go:457 +0xe5
created by github.com/prometheus/client_golang/prometheus.(*Registry).Gather in goroutine 5

Link to tracking Issue:

Testing:

Documentation:

@github-actions github-actions bot requested a review from mx-psi March 26, 2024 09:56
@mx-psi mx-psi changed the title Fix Datarace in metrics client [internal/datadog] Fix Datarace in metrics client Mar 26, 2024
@dineshg13 dineshg13 marked this pull request as ready for review March 26, 2024 14:17
@dineshg13 dineshg13 requested a review from a team as a code owner March 26, 2024 14:17
@mx-psi mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 26, 2024
@mx-psi
Copy link
Member

mx-psi commented Mar 26, 2024

You will need to rebase this one after #31972 is merged

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Does this need a changelog? Seems users can run into this bug

@dineshg13
Copy link
Member Author

@songy23 I would prefer not to add changelog for a data race bug.

@mx-psi
Copy link
Member

mx-psi commented Mar 26, 2024

I agree with Yang that this would need a changelog, I mistakenly assumed this did not have user-facing implications

@mx-psi mx-psi removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 26, 2024
@mx-psi mx-psi added the ready to merge Code review completed; ready to merge by maintainers label Mar 26, 2024
@mx-psi mx-psi requested a review from songy23 March 26, 2024 16:48
Co-authored-by: Yang Song <songy23@users.noreply.github.com>
@mx-psi mx-psi merged commit f308c4c into open-telemetry:main Mar 26, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 26, 2024
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this pull request May 8, 2024
**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.-->
Fixes the data race.
```
fatal error: concurrent map read and map write

goroutine 72665 [running]:
github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog.(*metricsClient).Gauge.func1({0x7b895a0?, 0xc07c930790?}, {0x940dac8, 0xc1996ce180})
	github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog@v0.96.0/metrics_client.go:49 +0x85
go.opentelemetry.io/otel/sdk/metric.(*meter).float64ObservableInstrument.func1.1({0x9443f08, 0xef10b00})
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/meter.go:286 +0x55
go.opentelemetry.io/otel/sdk/metric.(*pipeline).produce(0xc002852bd0, {0x9443f08, 0xef10b00}, 0xc108dd46c0)
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/pipeline.go:122 +0x168
go.opentelemetry.io/otel/sdk/metric.(*ManualReader).Collect(0xc0030d2aa0, {0x9443f08, 0xef10b00}, 0xc108dd46c0)
	go.opentelemetry.io/otel/sdk/metric@v1.24.0/manual_reader.go:123 +0xe2
go.opentelemetry.io/otel/exporters/prometheus.(*collector).Collect(0xc0030e4aa0, 0xc10a0083c0)
	go.opentelemetry.io/otel/exporters/prometheus@v0.46.0/exporter.go:158 +0x72
github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func1()
	github.com/prometheus/client_golang@v1.19.0/prometheus/registry.go:457 +0xe5
created by github.com/prometheus/client_golang/prometheus.(*Registry).Gather in goroutine 5
```

**Link to tracking Issue:** <Issue number if applicable>

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

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Yang Song <songy23@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal/datadog ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants