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

controller: fix reporter interval mix up with monitor interval #242

Merged

Conversation

Gandem
Copy link
Contributor

@Gandem Gandem commented Nov 18, 2024

In the times.New function signature, reporter interval is first: https://github.com/open-telemetry/opentelemetry-ebpf-profiler/blob/main/times/times.go#L117-L118.

However, in the controller package we provide monitor interval first. Currently this causes both to be mixed up when provided via the command line interface.

Let me know if you'd prefer we switch this to a struct so that we can explicitly specific which attribute is which.

In the `times` function signature, reporter interval is first: https://github.com/open-telemetry/opentelemetry-ebpf-profiler/blob/main/times/times.go#L117-L118.

However, in the `controller` package we provide monitor interval first. Currently this causes both to be mixed up when provided via the command line interface.
@Gandem Gandem requested review from a team as code owners November 18, 2024 18:50
Copy link
Member

@felixge felixge left a comment

Choose a reason for hiding this comment

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

LGTM. I like structs to avoid mistakes like this when passing many arguments of the same type, but I don't feel strongly about it. Should be a separate follow-up patch regardless.

@christos68k christos68k merged commit 9c8ddce into open-telemetry:main Nov 18, 2024
23 checks passed
ltrk2 added a commit to instana/opentelemetry-ebpf-profiler that referenced this pull request Dec 4, 2024
* Fix unwinding at syscall on aarch64 (open-telemetry#218)

* Add PID as an attribute in each sample (open-telemetry#212)

* ebpf: increase number of stack delta buckets (open-telemetry#231)

Signed-off-by: Florian Lehner <[email protected]>

* reporter: use htlhash attribute for profiling specific hash (open-telemetry#236)

Signed-off-by: Florian Lehner <[email protected]>

* reporter: drop fifo (open-telemetry#239)

Signed-off-by: Florian Lehner <[email protected]>

* Drop more unused code (open-telemetry#240)

* reporter: do not add empty attributes (open-telemetry#233)

Signed-off-by: Florian Lehner <[email protected]>

* controller: fix reporter interval mix up with monitor interval (open-telemetry#242)

* Extract reporter runloop (open-telemetry#228)

Co-authored-by: Christos Kalkanis <[email protected]>

* Extract lookupcgroupv2 out of the otlp reporter (open-telemetry#227)

* Add CPU id to trace and trace metadata (open-telemetry#249)

* reporter: don't expire actively used executables (open-telemetry#247)

* Remove legacy code from libpf.UnixTime64 (open-telemetry#252)

* Turn kernel module file parsing errors into warnings (open-telemetry#255)

Co-authored-by: Florian Lehner <[email protected]>

* readatbuf: add missing check when reading from tail chunk (open-telemetry#259)

* metrics: Don't send counters with 0 values (open-telemetry#246)

---------

Signed-off-by: Florian Lehner <[email protected]>
Co-authored-by: umanwizard <[email protected]>
Co-authored-by: Bhavna Jindal <[email protected]>
Co-authored-by: Florian Lehner <[email protected]>
Co-authored-by: Tim Rühsen <[email protected]>
Co-authored-by: Nayef Ghattas <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>
Co-authored-by: Christos Kalkanis <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Joel Höner <[email protected]>
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.

3 participants