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

[receiver/windowsperfcounters] Unit tests are not running on CI #32508

Closed
alxbl opened this issue Apr 18, 2024 · 7 comments · Fixed by #32518
Closed

[receiver/windowsperfcounters] Unit tests are not running on CI #32508

alxbl opened this issue Apr 18, 2024 · 7 comments · Fixed by #32518

Comments

@alxbl
Copy link
Member

alxbl commented Apr 18, 2024

Component(s)

receiver/windowsperfcounters

What happened?

Description

While attempting to add unit tests for #32384 I realized that the windowsperfcounters were failing. After pinging on Slack, it turns out that the windows test might not be running on CI.

Steps to Reproduce

cd $src/receivers/windowsperfcountersreceiver
go test

Expected Result

PASS

Actual Result

--- FAIL: TestCreateMetricsReceiver (0.00s)
panic: invalid character(s) in type "nop/498597c0-fa00-45b1-84cd-8971618f7d89" [recovered]
        panic: invalid character(s) in type "nop/498597c0-fa00-45b1-84cd-8971618f7d89"

goroutine 23 [running]:
testing.tRunner.func1.2({0x1139ae0, 0xc00021d240})
        C:/Program Files/Go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
        C:/Program Files/Go/src/testing/testing.go:1634 +0x377
panic({0x1139ae0?, 0xc00021d240?})
        C:/Program Files/Go/src/runtime/panic.go:770 +0x132
go.opentelemetry.io/collector/component.MustNewType(...)
        C:/Users/abeaulieu/go/pkg/mod/go.opentelemetry.io/collector/component@v0.98.1-0.20240412014414-62f589864e3d/config.go:156
go.opentelemetry.io/collector/receiver/scraperhelper.NewScraper({0xc00023c2a0?, 0x1236eff?}, 0xc00021d220, {0xc000091d10, 0x2, 0xc00023c1b0?})
        C:/Users/abeaulieu/go/pkg/mod/go.opentelemetry.io/collector/receiver@v0.98.1-0.20240412014414-62f589864e3d/scraperhelper/scraper.go:70 +0x114
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/windowsperfcountersreceiver.createMetricsReceiver({0x1163260?, 0x1198020?}, {{{{0x1236eff, 0x3}}, {0xc00023c1b0, 0x24}}, {0xc000224400, {0x134dde0, 0xc00021cd20}, {0x134de08, ...}, ...}, ...}, ...)
        C:/git/opentelemetry-contrib/receiver/windowsperfcountersreceiver/factory_windows.go:27 +0x318
go.opentelemetry.io/collector/receiver.CreateMetricsFunc.CreateMetricsReceiver(...)
        C:/Users/abeaulieu/go/pkg/mod/go.opentelemetry.io/collector/receiver@v0.98.1-0.20240412014414-62f589864e3d/receiver.go:134
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/windowsperfcountersreceiver.TestCreateMetricsReceiver(0xc0002111e0)
        C:/git/opentelemetry-contrib/receiver/windowsperfcountersreceiver/factory_windows_test.go:34 +0x343
testing.tRunner(0xc0002111e0, 0x128c180)
        C:/Program Files/Go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
        C:/Program Files/Go/src/testing/testing.go:1742 +0x390
exit status 2
FAIL    github.com/open-telemetry/opentelemetry-collector-contrib/receiver/windowsperfcountersreceiver  0.104s

Collector version

v0.98.0

Environment information

Environment

Windows 11

OpenTelemetry Collector configuration

N/A

Log output

N/A

Additional context

N/A

@alxbl alxbl added bug Something isn't working needs triage New item requiring triage labels Apr 18, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mx-psi mx-psi added priority:p1 High os:windows and removed needs triage New item requiring triage labels Apr 18, 2024
@mx-psi
Copy link
Member

mx-psi commented Apr 18, 2024

cc @pjanotti as the Windows platform owner

@mx-psi
Copy link
Member

mx-psi commented Apr 18, 2024

@mx-psi
Copy link
Member

mx-psi commented Apr 18, 2024

For the specific issue mentioned: I believe the error comes from

which passes a component Type with a / in it, so it's not something specific to OP's environment

@alxbl
Copy link
Member Author

alxbl commented Apr 18, 2024

As @mx-psi says, I replaced params.ID.String() (which is nop/$uuid and causes the panic due to / and - characters) by metadata.Type.String() as was done in #31135 for other components and the tests started passing.

@dashpole
Copy link
Contributor

@braydonk

@pjanotti
Copy link
Contributor

pjanotti commented Apr 18, 2024

[EDIT]: Sorry, missed the link above...
First thing it is not running because the build and test was not updated after groups were further split from the original 2, see https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/30901/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R29.

Re-enabling those will likely cause a bunch of failures, but, it is the way to go: I will open that PR first and then we fix the failures one by one until we can merge the updated test for Windows.

TylerHelmuth added a commit that referenced this issue Apr 18, 2024
**Description:**
Someone, and we may never know who, forgot to update the windows test
groups when they created
#30901

Fixes
#32508

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment