Skip to content

Conversation

ExceptionalHandler
Copy link
Contributor

Description

Adding stub function to compile metrics package on Windows

Changelog

Compile the package pkg/metrics on Windows

@ExceptionalHandler ExceptionalHandler requested a review from a team as a code owner March 20, 2025 18:51
Copy link

netlify bot commented Mar 20, 2025

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 6671543
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/67dc63b02e1de800081ec590
😎 Deploy Preview https://deploy-preview-3530--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Adding stub function to compile metrics package on Windows

Signed-off-by: Anadi Anadi <[email protected]>
@ExceptionalHandler ExceptionalHandler force-pushed the build/windows/raw_syscall_metrics branch from 6671543 to 8a8503b Compare March 20, 2025 18:57
@olsajiri olsajiri added the release-note/minor This PR introduces a minor user-visible change label Mar 24, 2025
// SPDX-License-Identifier: Apache-2.0
// Copyright Authors of Tetragon

package syscallmetrics
Copy link
Contributor

Choose a reason for hiding this comment

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

are there syscalls on windows ? no idea ;-)

if we need syscallmetrics package in windows then I think the syscallmetrics.Handle should have _windows/_linux versions, the generic version is now checking for raw_syscalls/sys_enter which will not happen on windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a compilation only change. We cannot remove the syscallmetrics code from Windows without impacting compilation of common code. Goland does not have #ifdef based conditional compilation, so we need stub packages to compile common code. .

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, understood, I just think it'd make more sense to me to do that with syscallmetrics.Handle than with rawSyscallName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I considered that but this approach minimizes code duplication, which is the approach we agreed upon in team meeting. This is the smallest change to to let the package compile n Windows.

We cannot rule out use of syscallmetrics in Windows in the future entirely, so stubbing out the whole package might not be the best approach.

Also we already committed raw_syscall_name_linux.go, wouldn't want to walk back on that....

@olsajiri olsajiri merged commit f4c1920 into cilium:main Mar 25, 2025
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants