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

feat: add pyroscope.receive_http component for profile handling #1886

Merged
merged 7 commits into from
Oct 29, 2024

Conversation

marcsanmi
Copy link
Contributor

@marcsanmi marcsanmi commented Oct 15, 2024

PR Description

This PR introduces the new pyroscope.receive_http component, which allows for receiving and forwarding Pyroscope profiles. Key features and changes include:

  • HTTP server that listens for profile data on the /ingest endpoint
  • Ability to forward received profiles to multiple pyroscope.write components
  • Tests covering various scenarios (profile sizes, error conditions, etc.)
  • Updated documentation explaining component usage and configuration options

Which issue(s) this PR fixes

Relates to #270

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@marcsanmi marcsanmi force-pushed the marcsanmi/add-pyroscope-receiver-ingest-support branch from 9ff53cf to 8181a7f Compare October 15, 2024 10:22
@marcsanmi marcsanmi force-pushed the marcsanmi/add-pyroscope-receiver-ingest-support branch from 8181a7f to ffa4dbd Compare October 18, 2024 14:07
@marcsanmi marcsanmi marked this pull request as ready for review October 21, 2024 07:35
@marcsanmi marcsanmi force-pushed the marcsanmi/add-pyroscope-receiver-ingest-support branch from 5faaeab to f536f23 Compare October 28, 2024 14:53
@marcsanmi
Copy link
Contributor Author

Let's wait for review from a Pyroscope team member as well before merging 🙂

Copy link
Contributor

@korniltsev korniltsev left a comment

Choose a reason for hiding this comment

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

LGTM with one nit regarding reading the whole profile in memory.

internal/component/pyroscope/write/write.go Outdated Show resolved Hide resolved
Copy link
Contributor

@korniltsev korniltsev left a comment

Choose a reason for hiding this comment

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

lgtm, great job

@mattdurham mattdurham enabled auto-merge (squash) October 29, 2024 12:19
Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Looks reasonable, will wait for clayton to review before approving.

Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Looks OK.

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Oct 29, 2024
@mattdurham mattdurham merged commit 044b2da into main Oct 29, 2024
19 checks passed
@mattdurham mattdurham deleted the marcsanmi/add-pyroscope-receiver-ingest-support branch October 29, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants