Skip to content

Conversation

@Nikki371
Copy link

Issue

Errors encountered during local make build on non Linux System.

Although the expectation is to use docker images or virtual OS, thought this might help clear the obvious build issues.

# go.opentelemetry.io/auto/internal/pkg/process
internal/pkg/process/allocate.go:42:13: invalid operation: pagesize * nCPU (mismatched types uint64 and int)
internal/pkg/process/allocate.go:63:14: cannot use nCPU (variable of type int) as uint64 value in struct literal
make: *** [build] Error 1
CGO_ENABLED=0 go build -o otel-go-instrumentation ./cli/...
# go.opentelemetry.io/auto/internal/pkg/instrumentation/bpffs
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:12:47: undefined: process.TargetDetails
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:16:28: undefined: process.TargetDetails
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:20:30: undefined: process.TargetDetails
make: *** [build] Error 1
CGO_ENABLED=0 go build -o otel-go-instrumentation ./cli/...
# go.opentelemetry.io/auto/internal/pkg/instrumentation/probe
internal/pkg/instrumentation/probe/probe.go:76:26: undefined: perf.Record
internal/pkg/instrumentation/probe/probe.go:78:24: undefined: perf.Reader
internal/pkg/instrumentation/probe/probe.go:236:23: undefined: perf.NewReader
internal/pkg/instrumentation/probe/probe.go:267:27: undefined: perf.ErrClosed
internal/pkg/instrumentation/probe/probe.go:321:27: undefined: perf.ErrClosed
internal/pkg/instrumentation/probe/probe.go:353:27: undefined: perf.ErrClosed
make: *** [build] Error 1

Environment

  • OS: Apple M2 (macOS : 15.3.1 (24D70))
  • Go Version: 1.24
  • Version: v.0.21.0

To Reproduce

Steps to reproduce the behavior:

  1. Clone opentelemetry-go-instrumentation to local
  2. run make build

Expected behavior

Expected make build to be successful on non linux systems too as the mock implementation is already present

Changes

  • Created platform-specific implementations for perf reader with Linux implementation using github.com/cilium/ebpf/perf and stub implementations for non-Linux platforms
  • Fixed type mismatches by updating process.TargetDetails to process.Info in bpffs package
  • Changed GetCPUCount return type from int to uint64 for consistency
  • Improved documentation for kernel lockdown mode constants
  • Updated import statements to use the new internal perf package

…tches

- Created platform-specific implementations for perf reader with Linux implementation
  using github.com/cilium/ebpf/perf and stub implementations for non-Linux platforms
- Fixed type mismatches by updating process.TargetDetails to process.Info in bpffs package
- Changed GetCPUCount return type from int to uint64 for consistency
- Improved documentation for kernel lockdown mode constants
- Updated import statements to use the new internal perf package
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 11, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@Nikki371 Nikki371 marked this pull request as ready for review March 11, 2025 12:17
@Nikki371 Nikki371 requested a review from a team as a code owner March 11, 2025 12:17
Copy link
Contributor

@RonFed RonFed left a comment

Choose a reason for hiding this comment

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

I'm not sure about the perf change.
It broke because of cilium/ebpf#1650
From the cilium slack it looks like it broke other stuff as well.
I'm not sure if we need to create these stubs here or it should be handled upstream in the cilium library.

@RonFed
Copy link
Contributor

RonFed commented Apr 9, 2025

Opened cilium/ebpf#1746
cc @MrAlias

@RonFed
Copy link
Contributor

RonFed commented Apr 18, 2025

Seeing the issue linked above is resolved, I think we can remove the perf stubs from this PR.

@MrAlias
Copy link
Contributor

MrAlias commented Apr 18, 2025

Seeing the issue linked above is resolved, I think we can remove the perf stubs from this PR.

Should we update to a commit hash version of that package to support this?

@RonFed
Copy link
Contributor

RonFed commented Apr 19, 2025

Should we update to a commit hash version of that package to support this?

we could wait for an official release, not sure how much time that would take.
I don't mind either way we choose.

@RonFed
Copy link
Contributor

RonFed commented Apr 29, 2025

notes from SIG meeting:

  • we should wait for an official release of cilium/ebpf - with the referenced fix.
  • other fixes in the PR are welcome (like the stubs fixing).
  • For testing purposes, you can use the latest main of upstream and see if the issue is resolved.

@Nikki371

@damemi
Copy link
Member

damemi commented Jun 10, 2025

ping @Nikki371 would you like to update this PR to remove the duplicate fix from cilium and include the other changes, or should we close this?

@MrAlias
Copy link
Contributor

MrAlias commented Jul 8, 2025

@RonFed what things would we like to preserve from this PR? Can you create an issue with the items we want to move to new PRs?

@RonFed
Copy link
Contributor

RonFed commented Jul 27, 2025

Closing, tracked by #2564

@RonFed RonFed closed this Jul 27, 2025
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.

4 participants