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

Update signalfxexporter to use gopsutil/v3 #5537

Conversation

mustafain117
Copy link
Contributor

Description:

Several components use gopsutil v2, which is being discontinued (shirou/gopsutil/issues/1078). This PR updates signalfxexporter component to use gopsutil v3.

Link to tracking Issue:
#5510

Testing:

  • Reviewed changes here to ensure moving from gopsutil v2 to v3 does not introduce a breaking change.
  • Ran existing tests for signalfxexporter, tests pass.

@mustafain117 mustafain117 requested a review from a team as a code owner September 30, 2021 20:48
@mustafain117 mustafain117 changed the title Update gopsutil to v3 signalfxexporter Update signalfxexporter to use gopsutil/v3 Sep 30, 2021
@@ -9,7 +9,7 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/internal/splunk v0.36.0
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/batchperresourceattr v0.36.0
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/experimentalmetricmetadata v0.36.0
github.com/shirou/gopsutil v3.21.8+incompatible
github.com/shirou/gopsutil/v3 v3.21.8
Copy link
Member

Choose a reason for hiding this comment

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

Could you bump this to v3.21.9? (It fixes a bug that I am trying to get rid off in #5544

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mx-psi
Sure, please note:
Before bumping signalfxexporter to gopsutil/v3 v3.21.8, make test failed locally with 1 duplicate symbol error as tracked in #5425
However, after bumping signalfxexporter gopsutil/v3 v3.21.8, running make test fails locally with 4 duplicate symbols(instead of 1 duplicate symbol prior):

/usr/local/go/pkg/tool/darwin_amd64/link: running clang failed: exit status 1
duplicate symbol '_get_temperature' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000033.o
duplicate symbol '_open_smc' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000033.o
duplicate symbol '_close_smc' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000033.o
duplicate symbol '_readdrivestat' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000036.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3864802333/000039.o
ld: 4 duplicate symbols for architecture x86_64

I believe the other 3 duplicate symbol errors need to be resolved(in gopsutil library) as well, please let me know how to proceed.
Here is the latest output from make test after bumping to v3.21.9:

/usr/local/go/pkg/tool/darwin_amd64/link: running clang failed: exit status 1
duplicate symbol '_get_temperature' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000033.o
duplicate symbol '_open_smc' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000033.o
duplicate symbol '_close_smc' in:
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000024.o
    /var/folders/6y/27qzvqc137v5dsm_m0f39_900000gs/T/go-link-3198491705/000033.o
ld: 3 duplicate symbols for architecture x86_64

Copy link
Member

@mx-psi mx-psi Oct 4, 2021

Choose a reason for hiding this comment

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

That's funny :) I guess I will have to open another PR to fix these new duplicate symbol errors upstream. It can be dealt with separately from these PRs

@mustafain117
Copy link
Contributor Author

Looks like there is already a PR addressing this and other components that need to be updated (#5570), closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants