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

[receiver/podman] do not skip lifecycle checks #29994

Open
codeboten opened this issue Dec 17, 2023 · 14 comments
Open

[receiver/podman] do not skip lifecycle checks #29994

codeboten opened this issue Dec 17, 2023 · 14 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed receiver/podman Stale

Comments

@codeboten
Copy link
Contributor

This test should not be skipped.

          add issue to not skip lifecycle checks

Originally posted by @codeboten in #29957 (comment)

@codeboten codeboten added help wanted Extra attention is needed good first issue Good for newcomers receiver/podman labels Dec 17, 2023
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

Copy link
Contributor

Pinging code owners for receiver/podman: @rogercoll. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@hamzmu
Copy link
Contributor

hamzmu commented Jan 11, 2024

@codeboten is this issue still open?

@codeboten
Copy link
Contributor Author

@hamzmu yup, tests are still skipped:

@hamzmu
Copy link
Contributor

hamzmu commented Jan 16, 2024

can you assign this issue to me?

@atoulme
Copy link
Contributor

atoulme commented Jan 20, 2024

Done!

@Sanket-0510
Copy link
Contributor

can I take this one?

@Sanket-0510
Copy link
Contributor

here also if you run the Lifecycle test for podmanreceiver it will fail
Screenshot from 2024-03-03 18-09-25

Error - permission denied

@hamzmu
Copy link
Contributor

hamzmu commented Mar 9, 2024

@codeboten @rogercoll any idea how to run the tests? This is one of my first prs and a similar issue suggested to ran the following:
make generate, make -C receiver/podmanreceiver test
giving me the following error/message:
Screen Shot 2024-03-09 at 12 08 28 AM
Is there anything else I need to validate the tests?

Copy link
Contributor

github-actions bot commented May 9, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label May 9, 2024
@rogercoll
Copy link
Contributor

WIP

@github-actions github-actions bot removed the Stale label May 10, 2024
@rogercoll
Copy link
Contributor

rogercoll commented May 10, 2024

The current lifecycle of the podman's receiver (also the docker one) depends on the start scraper's functionality. On start, it tries to get all the healthy containers so it does not need to fetch them all on every scrape. The lifecycle checks fail, as it requires a Podman's socket on startup (unix:///run/podman/podman.sock) to fetch all the containers: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/podmanreceiver/receiver.go#L72

I think for the dockerstats receiver works because the socket is available on the CI. It fails on my local environment (docker disabled):

=== RUN   TestComponentLifecycle/metrics-lifecycle
    generated_component_test.go:62: 
        	Error Trace:	/opentelemetry-collector-contrib/receiver/dockerstatsreceiver/generated_component_test.go:62
        	Error:      	Received unexpected error:
        	           	Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
        	Test:       	TestComponentLifecycle/metrics-lifecycle
--- FAIL: TestComponentLifecycle (0.01s)
    --- PASS: TestComponentLifecycle/metrics-shutdown (0.00s)
    --- FAIL: TestComponentLifecycle/metrics-lifecycle (0.00s)

@codeboten Do you know how this issue is approach by other components? Should we change the scraper start logic? It seems to me, that enabling the socket on the CI is more an integration test.

@rogercoll
Copy link
Contributor

For the moment, I create PR to align the shutdown method with the dockestats receiver.

andrzej-stencel pushed a commit that referenced this issue May 31, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

- Adds the shutdown scraper method. The implementation cancels the
`containerEventLoop` go routine. Similar to the dockerstats:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/receiver.go#L76
- Moves the configuration validation to the start method, in alignment
with the dockerstats receiver:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/receiver.go#L55

**Link to tracking Issue:** Related issue
#29994

**Testing:** <Describe what testing was performed and which tests were
added.>
Use scraper instead of metrics receiver interface.

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jul 10, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jul 11, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

- Adds the shutdown scraper method. The implementation cancels the
`containerEventLoop` go routine. Similar to the dockerstats:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/receiver.go#L76
- Moves the configuration validation to the start method, in alignment
with the dockerstats receiver:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/receiver.go#L55

**Link to tracking Issue:** Related issue
open-telemetry#29994

**Testing:** <Describe what testing was performed and which tests were
added.>
Use scraper instead of metrics receiver interface.

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed receiver/podman Stale
Projects
None yet
Development

No branches or pull requests

5 participants