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

chore: [receiver/hostmetrics] Fix integration tests #36847

Merged

Conversation

rogercoll
Copy link
Contributor

Description

  1. Integration tests that use /proc should not compare the PID, parent PID and owner attributes as they depend on the environment:
                        pmetrictest.IgnoreResourceAttributeValue("process.owner"),
			pmetrictest.IgnoreResourceAttributeValue("process.parent_pid"),
			pmetrictest.IgnoreResourceAttributeValue("process.pid"),
  1. and 3. Root path was not correctly set, as the Config.Unmarshal() method is not called during this initialization. Fixed by using scraper's config interface:
                                pCfg.SetRootPath(rootPath)
				pCfg.SetEnvMap(setGoPsutilEnvVars(rootPath, &osEnv{}))
  1. Some testdata/e2e/proc/1 required files were missing (statm, exe, cgroup, status).

Link to tracking issue

Fixes #32536

Testing

Documentation

@rogercoll
Copy link
Contributor Author

Skip changelog?

@rogercoll rogercoll changed the title [receiver/hostmetrics]: Fix integration tests chore: [receiver/hostmetrics] Fix integration tests Dec 16, 2024
@braydonk
Copy link
Contributor

Triage: Adding skip-changelog since this is purely a test fix.

@braydonk braydonk added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 16, 2024
Copy link
Contributor

@braydonk braydonk 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 this!

- key: process.owner
value:
stringValue: atoulme
stringValue: neck
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're ignoring these attributes now anyway maybe these can be removed? Or does the integration test framework expect them in some way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, the test framework actually removes them, so let's remove them as well 3b5936a

@dmitryax dmitryax merged commit defc07b into open-telemetry:main Dec 17, 2024
161 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 17, 2024
@rogercoll rogercoll deleted the fix_hostmetrics_integration_tests branch December 17, 2024 06:50
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/hostmetrics Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[receiver/hostmetrics] Process scrape integration test failing
4 participants