[extension/cgroupruntimeextension] chore: disable GoMaxProcs by default in cgroupruntimeextension#47487
[extension/cgroupruntimeextension] chore: disable GoMaxProcs by default in cgroupruntimeextension#47487heliapb wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
rogercoll
left a comment
rogercoll
left a comment
There was a problem hiding this comment.
I don't think we should remove as it might still be helpful in ECS environments, see https://github.com/rdforte/gomaxecs?tab=readme-ov-file#go-125
An alternative would be making the gomaxprocs opt-in, wdyt?
Thanks for the feedback, sound good. Going to change to draft and make the changes. |
|
Please add a changelog. |
heliapb
force-pushed
the
chore/rmgomaxprc
branch
from
2f9d320 to
744044a
Compare
Signed-off-by: Hélia Barroso <helia_barroso@hotmail.com>
heliapb
force-pushed
the
chore/rmgomaxprc
branch
from
744044a to
334cf69
Compare
heliapb
changed the title
|
Hi @rogercoll changed to disable by default, so to opt-in. Let me know if anything else needs fix. |
|
/workflow-approve |
| Linux container environments. Users on AWS ECS should explicitly set | ||
| `gomaxprocs.enabled: true` as Go 1.25 does not read ECS task metadata | ||
| for CPU limits. See https://go.dev/blog/container-aware-gomaxprocs. | ||
| issues: [0] |
There was a problem hiding this comment.
| issues: [0] | |
| issues: [47487] |
| @@ -0,0 +1,10 @@ | |||
| change_type: enhancement | |||
There was a problem hiding this comment.
| change_type: enhancement | |
| change_type: breaking |
I'm not sure if we should treat this as a breaking change.
@rogercoll @atoulme WDYT?
|
What's the benefit of this change? |
Signed-off-by: Hélia Barroso <helia_barroso@hotmail.com>
Hi @mx-psi well I think might a bit redundant to have both automaxprocs and the Go runtime set at the same time for anyone that uses go 1.25 in k8s or other for of container app, and thus with this change becomes an opt in makes more sense in my point of view. And as @rogercoll pointed, it might still be helpful in ECS environments, see https://github.com/rdforte/gomaxecs?tab=readme-ov-file#go-125. Let me know if there's anything that I'm missing. Thankls |
|
@heliapb It would help to file an issue explaining concretely the problem described here #47487 (comment) maybe with a little more detail. |
6 participants
Description
No issue, based of https://go.dev/blog/container-aware-gomaxprocs
Link to tracking issue
Fixes
Testing
Documentation