Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Allow to collect host architecture #1483
Allow to collect host architecture #1483
Changes from 4 commits
933ba47
f3115a0
dc3fdce
f6df42a
3cf01a1
cd048f9
49fb678
7a98082
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Values here neither match the common convention for Linux / Debian arch, e.g. not matching what's reported by
uname -m
, nor match what Node.jsos.arch()
API would have reported; nor what Rust architecture uses for the standard set of arch.... For different languages - you'd have different strings. And none of them would match the list proposed here 😄 So it's going to be quite an adventure to normalize to that proposed list 🤷I believe a better set could be this:
C++, Rust, or Node - typically have their own API to obtain the process arch string. This string is going to be in their own common format. For Node - this list is well-described here: https://nodejs.org/api/os.html#os_os_arch - I personally believe Node's list is quite reasonable.
More complete set of architectures is described here in Linux kernel tree:
https://github.com/torvalds/linux/tree/master/arch
If you truly intend to describe the host arch , it would be reasonable to respect the Linux standard names for the archs, as the most prominent host OS. That's also what
uname -m
command would typically report, namely[arm|arm64|ia64|mips|x86]
. I would've left the loophole in this list to allow reporting what OS language-specific API reports as architecture. Any data analysis within a slice of data reported by agents written in specific language is consistent: a language (e.g.node
) would report only a fixed set of archs. There is also not going to be any need to normalize.I don't like this proposed list because it does not seem to align with the arch lists typically captured by prominent language APIs, neither matches the Linux kernel arch list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks so much better. Go has a similar approach here. I initially wanted to propose something more aligned with uname but the formatting of OS names made me want to follow the UPPER_CASE style here. I'm modifying this PR in a bit.