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

processcollector: Overhaul processcollector for consistency and testability #1591

Closed
bwplotka opened this issue Aug 20, 2024 · 0 comments · Fixed by #1644
Closed

processcollector: Overhaul processcollector for consistency and testability #1591

bwplotka opened this issue Aug 20, 2024 · 0 comments · Fixed by #1644

Comments

@bwplotka
Copy link
Member

bwplotka commented Aug 20, 2024

While investigating and fixing #1584 I noticed some tech-debt build up over time:

image

  • func (c *processCollector) Describe(ch chan<- *Desc) { is always the same despite on different platforms we don't expect certain metrics. Perhaps to move per platform?
  • We have noop files for wasip1 and js, but for darwin (mac) we simply use "other" file and error out in silent way (if Errors is equal to false). Let's make it consistent and either error all of them silently (and verbosely if Errors == true), or have all of them noop. This is reflected by inconsistency in prod files build tags and test file build tags.
  • Each working platform such ensure that maximum amount of metrics matches list of defined metrics in Description. This is to ensure we don't hit collected metric process_network_receive_bytes_total counter with unregistered descriptor #1584 again. Some ideas:
    • Move internals behind interface, separate for windows and linux.
    • Use reflection in tests to make sure all desc defined are described in Description method?
    • Add comment if nothing else to not forget about syncing desc's with Description.
    • Returning empty Description (: PedanticRegistry ignores that case. This is ofc is bad as registry allows to validate against duplicates.
    • ?

Acceptance Criteria

  • Fix the above.

Also, added issue for implementing those for darwin: #1590

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant