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

Refactor podman #10421

Merged
merged 24 commits into from
Jul 8, 2022
Merged

Refactor podman #10421

merged 24 commits into from
Jul 8, 2022

Conversation

rogercoll
Copy link
Contributor

Description:

  • Changes Podman client to Libpod Client (variable naming)
  • Adds list and event Libpod API queries
  • Main logic changed: instead of scraping all container stats this approach maintains a map of running containers and fetches their stats one by one. This modification is to align the logic with the Docker's receiver, the solutions makes it very easily to add new features (e.g container filtering).
  • Adds container's image in the metrics as attribute.

Link to tracking Issue: #9013

Testing: Added for the corresponding functions.

Documentation: No change required

@rogercoll
feat: move podman client to libpod client
e8628fd
@rogercoll
feat: add new container scraper entity
9dfa0e9
@rogercoll
feat: add new methods to client
8cc29c6 
- list the running containers
- events
@rogercoll
feat: new logic to container scraper
bb6ceef 
The container scraper spawns a go routine to listen for container events
and keep track of the running containers. Only containers in the local
map will be fetched, thus we can easily add inclusion/exclusion filters.
In addition, the containers map contain extra information like the
pod name.
@rogercoll
feat: fetch stats per container
0179ecd
@rogercoll
feat: add list container test
02dce97
@rogercoll
feat: add test case for image attribute
8a18a8f
@rogercoll rogercoll requested a review from a team as a code owner May 29, 2022 14:11
@rogercoll rogercoll requested a review from djaglowski May 29, 2022 14:11
@rogercoll rogercoll force-pushed the refactor_podman branch 2 times, most recently from 5c752da to a87118a Compare May 29, 2022 15:07
@rogercoll rogercoll mentioned this pull request Jun 10, 2022
Closed
TylerHelmuth
TylerHelmuth reviewed Jun 10, 2022
View reviewed changes
receiver/podmanreceiver/libpod_client.go Show resolved Hide resolved
receiver/podmanreceiver/podman.go Outdated Show resolved Hide resolved
receiver/podmanreceiver/podman.go Outdated Show resolved Hide resolved
receiver/podmanreceiver/podman.go Outdated Show resolved Hide resolved
receiver/podmanreceiver/libpod_client_test.go Show resolved Hide resolved
dmitryax
dmitryax reviewed Jun 17, 2022
View reviewed changes
receiver/podmanreceiver/podman.go Outdated Show resolved Hide resolved
receiver/podmanreceiver/libpod_data.go Outdated Show resolved Hide resolved
TylerHelmuth
TylerHelmuth reviewed Jun 27, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
mx-psi
mx-psi reviewed Jul 5, 2022
View reviewed changes
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

The changelog process changed. Could you revert your changes to CHANGELOG.md and follow the instructions on the contributing docs to add a changelog entry? You may need to create a tracking issue. Thanks!

dmitryax
dmitryax approved these changes Jul 5, 2022
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the delay. LGTM. Please rebase and update changelog entry as @mx-psi mentioned

receiver/podmanreceiver/go.mod Outdated Show resolved Hide resolved
@TylerHelmuth
Copy link
Member

@rogercoll I think we need 1 more make gotidy then we'll be good to go.

@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label Jul 8, 2022
@dmitryax dmitryax merged commit e149ae2 into open-telemetry:main Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mx-psi mx-psi mx-psi left review comments

@TylerHelmuth TylerHelmuth TylerHelmuth approved these changes

@dmitryax dmitryax dmitryax approved these changes

@djaglowski djaglowski Awaiting requested review from djaglowski djaglowski is a code owner automatically assigned from open-telemetry/collector-contrib-approvers

Assignees

@codeboten codeboten

Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rogercoll @TylerHelmuth @mx-psi @dmitryax @codeboten