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 added 7 commits May 29, 2022 13:53
@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 review from a team and 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
rogercoll and others added 2 commits June 14, 2022 17:47
@rogercoll @TylerHelmuth
Update receiver/podmanreceiver/podman.go
52d22e0 
Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
@rogercoll
change variable name for listing containers
37c5df1
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

@TylerHelmuth TylerHelmuth TylerHelmuth approved these changes

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

@dmitryax dmitryax dmitryax approved these changes

@djaglowski djaglowski Awaiting requested review from djaglowski

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