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

Pure Storage Flash Array receiver #14969

Merged

Conversation

dgoscn
Copy link
Contributor

@dgoscn dgoscn commented Oct 17, 2022

Description:
New Pure Storage FlashArray receiver component

Link to tracking Issue:
#14886

Testing:
Simple functional testing as with other components

Documentation:
The documentation is not ready at the moment, but will be displayed on README.md alongside with documentation.md inside the receiver/purefareceiver project

@dgoscn dgoscn requested a review from a team as a code owner October 17, 2022 01:27
@dgoscn
Copy link
Contributor Author

dgoscn commented Oct 17, 2022

cc @jpkrohling

receiver/purefareceiver/README.md Outdated Show resolved Hide resolved
receiver/purefareceiver/README.md Outdated Show resolved Hide resolved
receiver/purefareceiver/config.go Show resolved Hide resolved
receiver/purefareceiver/config.go Outdated Show resolved Hide resolved
receiver/purefareceiver/scraper.go Outdated Show resolved Hide resolved
jpkrohling
jpkrohling previously approved these changes Oct 19, 2022
@jpkrohling jpkrohling added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 19, 2022
@jpkrohling jpkrohling dismissed their stale review October 19, 2022 17:15

sorry, this is still missing some things

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Please, add this component also to other plumbing places, like dependabot and code owners. We might have a guide with the requirements for the initial commit. If not, let me know and I'll find a good example to follow.

.github/CODEOWNERS Outdated Show resolved Hide resolved
jpkrohling
jpkrohling previously approved these changes Oct 20, 2022
@jpkrohling
Copy link
Member

The CI is failing with:

Error: receiver/purefareceiver/factory_test.go:1:1: expected 'package', found 'EOF'

You can reproduce it locally with make lint:

$ make lint
factory_test.go:1:1: expected 'package', found 'EOF'
factory_test.go:1:1: expected 'package', found 'EOF'
addlicense FAILED => add License errors:\n
./receiver.go
./factory_test.go\n
Use 'make addlicense' to fix this.
make: *** [../../Makefile.Common:90: checklicense] Error 1

@dgoscn
Copy link
Contributor Author

dgoscn commented Oct 21, 2022

The CI is failing with:

Error: receiver/purefareceiver/factory_test.go:1:1: expected 'package', found 'EOF'

You can reproduce it locally with make lint:

$ make lint
factory_test.go:1:1: expected 'package', found 'EOF'
factory_test.go:1:1: expected 'package', found 'EOF'
addlicense FAILED => add License errors:\n
./receiver.go
./factory_test.go\n
Use 'make addlicense' to fix this.
make: *** [../../Makefile.Common:90: checklicense] Error 1

Yes, thank you @jpkrohling. I am already working on these files with this behavior (blank files), until the end of the day they will be filled then the CI (I hope) will make the correct lint

@bogdandrutu
Copy link
Member

@jpkrohling since your review, seem to be lots of changes, can you do another round and merge the PR :)

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

I started reviewing it again, but not sure it's ready for review. @dgoscn, can you please confirm? I think there's value in having a PR with the config only and the basic skeleton, with the implementation for the array scraper being on the next PR.

receiver/purefareceiver/README.md Show resolved Hide resolved
receiver/purefareceiver/array_scraper.go Outdated Show resolved Hide resolved
receiver/purefareceiver/array_scraper_test.go Outdated Show resolved Hide resolved
@jpkrohling jpkrohling dismissed their stale review November 23, 2022 13:36

revoking approval, as the PR has changed

@dgoscn dgoscn marked this pull request as draft November 25, 2022 13:35
@jpkrohling jpkrohling force-pushed the purestorage_flasharray_receiver branch 2 times, most recently from 3322c55 to 8bcedf3 Compare November 28, 2022 12:57
@jpkrohling jpkrohling marked this pull request as ready for review November 28, 2022 12:58
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@jpkrohling jpkrohling merged commit d733380 into open-telemetry:main Nov 28, 2022
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants