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.

Sign up for GitHub

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

Jump to bottom

Fix(CI): 'update-otel' make target fails with 'go-chi' dependency issue #8838

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Fix(CI): 'update-otel' make target fails with 'go-chi' dependency issue #8838

merged 1 commit into from
Mar 29, 2022

Conversation

hickeyma
Copy link
Contributor

This PR replaces the chi dependency with v4.0.0 until the issue is resolved with the dd-trace-go module.

Fixes #8837

Signed-off-by: Martin Hickey martin.hickey@ie.ibm.com

@hickeyma hickeyma requested a review from a team March 24, 2022 22:12
@hickeyma hickeyma requested a review from mx-psi as a code owner March 24, 2022 22:12
@hickeyma
Copy link
Contributor Author

hickeyma commented Mar 25, 2022 •
edited
Loading

Don't think a changelog is required.

@hickeyma
Copy link
Contributor Author

hickeyma commented Mar 25, 2022 •
edited
Loading

Getting fails on loadtest/TestMetricsFromFile and unittest. The errors are as follows..

unittest:

=== RUN   TestEndToEnd
[17684](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17684)
    receiver_test.go:118: 
[17685](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17685)
        	Error Trace:	receiver_test.go:118
[17686](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17686)
        	            				receiver_test.go:100
[17687](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17687)
        	            				receiver_test.go:72
[17688](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17688)
        	Error:      	Should be true
[17689](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17689)
        	Test:       	TestEndToEnd
[17690](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689034341?check_suite_focus=true#step:8:17690)
        	Messages:   	timestamp_now metric not found

loadtest/TestMetricsFromFile

--- FAIL: TestMetricsFromFile (3.16s)
[294](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:294)
FAIL
[295](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:295)
exit status 1
[296](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:296)
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/testbed/tests	80.561s
[297](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:297)
# Test PerformanceResults
[298](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:298)
Started: Fri, 25 Mar 2022 08:39:24 +0000
[299](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:299)

[300](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:300)
Test                                    |Result|Duration|CPU Avg%|CPU Max%|RAM Avg MiB|RAM Max MiB|Sent Items|Received Items|
[301](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:301)
----------------------------------------|------|-------:|-------:|-------:|----------:|----------:|---------:|-------------:|
[302](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:302)
Metric10kDPS/Carbon                     |PASS  |     16s|    93.5|   111.3|         63|         90|    443100|        443100|
[303](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:303)
Metric10kDPS/OpenCensus                 |PASS  |     15s|    30.7|    31.3|         67|         94|   1049300|       1049300|
[304](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:304)
Metric10kDPS/OTLP                       |PASS  |     15s|    36.3|    36.7|         68|         96|   1049300|       1049300|
[305](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:305)
Metric10kDPS/OTLP-HTTP                  |PASS  |     15s|    38.8|    39.7|         64|         90|   1050000|       1050000|
[306](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:306)
Metric10kDPS/SignalFx                   |PASS  |     15s|    54.8|    55.0|         70|         95|   1049300|       1049300|
[307](https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/5689071865?check_suite_focus=true#step:14:307)
MetricsFromFile                         |FAIL  |      3s|    33.0|    33.0|         31|         95|      3074|          2914|RAM consumption is 95 MiB, max expected is 94 MiB

Not really sure what is causing this. I would appreciate any feedback.

Also, getting 'timeout' issues when trying to run the test locally.

@mx-psi mx-psi requested a review from Aneurysm9 March 25, 2022 11:32
@mx-psi mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 25, 2022
@hickeyma
Copy link
Contributor Author

Update => Test passing locally.

@mx-psi
Copy link
Member

mx-psi commented Mar 25, 2022

Meant to write a comment but forgot to do it 😅 I don't think the test failures are related to the changes on this PR, you can ignore them (and/or raise an issue so that they get fixed). I requested a review from @Aneurysm9 since he also faces the go-chi issue and can verify that this solves it for him.

@hickeyma
Copy link
Contributor Author

Thanks @mx-psi. Seems to be some flakiness going on. Tests are passing now! :)

@Aneurysm9
Copy link
Member

This is the same workaround we applied in the AWS distro. Can confirm it was working there.

Please ensure you've merged origin/main recently as it seems that this branch has out-of-date test workflow definitions.

@hickeyma
Copy link
Contributor Author

Thanks for review and feedback @Aneurysm9.

Please ensure you've merged origin/main recently as it seems that this branch has out-of-date test workflow definitions.

Rebased.

@hickeyma
Copy link
Contributor Author

@mx-psi Looks like the load tests that are failing are flakes.

@hickeyma
Fix 'update-otel' make target fails with 'go-chi' dependency issue
dbbf5e5 
opentelemetry-collector-contrib has a dependency on `gopkg.in/DataDog/dd-trace-go.v1 v1.37.0`.
`gopkg.in/DataDog/dd-trace-go.v1 v1.37.0` has a dependency on `github.com/go-chi/chi v4.0.0-rc1`.
However, the tag has been removed from the repository for `v4.0.0-rc1`.

This means then that the `update-otel` target fails as follows:

go: gopkg.in/DataDog/dd-trace-go.v1@v1.37.0 requires
	github.com/go-chi/chi/v4@v4.0.0-rc1: reading github.com/go-chi/chi/go.mod at
revision v4.0.0-rc1: unknown revision v4.0.0-rc1

This PR replaces the chi dependency with `v4.0.0` until the issue is resolved with
the `dd-trace-go` module.

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
@hickeyma
Copy link
Contributor Author

hickeyma commented Mar 29, 2022 •
edited
Loading

Rebased there again. If this is good to go, does someone mind merging this? It's just that there will potentially be lots of changes around the Go module dependencies.

@mx-psi
Copy link
Member

mx-psi commented Mar 29, 2022

Rebased there again. If this is good to go, does someone mind merging this? It's just that there will potentially be lots of changes around the Go module dependencies.

I marked as ready to merge, a maintainer will give a final look and should merge it soon :)

@Aneurysm9 Aneurysm9 added the ready to merge Code review completed; ready to merge by maintainers label Mar 29, 2022
@codeboten codeboten merged commit 2980ae3 into open-telemetry:main Mar 29, 2022
@hickeyma hickeyma deleted the fix/go-chi-dep-issue branch March 30, 2022 07:53
@hickeyma
Copy link
Contributor Author

Thanks for the reviews folks and the merge.

@hickeyma hickeyma mentioned this pull request Apr 8, 2022
Closed
6 tasks
@ahayworth ahayworth mentioned this pull request May 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeboten codeboten codeboten approved these changes

@Aneurysm9 Aneurysm9 Aneurysm9 approved these changes

@mx-psi mx-psi mx-psi approved these changes

Assignees

@dmitryax dmitryax

Labels
ready to merge Code review completed; ready to merge by maintainers Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CI] 'update-otel' make target fails with 'go-chi' dependency issue
5 participants
@hickeyma @mx-psi @Aneurysm9 @codeboten @dmitryax