Skip to content

[receiver/hostmetrics] Add skeleton for new hostmetrics scraper: nfsscraper #40050

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
Merged
merged 33 commits into from
Jun 11, 2025
Merged

[receiver/hostmetrics] Add skeleton for new hostmetrics scraper: nfsscraper #40050

merged 33 commits into from
Jun 11, 2025

Conversation

johnleslie
Copy link
Contributor

@johnleslie johnleslie commented May 13, 2025 •
edited
Loading

nfsscraper will provide metrics for the OS NFS client and server.

Link to tracking issue #39978

nfsscraper is not initialized from hostmetricsreceiver in this skeleton PR. I did need to include a single metric in metadata.yaml so mdatagen would create the required files for tests to pass.

@johnleslie
Add skeleton for new hostmetrics scraper: nfsscraper
1a7852d 
nfsscraper will provide metrics for the OS NFS client and server.
@johnleslie johnleslie requested review from dmitryax, braydonk and a team as code owners May 13, 2025 19:14
@braydonk
Copy link
Contributor

I added a new Makefile target to fix that codeowners action failure; if you update this branch you can run make gencodeowners to resolve the issue. I can approve the normal workflows after that.

@atoulme
Copy link
Contributor

atoulme commented May 15, 2025

Please add a changelog. Run make chlog-new from the root of the checkout to generate the changelog entry.

@johnleslie johnleslie force-pushed the add-nfsscraper-skeleton branch from 7d9e2fd to 41265e9 Compare May 15, 2025 00:47
@johnleslie
Copy link
Contributor Author

I added a new Makefile target to fix that codeowners action failure; if you update this branch you can run make gencodeowners to resolve the issue. I can approve the normal workflows after that.

Done

@johnleslie
Copy link
Contributor Author

Please add a changelog. Run make chlog-new from the root of the checkout to generate the changelog entry.

Done

@braydonk
Copy link
Contributor

In #40093 I did some cleanup in the systemscraper around OS support. You can do something similar here to fix the failing Windows tests.

@johnleslie johnleslie changed the title Add skeleton for new hostmetrics scraper: nfsscraper [receiver/hostmetrics] Add skeleton for new hostmetrics scraper: nfsscraper May 19, 2025
@johnleslie
Copy link
Contributor Author

In #40093 I did some cleanup in the systemscraper around OS support. You can do something similar here to fix the failing Windows tests.

I fixed up factory.go/factory_test.go in: 56caaad

However, generated_component_test.go is still failing the Windows tests: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/15123062752/job/42516261379?pr=40050

Suggestions on resolving this? I don't see OS checks in the other scrapers' generated_component_test.go.

@github-actions github-actions bot mentioned this pull request May 27, 2025
Closed
@github-actions github-actions bot mentioned this pull request Jun 3, 2025
Open
@braydonk
Copy link
Contributor

braydonk commented Jun 4, 2025

I think that CI failure would be fixable by resolving the conflicts on the branch.

@johnleslie
Copy link
Contributor Author

I think that CI failure would be fixable by resolving the conflicts on the branch.

done

braydonk
braydonk reviewed Jun 4, 2025
View reviewed changes
go.mod Outdated
@@ -16,3 +16,120 @@ retract (
v0.65.0
v0.37.0 // Contains dependencies on v0.36.0 components, which should have been updated to v0.37.0.
)

require (
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm not sure what happened here but doesn't look intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

receiver/hostmetricsreceiver/go.mod Outdated
@@ -29,6 +29,7 @@ require (
go.opentelemetry.io/collector/scraper v0.127.1-0.20250603105141-605011a1fea8
go.opentelemetry.io/collector/scraper/scraperhelper v0.127.1-0.20250603105141-605011a1fea8
go.opentelemetry.io/collector/scraper/scrapertest v0.127.1-0.20250603105141-605011a1fea8
go.opentelemetry.io/collector/semconv v0.127.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this module version needs to match the other go.opentelemetry.io/collector modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

andrzej-stencel
andrzej-stencel approved these changes Jun 9, 2025
View reviewed changes
Copy link
Member

@andrzej-stencel andrzej-stencel left a comment

Choose a reason for hiding this comment

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

Looks good from my side, @braydonk PTAL if you can

braydonk
braydonk approved these changes Jun 9, 2025
View reviewed changes
@braydonk braydonk added the ready to merge Code review completed; ready to merge by maintainers label Jun 9, 2025
@github-actions github-actions bot mentioned this pull request Jun 10, 2025
Open
@songy23 songy23 merged commit a785ef3 into open-telemetry:main Jun 11, 2025
176 of 177 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 11, 2025
dd-jasminesun pushed a commit to DataDog/opentelemetry-collector-contrib that referenced this pull request Jun 23, 2025
@johnleslie @braydonk @dd-jasminesun
[receiver/hostmetrics] Add skeleton for new hostmetrics scraper: nfss…
73bb740 
…craper (open-telemetry#40050)

nfsscraper will provide metrics for the OS NFS client and server.

#### Link to tracking issue
[https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/39978](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/39978)

nfsscraper is not initialized from hostmetricsreceiver in this skeleton
PR. I did need to include a single metric in metadata.yaml so mdatagen
would create the required files for tests to pass.

---------

Co-authored-by: Braydon Kains <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrzej-stencel andrzej-stencel andrzej-stencel approved these changes

@braydonk braydonk braydonk approved these changes

@dmitryax dmitryax Awaiting requested review from dmitryax dmitryax is a code owner

Assignees

@andrzej-stencel andrzej-stencel

Labels
ready to merge Code review completed; ready to merge by maintainers receiver/hostmetrics reports/distributions/contrib.yaml reports/distributions/core.yaml reports/distributions/k8s.yaml waiting-for-code-owners
Projects
None yet
Milestone
next release
Development

Successfully merging this pull request may close these issues.
5 participants
@johnleslie @braydonk @atoulme @andrzej-stencel @songy23