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.

Sign up for GitHub

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

Jump to bottom

POC for the new Collector interface #219

Merged
merged 7 commits into from
Apr 16, 2024
Merged

POC for the new Collector interface #219

merged 7 commits into from
Apr 16, 2024

Conversation

likhitha77
Copy link
Contributor

@likhitha77 likhitha77 commented Apr 8, 2024 •
edited by felixsch
Loading

Description

Related card : https://trello.com/c/u1pb12AM/3245-teddpoc-rfc-new-approach-for-telemetry-data-on-suseconnect-ng

This is a POC for the refactored new implementation for the metric collection interface. This is intended to improve on the current hw_infos metrics and make it more dynamic.

Requirements can be found here: https://etherpad.opensuse.org/p/suseconnect-collector-framework

How to test

  • Check the code to see if the new implementation is easy to understand and it makes sense as an improvement
  • Run the tests for the collectors package : go test ./internal/collectors/

Thanks for the review!

PS: Be aware this pull request is against master right now, to run the CI pipelines. It depends on: #218 Please review this first!

@likhitha77 likhitha77 changed the base branch from refactor_zypper_implementation to main April 8, 2024 14:32
@likhitha77
Copy link
Contributor Author

[TEDD][POC/RFC] New approach for telemetry data on SUSEconnect-ng

mssola
mssola requested changes Apr 10, 2024
View reviewed changes
Copy link
Contributor

@mssola mssola left a comment

Choose a reason for hiding this comment

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

I stopped the review because I believe that there are changes that conflict with the other PR. I guess this PR is on top of the other?

Anyways, let's get one of the PR merged and then we can work on this one.

internal/collectors/cpu_test.go Outdated Show resolved Hide resolved
internal/collectors/lscpu.go Show resolved Hide resolved
internal/connect/client.go Outdated Show resolved Hide resolved
internal/credentials/credentials.go Outdated Show resolved Hide resolved
@likhitha77 likhitha77 changed the base branch from main to refactor_zypper_implementation April 10, 2024 11:44
@likhitha77
Copy link
Contributor Author

I stopped the review because I believe that there are changes that conflict with the other PR. I guess this PR is on top of the other?

Anyways, let's get one of the PR merged and then we can work on this one.

Hey Miquel,
Sorry! We should've added a WIP label. The diff was supposed to be against therefactor_zypper_implementation branch. But we had temporarily changed it to main to run the CI.

I changed the base back to the zypper branch now. The diff should make more sense now! Sorry for the confusion

@mssola mssola changed the title POC for the new Collector interface WIP: POC for the new Collector interface Apr 10, 2024
@mssola mssola added the wip Work in progress label Apr 10, 2024
Base automatically changed from refactor_zypper_implementation to main April 12, 2024 10:43
@likhitha77 likhitha77 removed the wip Work in progress label Apr 15, 2024
@likhitha77 likhitha77 changed the title WIP: POC for the new Collector interface POC for the new Collector interface Apr 15, 2024
mssola
mssola approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@mssola mssola left a comment

Choose a reason for hiding this comment

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

Nice!

digitaltom
digitaltom approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@digitaltom digitaltom left a comment

Choose a reason for hiding this comment

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

Looks good!

felixsch
felixsch reviewed Apr 15, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@felixsch felixsch merged commit 584e050 into main Apr 16, 2024
2 checks passed
@mssola mssola deleted the collector_interface branch July 4, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixsch felixsch felixsch left review comments

@digitaltom digitaltom digitaltom approved these changes

@mssola mssola mssola approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@likhitha77 @digitaltom @mssola @felixsch