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

[testbed] disable setting grpc log #9564

Merged

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Apr 27, 2022

This is causing a race condition with the InProcessCollector.

This change includes:

Fixes #9557

@bogdandrutu bogdandrutu marked this pull request as ready for review April 28, 2022 14:30
@bogdandrutu bogdandrutu requested a review from a team as a code owner April 28, 2022 14:30
@bogdandrutu
Copy link
Member

@codeboten please run make update-otel

@codeboten codeboten added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 28, 2022
Copy link
Contributor Author

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Alex Boten added 2 commits April 28, 2022 08:01
This is causing a race condition with the InProcessCollector.
@codeboten codeboten force-pushed the codeboten/disable-setting-grpc-log branch from 0e9a760 to 5f3aff3 Compare April 28, 2022 15:01
@bogdandrutu
Copy link
Member

=== RUN   TestMetricGroupData_toDistributionUnitTest/histogram_with_startTimestamp_of_11
    otlp_metricfamily_test.go:181: 
        	Error Trace:	otlp_metricfamily_test.go:181
        	Error:      	Not equal: 
        	            	expected: internal.HistogramDataPoint{orig:(*v1.HistogramDataPoint)(0xc00012e360)}
        	            	actual  : internal.HistogramDataPoint{orig:(*v1.HistogramDataPoint)(0xc00012e2d0)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -29,4 +29,3 @@
        	            	   },
        	            	-  ExplicitBounds: ([]float64) {
        	            	-  },
        	            	+  ExplicitBounds: ([]float64) <nil>,
        	            	   Exemplars: ([]v1.Exemplar) <nil>,
        	Test:       	TestMetricGroupData_toDistributionUnitTest/histogram_with_startTimestamp_of_11
        	Messages:   	Expected the points to be equal
=== RUN   TestMetricGroupData_toDistributionUnitTest/histogram_that_is_stale
    otlp_metricfamily_test.go:181: 
        	Error Trace:	otlp_metricfamily_test.go:181
        	Error:      	Not equal: 
        	            	expected: internal.HistogramDataPoint{orig:(*v1.HistogramDataPoint)(0xc0003f3290)}
        	            	actual  : internal.HistogramDataPoint{orig:(*v1.HistogramDataPoint)(0xc0003f3200)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -27,4 +27,3 @@
        	            	   },
        	            	-  ExplicitBounds: ([]float64) {
        	            	-  },
        	            	+  ExplicitBounds: ([]float64) <nil>,
        	            	   Exemplars: ([]v1.Exemplar) <nil>,
        	Test:       	TestMetricGroupData_toDistributionUnitTest/histogram_that_is_stale
        	Messages:   	Expected the points to be equal

This error is caused by a change in CopyTo where we don't copy empty array, we live it nil in that case.

@dmitryax
Copy link
Member

dmitryax commented Apr 28, 2022

@codeboten @bogdandrutu do you think we can update the test to change original slices to nil? Otherwise I can also update the CopyTo to make exact copy of slices

@codeboten
Copy link
Contributor Author

@dmitryax I updated the test

@codeboten codeboten merged commit b9915f7 into open-telemetry:main Apr 28, 2022
@codeboten codeboten deleted the codeboten/disable-setting-grpc-log branch April 28, 2022 18:42
djaglowski pushed a commit to djaglowski/opentelemetry-collector-contrib that referenced this pull request May 10, 2022
…-telemetry#9564)

This change includes:
- set the flag to disable setting the grpc logger from the `InProcessCollector`
- `make update-otel`
- update calls to `featuregate` as a result of open-telemetry/opentelemetry-collector@addf0ef
- update tests as a result of open-telemetry/opentelemetry-collector@5658f35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing tests: TestHarness_MetricsGoldenData/otlp-otlp
5 participants