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

[podmanreceiver] add scraper's shutdown method #32981

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

rogercoll
Copy link
Contributor

Description:

Link to tracking Issue: Related issue #29994

Testing:
Use scraper instead of metrics receiver interface.

Documentation:

@rogercoll rogercoll requested a review from a team as a code owner May 10, 2024 14:04
@rogercoll rogercoll requested a review from atoulme May 10, 2024 14:04
@rogercoll
Copy link
Contributor Author

No changes on the end user, skip changelog?

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

I think since shutdown wasn't being called properly before (and we were leaking a goroutine), this can be considered a bug fix, so we probably want a changelog.

receiver/podmanreceiver/receiver.go Outdated Show resolved Hide resolved
receiver/podmanreceiver/receiver_test.go Outdated Show resolved Hide resolved
@crobert-1
Copy link
Member

CI/CD failure:

Error: ../../receiver/podmanreceiver/factory.go:54:10: assignment mismatch: 1 variable but newMetricsReceiver returns 2 values
Error: ../../receiver/podmanreceiver/factory.go:54:51: not enough arguments in call to newMetricsReceiver
	have (receiver.CreateSettings, *Config, nil)
	want (context.Context, receiver.CreateSettings, *Config, consumer.Metrics, any)
make: *** [Makefile:[32](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9033747495/job/24824836747?pr=32981#step:7:33)7: otelcontribcol] Error 1

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

.chloggen/add_shutdown_podman.yaml Outdated Show resolved Hide resolved
rogercoll and others added 3 commits May 13, 2024 20:24
Co-authored-by: Curtis Robert <crobert@splunk.com>
Co-authored-by: Curtis Robert <crobert@splunk.com>
Copy link

linux-foundation-easycla bot commented May 19, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@atoulme
Copy link
Contributor

atoulme commented May 19, 2024

See CLA issues.

@rogercoll
Copy link
Contributor Author

rogercoll commented May 20, 2024

Seems to be related with the suggested applied commits. Could you take a look @crobert-1 ?

@crobert-1
Copy link
Member

/easycla

@crobert-1
Copy link
Member

My apologies, not sure what's going on here. I've filed a support ticket against EasyCLA, I'll share any relevant updates.

@crobert-1
Copy link
Member

Ticket update:

On Friday we released co-author checks in EasyCLA. It seems there is a bug win the logic that is causing the missing ID on commit scenario. I am engaging our engineers to investigate this further. Thank you for reporting this issue.

@jarias-lfx
Copy link

/easycla

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

Successfully merging this pull request may close these issues.

None yet

5 participants