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

cmd/trace-agent: enable build for aix/ppc64 #6738

Merged
merged 2 commits into from
Mar 4, 2021
Merged

Conversation

knusbaum
Copy link
Contributor

This commit adds support for building the trace-agent binary on AIX 7.2 for
Power8 and Power9 architectures. The trace-agent binary is confirmed to run
and successfully submit traces to the intake.

This PR depends on a PR to our viper fork (DataDog/viper#15). As such, his PR requires a version bump to github.com/DataDog/viper once that PR is merged and a new version is tagged.

@knusbaum knusbaum added the team/agent-apm trace-agent label Nov 11, 2020
@knusbaum knusbaum requested a review from a team as a code owner November 11, 2020 17:32
@knusbaum knusbaum requested a review from a team as a code owner November 11, 2020 20:39
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Please ask for another review once the dependant PR is merged and this one is updated.

pkg/trace/sampler/scoresampler_test.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@knusbaum knusbaum force-pushed the knusbaum/aix-build branch 3 times, most recently from d15da55 to ea1514a Compare November 18, 2020 19:26
@knusbaum knusbaum requested a review from gbbr December 2, 2020 22:26
@knusbaum knusbaum added this to the 7.25.0 milestone Dec 2, 2020
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Thanks! Nice job!

It would be nice if we could have CI run at least the integration tests on AIX too. Or, do you think it shouldn't be needed? Regardless, this solution (if we decide it's worthwhile) does not have to be for this PR.

pkg/trace/sampler/scoresampler_test.go Outdated Show resolved Hide resolved
pkg/trace/watchdog/cpu_aix.go Show resolved Hide resolved
pkg/trace/watchdog/cpu_aix.go Outdated Show resolved Hide resolved
pkg/trace/watchdog/cpu_aix.go Outdated Show resolved Hide resolved
pkg/trace/watchdog/cpu_aix.go Show resolved Hide resolved
releasenotes/notes/add-aix-support-0170d23332663a08.yaml Outdated Show resolved Hide resolved
@knusbaum
Copy link
Contributor Author

knusbaum commented Dec 4, 2020

It would be nice if we could have CI run at least the integration tests on AIX too. Or, do you think it shouldn't be needed? Regardless, this solution (if we decide it's worthwhile) does not have to be for this PR.

Definitely. We will hopefully be able to automate the CI moving forward. Right now I'm running the unit tests and integration tests manually on an AIX machine.

@truthbk truthbk modified the milestones: 7.25.0, 7.26.0 Dec 14, 2020
@ogaca-dd ogaca-dd modified the milestones: 7.26.0, 7.27.0 Jan 25, 2021
@gbbr gbbr force-pushed the knusbaum/aix-build branch 2 times, most recently from bf8802e to 438340b Compare February 25, 2021 11:46
Copy link
Member

@olivielpeau olivielpeau left a comment

Choose a reason for hiding this comment

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

LGTM, but the android build in gitlab is failing on this error, needs to be fixed:

# github.com/DataDog/datadog-agent/pkg/config
/go/src/github.com/DataDog/datadog-agent/pkg/config/config_android.go:35:14: cannot use afs (type AssetFs) as type afero.Fs in argument to config.SetFs:
	AssetFs does not implement afero.Fs (missing Chown method)

Probably because type AssetFs in pkg/config/config_android.go needs to be updated to reflect the changes in the interface of afero.Fs introduced by the version update on github.com/spf13/afero

knusbaum and others added 2 commits March 4, 2021 11:21
This commit adds support for building the trace-agent binary on AIX 7.2 for
Power8 and Power9 architectures. The trace-agent binary is confirmed to run
and successfully submit traces to the intake.
@gbbr gbbr force-pushed the knusbaum/aix-build branch from 438340b to 2e31209 Compare March 4, 2021 09:28
@gbbr gbbr requested a review from olivielpeau March 4, 2021 10:44
@gbbr
Copy link
Contributor

gbbr commented Mar 4, 2021

@olivielpeau I think we're good now. Can you PTAL?

Copy link
Member

@olivielpeau olivielpeau left a comment

Choose a reason for hiding this comment

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

CI is green, GTG!

@gbbr gbbr merged commit b2c5cde into master Mar 4, 2021
@gbbr gbbr deleted the knusbaum/aix-build branch March 4, 2021 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/agent-apm trace-agent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants