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

Parse Go versions that contain GOEXPERIMENT suffixes. #389

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

zchee
Copy link
Contributor

@zchee zchee commented Oct 16, 2023

Trim the X:... suffix when any GOEXPERIMENT is enabled.

Currently, can't parse the Go version if the target Go binary enables any GOEXPERIMENT. Seems that hashicorp/go-version doesn't support it.
When if enable arenas GOEXPERIMENT, the error message is here:

Malformed version: 1.21.3 X:arenas

Trim X: (note that started one space) before passing vers to version.NewVersion.


This package hasn't a test case but confirmed at least my GKE cluster.

@zchee zchee requested a review from a team October 16, 2023 10:29
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

Please add a changelog entry for this.

internal/pkg/process/module.go Show resolved Hide resolved
@zchee zchee changed the title internal/pkg/process: fix to trim GOEXPERIMENT suffix Fix to trim GOEXPERIMENT suffix Oct 19, 2023
@zchee zchee force-pushed the fix-goversion-exp branch from 842adf6 to ca14289 Compare October 19, 2023 17:49
@zchee
Copy link
Contributor Author

zchee commented Oct 19, 2023

@MrAlias PTAL.

CHANGELOG.md Outdated Show resolved Hide resolved
internal/pkg/process/module.go Outdated Show resolved Hide resolved
@zchee zchee force-pushed the fix-goversion-exp branch from ca14289 to dc4beee Compare October 20, 2023 02:38
@zchee
Copy link
Contributor Author

zchee commented Oct 20, 2023

@MrAlias PTALA.

@zchee zchee changed the title Fix to trim GOEXPERIMENT suffix Parse Go versions that contain GOEXPERIMENT suffixes. Oct 20, 2023
CHANGELOG.md Show resolved Hide resolved
@MrAlias MrAlias merged commit 598d39d into open-telemetry:main Oct 20, 2023
@zchee zchee deleted the fix-goversion-exp branch October 20, 2023 15:27
@MrAlias MrAlias added this to the v0.8.0-alpha milestone Nov 14, 2023
@MrAlias MrAlias mentioned this pull request Nov 14, 2023
@mike2194
Copy link

mike2194 commented May 2, 2024

seems this fix is reverted in v0.12.0-alpha

seeing the following error from GOEXPERIMENT "nocoverageredesign"

{"level":"info","ts":1714599599.5173488,"logger":"Instrumentation.Analyzer","caller":"process/discover.go:76","msg":"found process","pid":30}
{"level":"error","ts":1714599599.5178316,"logger":"go.opentelemetry.io/auto","caller":"cli/main.go:103","msg":"failed to create instrumentation","error":"Malformed version: 1.22.0 X:nocoverageredesign","stacktrace":"main.main\n\t/app/cli/main.go:103\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:271"}```

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.

3 participants