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

Exclude 386 from opentelemetry-collector-contrib builds #83

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Mar 17, 2022

Context: open-telemetry/opentelemetry-collector#5013 (comment)

We remove both Linux and Windows, since the issue affects all OSs.

@mx-psi
Copy link
Member Author

mx-psi commented Mar 17, 2022

cc @jpkrohling can you take a look?


distsFlag = flag.String("d", "", "Collector distributions(s) to build, comma-separated")
)

func architecturesForDist(dist string) []string {
architectures := []string{"386", "amd64", "arm64"}
if dist == "otelcol-contrib" {
Copy link
Member

Choose a reason for hiding this comment

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

This is a good solution for this release and for the time being, but this would need to go into a config somewhere. I would even argue that we can remove 386 entirely, not only for the contrib. But we can have this discussion after this release :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO reintroducing 386 is fine (since the additional effort to support it is small) but we need to have CI tests first on core and contrib to prevent issues like the one on this release (I created open-telemetry/opentelemetry-collector/issues/5031 and open-telemetry/opentelemetry-collector-contrib/issues/8544 to track this)

@jpkrohling jpkrohling merged commit fe8f761 into open-telemetry:main Mar 17, 2022
mx-psi added a commit that referenced this pull request Jun 1, 2022
codeboten pushed a commit that referenced this pull request Jun 3, 2022
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.

2 participants