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

process_xxx_memory statistics for macOS (without cgo) #1629

Closed
wants to merge 2 commits into from

Conversation

mharbison72
Copy link
Contributor

This is the alternative to PR 1616, but keeping the cgo usage isolated to tests so that users don't need to fiddle with enabling cgo, or deal with losing cross platform support.

If this is viable, we can probably finish off the rest of #1600 with this technique. (Currently, RSS, VSIZE, and the network traffic stats are not reported on macOS- this handles the first two items.)

@bwplotka

prometheus#1600)

Unfortunately, these values aren't available from getrusage(2), or any other
builtin Go API.  Using cgo is one alternative.  It's possible to conditionalize
everything such that cgo can remain disabled on non-Darwin platforms, or even
when cross-compiling Darwin executables on a non-Darwin platform (and stub in
code that causes the metrics to not be exported).  `CGO_ENABLED=1` is set by
default on macOS, but unfortunately is off for the non-host architecture, even
when gcc supports cross-compiling.  (e.g. building with GOARCH=amd on an M2 mac
skipped the cgo code.)  I think that's too subtle of a distinction to rely on
cgo.

There's no builtin equivalent of `syscall.NewLazyDLL()` and `.NewProc()` on
macOS that Go provides for Windows, so we're stuck with a 3rd party dependency.
But it seems stable, maintained, ang getting a fair amount of usage.  I'm
avoiding their struct deserialization because these native structs are packed
differently than the equivalent Go structs, which was causing bad values to be
returned.

The code is heavy with inline comments, and I tried keeping the type names the
same as the C code to make it easier to search for them.  I'm not sure that we
need to do the `mach_vm_region()` call to adjust the `task_info()` values,
because I've never seen that conditional evaluate to True on either amd64,
arm64, or when amd64 is run under Rosetta.  But this is what `ps(1)` does, and
I think it's reasonable to try to match that unless somebody knows it's dead
code.

Signed-off-by: Matt Harbison <mharbison72@gmail.com>
Here we have to use cgo, but since it's conditionalized to 1) tests, 2) only on
Darwin, 3) when `CGO_ENABLED=1`, and 4) gracefully skips when it is disabled, I
think it's OK.  There were many layers to the C types that are defined, and
subtle sizing and offset issues that the C compiler would handle that need to be
correct here, so I think it's worth having around.  I don't expect any of the
sizes or offsets to change until a new architecture is supported for macOS, but
these are definitely not the more documented APIs.

Signed-off-by: Matt Harbison <mharbison72@gmail.com>
@mharbison72
Copy link
Contributor Author

The cgo version was landed instead of this.

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.

1 participant