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/snowflakereceiver] #31260

Merged
merged 11 commits into from
Apr 4, 2024

Conversation

shalper2
Copy link
Contributor

Description: Refactored the scrape function in scrape.go to take better advantage of go's language features. Namely, utilizing go routines and channels to parallelize the individual metric scrapes which make up the overall scrape function. This greatly improves the performance of the receiver.

Link to tracking Issue:

Testing: Unit tests and standalone collectors continue to pass/work

Documentation: No changes to the documentation.

@github-actions github-actions bot added the receiver/snowflake Snowflake receiver label Feb 14, 2024
@github-actions github-actions bot requested a review from dmitryax February 14, 2024 16:42
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 29, 2024
@shalper2 shalper2 force-pushed the snowflake-scrape-refactor branch from 686ee3d to fbdb211 Compare March 6, 2024 14:21
@shalper2 shalper2 marked this pull request as ready for review March 6, 2024 14:22
@shalper2 shalper2 requested a review from a team March 6, 2024 14:22
@github-actions github-actions bot removed the Stale label Mar 7, 2024
@shalper2 shalper2 force-pushed the snowflake-scrape-refactor branch 2 times, most recently from a229620 to 3bbf863 Compare March 13, 2024 14:30
@shalper2 shalper2 force-pushed the snowflake-scrape-refactor branch from f3d9b5a to f65a380 Compare March 18, 2024 21:51
@dmitryax
Copy link
Member

dmitryax commented Apr 2, 2024

Unfortunately, the Record...DataPoint functions generated by mdatagen are not thread-safe. You can parallelize the data fetching, but not the recording part

@dmitryax
Copy link
Member

dmitryax commented Apr 2, 2024

Actually, nevermind my previous comment. As long as we are not recording datapoints for the same metrics in different threads, we are good

}

wg.Wait()
errChan <- nil
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
errChan <- nil
close(errChan)

eOut <- errs
return
case err, ok := <-eQueue:
if err == nil || !ok {
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to send nil to the channel, just close the eQueue instead.

Suggested change
if err == nil || !ok {
if !ok {

@dmitryax dmitryax merged commit 64f56af into open-telemetry:main Apr 4, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/snowflake Snowflake receiver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants